WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29074 closed defect (bug) (fixed)

Disable background page scrolling when modals are open

Reported by: celloexpressions Owned by: ocean90
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch commit
Focuses: ui Cc:

Description

For everything ranging from media modal/grid-details, to plugin install, and pretty much anywhere where the modal prevents interacting with the background page. Can use a common modal-open class to set body to overflow: hidden.

Themes already does this.

I'm working on a patch. If someone could make a list of modals where we should do this, that would be good. So far, have:

  • Media modal
  • Media grid attachment details modal
  • Plugin details modal

Attachments (2)

29074.diff (2.1 KB) - added by celloexpressions 6 years ago.
Disable page scrolling for media, standardize themes to use the same class.
29074.2.diff (3.0 KB) - added by celloexpressions 6 years ago.
Also fix for plugin details, although this requires patching thickbox. Not sure if we should do that, but it doesn't seem to cause issues anywhere else.

Download all attachments as: .zip

Change History (9)

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


6 years ago

@celloexpressions
6 years ago

Disable page scrolling for media, standardize themes to use the same class.

@celloexpressions
6 years ago

Also fix for plugin details, although this requires patching thickbox. Not sure if we should do that, but it doesn't seem to cause issues anywhere else.

#2 @celloexpressions
6 years ago

  • Keywords has-patch added; needs-patch removed

Looks like doing this for the plugin details modal would require patching thickbox - are we okay with doing that? Would only be necessary until we backbonify plugins.

Either way this is a pretty simple fix; 29074.diff without plugins/thickbox, 29074.2.diff with.

#3 @afercia
6 years ago

hi @celloexpressions, that would be great not just for the scrolling issue.
Please consider to extend your idea cause one thing that really misses it's an easy way to detect when *any* overlay + modal is "open" in the admin. I recently had the need to detect if an overlay was shown for a small plugin I'm using for development purposes where I need a couple of controls displayed with a higher stack order than the overlays. That makes sense since it's a plugin for development only but I bet many developers would find very useful something like: "if any overlay is open, then do this" thing.
I ended up doing my own thing checking for:

#fullscreen-overlay
#mce-modal-block
#wp-link-backdrop
.media-modal-backdrop
.theme-install-overlay
.wp-full-overlay
.ui-find-overlay
#TB_overlay
#wp-auth-check-bg
.notification-dialog-background

and then checking if they're :visible. And maybe I'm still missing some overlay :)
Of course, it would have been far easier to check just for "modal-open" on the body. Or "has-overlay", whatever :)

#4 follow-up: @ocean90
6 years ago

  • Keywords commit added

I will go with 29074.2.diff. Only one think I'm currently not sure: Is it possible that a modal height (probably thickbox) is larger than the viewport height, if so, does scrolling still works?

#5 in reply to: ↑ 4 @celloexpressions
6 years ago

Replying to ocean90:

I will go with 29074.2.diff. Only one think I'm currently not sure: Is it possible that a modal height (probably thickbox) is larger than the viewport height, if so, does scrolling still works?

I believe thickbox handles that by automatically resizing itself and doing an internal scrollbar (in the modal).

#6 @ocean90
6 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 29346:

Disable background page scrolling when modals are open.

props celloexpressions.
fixes #29074.

#7 @ocean90
6 years ago

#28911 was marked as a duplicate.

Note: See TracTickets for help on using tickets.