OS: Ubuntu 18.04.4 LTS
PHP Version: 7
Drupal Version: 8.8.x

Problem/Motivation

- The .ajax-progress--fullscreen z-index is set to 1000 in '/claro/css/components/ajax-progress.module.css', line 141.
- The .ui-dialog z-index is set to 1260 in '/claro/css/components/ajax-progress.module.css?', line 14.
- For people opening a single dialog, this is fine, as the .ajax-progress--fullscreen is replaced by the .ui-widget-overlay and the dialog itself. However, when the .ajax-progress--fullscreen is triggered from an already open dialog, the current dialog remains exposed for accidental multiple submissions at z-index 1260 (until it is replaced by the new dialog) as the loader is running behind it at index 1000.

Proposed resolution

Increase the z-index of the .ajax-progress--fullscreen class above that of the .ui-dialog class.

From #31
Setting the z-index for .ajax-progress--fullscreen class to 1300 because the z-index for .ui-dialog class is 1260 and any thing which is above 1260 shall work.You can find the z-indexes in drupal for reference.

From #34
Based on this rationale, the z-index should be 1261 then as we want to avoid z-index creep. Either the z-index or the explanation should be modified.

CommentFileSizeAuthor
#26 interdiff_22-26.txt411 bytesAbhisheksingh27
#26 3118813-26.patch1.56 KBAbhisheksingh27
#22 interdiff-17_22.txt2.17 KBGauravvvv
#22 3118813-22.patch2.17 KBGauravvvv
#17 3118813-17.patch2.16 KBranjith_kumar_k_u
#14 drupal-bump_up_ajax_spinner_z_index_for_modals-3118813-14-8.x.patch2.68 KBGuyPaddock
#13 drupal-bump_up_ajax_spinner_z_index_for_modals-3118813-13-8.x.patch2 KBGuyPaddock
#7 ajax-progress-3118813.patch935 byteszliante

Issue fork drupal-3118813

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Therapychild created an issue. See original summary.

Therapychild’s picture

Therapychild CreditAttribution: Therapychild commented
Therapychild’s picture

Therapychild CreditAttribution: Therapychild commented
Therapychild’s picture

Therapychild CreditAttribution: Therapychild commented
Therapychild’s picture

Therapychild CreditAttribution: Therapychild commented
Therapychild’s picture

Therapychild CreditAttribution: Therapychild commented
zliante’s picture

zliante CreditAttribution: zliante at Websolutions Agency commented
FileSize
ajax-progress-3118813.patch935 bytes

Hello, I have looked at your issue and went ahead and resolved it for you. Please test and verify.

zliante’s picture

zliante CreditAttribution: zliante at Websolutions Agency commented
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina at Lullabot commented
Status: Active » Needs work

Going from 1000 to 9999 for a z-index is too much. I'm pretty sure there's a number that still makes sense and it's higher that 1000 (maybe 1500?). Probably some research to know which other values are out there would be useful.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina at Lullabot commented
Related issues: +#3023311: Modal dialog style update

Maybe this is something that might be affected by #3023311: Modal dialog style update?

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

GuyPaddock’s picture

GuyPaddock CreditAttribution: GuyPaddock at Inveniem commented

We just ran into this with the Media Library. Without this patch, there's no feedback to the user when switching between vertical tabs or performing searches/filters in the modal.

GuyPaddock’s picture

GuyPaddock CreditAttribution: GuyPaddock at Inveniem commented
FileSize
drupal-bump_up_ajax_spinner_z_index_for_modals-3118813-13-8.x.patch2 KB

It looks like this also needs to be bumped up in core/modules/system/css/components/ajax-progress.module.css as well. Attached is a re-roll with that file patched as well.

I see no issue with 9999 since ui-tooltip (another floating/hovering UI affordance) uses the same z-index according to:
https://www.drupal.org/docs/theming-drupal/z-indexes-in-drupal-8

That page will need to be updated after this patch goes in.

GuyPaddock’s picture

GuyPaddock CreditAttribution: GuyPaddock at Inveniem commented
Status: Needs work » Needs review
FileSize
drupal-bump_up_ajax_spinner_z_index_for_modals-3118813-14-8.x.patch2.68 KB

Looks like core/themes/stable/css/system/components/ajax-progress.module.css was also missed. Attached patch includes that as well.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commented
FileSize
3118813-17.patch2.16 KB

Rerolled #14 for 9.4

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

per #9 going to 9999 is too high. Whatever version is decided and why should be added to the issue summary please.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented
Issue summary: View changes

Currently .ui-dialog have a z-index of 1260. As suggested in #9, we can have z-index as 1500.

--jui-dialog-z-index: 1260;
.ui-dialog {
  z-index: var(--jui-dialog-z-index);
}
Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3118813-22.patch2.17 KB
interdiff-17_22.txt2.17 KB

Updated issue summary and attached patch with interdiff for same. please review

Status: Needs review » Needs work

The last submitted patch, 22: 3118813-22.patch, failed testing. View results

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented
Status: Needs work » Needs review

Unrelated failure. restoring the status

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

#9 mentioned adding some research to land on 1500. Can you provide what that was and add to the IS please.

Abhisheksingh27’s picture

Abhisheksingh27 CreditAttribution: Abhisheksingh27 at OpenSense Labs for DrupalFit commented
Status: Needs work » Needs review
FileSize
3118813-26.patch1.56 KB
interdiff_22-26.txt411 bytes

Adding reroll for 10.1.x as above patch failed on 10.1.x.
Removed changes of "b/core/themes/stable/css/system/components/ajax-progress.module.css" because it doesn't exist in 10.1.x

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

Doesn't answer #25

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Issue tags: +Field UX

Tagging with Field UX as upcoming changes will potentially add more usages of ajax progress in dialogs.

Utkarsh_33 made their first commit to this issue’s fork.

Utkarsh_33 opened merge request !3814

Utkarsh_33’s picture

Utkarsh_33 CreditAttribution: Utkarsh_33 at Acquia commented

Setting the z-index for .ajax-progress--fullscreen class to 1300 because the z-index for .ui-dialog class is 1260 and any thing which is above 1260 shall work.You can find the z-indexes in drupal for reference.

Utkarsh_33’s picture

Utkarsh_33 CreditAttribution: Utkarsh_33 at Acquia commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

Thank for the reasoning. Added that to the issue summary too

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Status: Reviewed & tested by the community » Needs work

Re #31

Setting the z-index for .ajax-progress--fullscreen class to 1300 because the z-index for .ui-dialog class is 1260 and any thing which is above 1260 shall work.You can find the z-indexes in drupal for reference.

Based on this rationale, the z-index should be 1261 then as we want to avoid z-index creep. Either the z-index or the explanation should be modified.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Needs work » Reviewed & tested by the community

Good suggestion @bnjmnm, thanks @Utkarsh_33 for the prompt change

  • bnjmnm committed 0a32294b on 10.1.x authored by Utkarsh_33 
    Issue #3118813 by Utkarsh_33, GuyPaddock, Gauravvvv, Abhisheksingh27,...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Tested manually with every core theme. This is fixed where it needs to be fixed, and doesn't introduce changes where nothing needs to be fixed. Committed to 10.1.x.

Status: Fixed » Closed (fixed)

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