Updated: Comment #0

Problem/Motivation

When a modal is open and the entire modal isn't visible on the screen, you can't scroll down to access the Save button.

There are a few other issues going with modal updates, however this is patch is simply to fix the bug with scrolling.

Proposed resolution

Patch from nod_ #9 in #2067263 fixes this bug.

Steps to test:
Before patch (using Chrome, haven't replicated in other browsers)

  1. On a clean Drupal 8 install, go to admin/structure/block
  2. Click on one of the blocks to place (or go to another page and open a modal)
  3. Ensure that the entire modal is not visible in your browser window
  4. Try to scroll down to click Save
  5. Scrolling doesn't work, you have to refresh the page
  6. Same problem in admin/config/development/sync page
  7. Same problem in node/add when trying to insert an image with WYSIWYG editor

After patch

  1. Apply patch
  2. Do the same
  3. Scrolling works

#2067263: Drupal dialog placement must take displacing viewport elements, like the Toolbar, into account when calculating placement
#2072037: Drupal dialog modal background z-index is set too low to reliably occlude core UI components

Files: 
CommentFileSizeAuthor
#18 core-js-resiez-2095225-17.patch3.81 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 58,428 pass(es).
[ View ]
#15 core-js-resiez-2095225-15.patch4.23 KBnod_
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-js-resiez-2095225-15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 core-js-resiez-2095225-12.patch3.81 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 58,941 pass(es).
[ View ]
#12 core-js-resiez-2095225-12.patch4.53 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 58,696 pass(es).
[ View ]
#11 Synchronize configuration drupal8.localhost.png50.94 KBpameeela
#10 core-js-resiez-2095225-10.patch3.3 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 58,507 pass(es).
[ View ]
#6 Screen Shot 2013-09-22 at 17.18.41.png75.3 KBWim Leers
#3 core-js-resiez-2095225-3.patch2.71 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 59,219 pass(es).
[ View ]
core-js-modal-position-2067263-9.patch346 bytespameeela
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Comments

Status:Active» Needs review

Status:Needs review» Needs work

To be clear, this happens on windows that are not tall enough to fit the modal, correct? I don't think the patch is a full fix - on windows like that the window then does not fit. Maybe we need to calculate the height of the window and height of the modal and only disable autoResize if the modal is bigger than window?

Status:Needs work» Needs review
StatusFileSize
new2.71 KB
PASSED: [[SimpleTest]]: [MySQL] 59,219 pass(es).
[ View ]

After reviewing where autoresize is used, turned out it gets in the way more than it helps and the initial justification to get it in is not convincing #1879120-75: Use Drupal-specific image and link plugins — use core dialogs rather than CKEditor dialogs, containing alterable Drupal forms.

Patch takes out autoresize and makes dialogs works everywhere.

Status:Needs review» Reviewed & tested by the community

Patch applies well and it fixes the problem so let's wait for testbot and RTBC! :)

@meba that may be a cleaner solution but for now I think it's critical to get a fix in. Maybe you can create a follow up issue to implement that?

Tested the patch and it resolves the issue of not being able to access the entire modal.

Status:Reviewed & tested by the community» Needs work
StatusFileSize
new75.3 KB

This is definitely not RTBC. This is not fundamentally broken, because it works fine here:

  1. Resize your browser to be e.g. 200 px high.
  2. Go to node/add/article.
  3. Click the "add image" button.

It'll look like this:
Screen Shot 2013-09-22 at 17.18.41.png

So we should look into why it breaks on admin/structure/block.


Removing autoResize, like #5 does, is problematic too, because now you can scroll your dialog away while your cursor is on the black "jQuery UI overlay" (not the Drupal overlay) that sits between the Drupal site and the jQuery UI Dialog. That's extremely weird.

You should never be able to scroll away a dialog. You should be able to scroll the contents of the dialog, not the dialog itself.

It's also broken at admin/config/development/sync when you click view difference, if there is more content than fits the screen. You can scroll down to view it all but it immediately jumps back to the top.

Priority:Normal» Major

Changing this to major. It stops me saving blocks settings on a 13-inch laptop, seems a pretty wide use case.

@Wim is it not possible to get this fix in as a workaround with the understanding that a better approach is needed afterward?

Assigned:Unassigned» nod_

ok, deeper bug with the ajax dialog integration.

Assigned:nod_» Unassigned
Status:Needs work» Needs review
StatusFileSize
new3.3 KB
PASSED: [[SimpleTest]]: [MySQL] 58,507 pass(es).
[ View ]

So many fails, so little lines.

When using data-dialog-options, the ajax framework wasn't putting the default dialog options either.

Still not happy with the code but now it works at least.

Patch fixes the bug so you can scroll. But now the modal opens with the bottom visible rather than starting from the top, though you can scroll up. Screenshot attached is what you get when you click in view differences.

Synchronize configuration   drupal8.localhost.png

Not sure what to mark this one as we've still got the page scrolling behaviour. I think I'll leave it for more feedback :)

StatusFileSize
new4.53 KB
PASSED: [[SimpleTest]]: [MySQL] 58,696 pass(es).
[ View ]

lol I hate this code.

It was a scope issue.

StatusFileSize
new3.81 KB
PASSED: [[SimpleTest]]: [MySQL] 58,941 pass(es).
[ View ]

wrong patch.

Status:Needs review» Needs work

Patch fixes the bug at admin/structure/block and admin/config/development/sync and the scrolling is inside the modal.

But there's a regression at node/add/article when inserting an image via WYSIWYG, it scrolls the whole page rather than just the modal.

StatusFileSize
new4.23 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-js-resiez-2095225-15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

BAM!

Status:Needs work» Needs review

double BAM!

Status:Needs review» Needs work

The last submitted patch, core-js-resiez-2095225-15.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.81 KB
PASSED: [[SimpleTest]]: [MySQL] 58,428 pass(es).
[ View ]

Triple BAM!

forgot to clean up my git repo before diffin'

Patch applies well and seems to fix the problem. Let's see what @pameeela thinks and go for RTBC!

Status:Needs review» Reviewed & tested by the community

It works! It works!

Tested at:

admin/structure/block
admin/config/development/sync (with a long diff)
node/add/article (when inserting an image via WYSIWYG)

and the scrolling is within in the modal.

Status:Reviewed & tested by the community» Fixed

OMG THIS IS MY FAVOURITE PATCH IN THE WORLD EVER!!!

Committed and pushed to 8.x. :D

omg, fastest js commit ever by webchick \o/

Check out #787896-46: Add a link so that administrators can return to their most recently visited non-admin page too, it will end up making people stop bitching about the overlay.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

Added other pages where you can reproduce the problem.