Problem/Motivation

The close even is not triggered when a modal dialog opens another modal dialog. This is a known bug: #2741877: Nested modals don't work: opening a modal from a modal closes the original. This bug in combination with #2707291: Disable body-level scrolling when a dialog is open as a modal results in behavior where if you have opened a dialog from a dialog, the body remains locked even after you've closed the dialog. This can be seen at least in the Views UI.

Steps to reproduce

  1. Go to Views UI
  2. Press "Add" in the Fields section
  3. Add any field
  4. Configure the field and click apply
  5. The dialog closes, but it's not possible to scroll 💥

After patch

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 gif.gif640.24 KBgauravvvv
#4 interdiff-2_4.txt358 bytesgauravvvv
#4 3377900-4.patch1.34 KBgauravvvv
#2 3377900-2.patch1.3 KBlauriii

Issue fork drupal-3377900

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
StatusFileSize
new 3377900-2.patch1.3 KB

This might be something worth writing a test for. Here's a fix for this.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For CC failure.

Anyway to add a test?

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new 3377900-4.patch1.34 KB
new interdiff-2_4.txt358 bytes
new gif.gif640.24 KB

Fixed custom command failure, keeping NW for tests

After patch

gauravvvv’s picture

gauravvvv commented
Issue summary: View changes
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue tags: +Needs tests
omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey

omkar.podey opened merge request !4520

omkar.podey’s picture

omkar.podey commented
Assigned: omkar.podey » Unassigned

.

omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey
smustgrave’s picture

smustgrave commented
Assigned: omkar.podey » Unassigned

Thank you for working on this but told per etiquette shouldn’t assign tickets to yourself unless a maintainer.

Simple comment that you’re working on it should be good.

omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey
smustgrave’s picture

smustgrave commented
Assigned: omkar.podey » Unassigned

Please see comment #11 here’s a link too for Drupal etiquette https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett...

Thanks

omkar.podey’s picture

omkar.podey commented
omkar.podey’s picture

omkar.podey commented

thanks, Working on it now.

omkar.podey’s picture

omkar.podey commented
Status: Needs work » Needs review
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Status: Needs review » Reviewed & tested by the community
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: -Needs tests
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

It looks like the test is passing locally even without the fix.

I'm also wondering if \Drupal\Tests\views_ui\FunctionalJavascript\DisplayTest is the best class to include this. Maybe \Drupal\Tests\views_ui\FunctionalJavascript\AdminAjaxTest would be a better fit?

omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey
omkar.podey’s picture

omkar.podey commented
Assigned: omkar.podey » Unassigned
Status: Needs work » Needs review
utkarsh_33’s picture

utkarsh_33 commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Tested this locally and the body is scrollable once the dialog is closed.Also it has the test coverage so removing the tags now.Marking it RTBC.

  • lauriii committed a5cd2d7a on 11.x 
    Issue #3377900 by omkar.podey, lauriii, Gauravvvv, Utkarsh_33: Scrolling...

lauriii closed merge request !4520

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed a5cd2d7 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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