Admin toolbar menu z-index sucks with devel module

Created on 23 January 2023, over 2 years ago

Problem/Motivation

When devel module is installed, goes Admin toolbar drop down menu behind itยดs status message.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

๐Ÿ› Bug report
Status

Active

Version

3.3

Component

User interface

Created by

๐Ÿ‡ซ๐Ÿ‡ฎFinland Klaus Juhantalo

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Klaus Juhantalo
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia rajneeshb New Delhi

    Devel sf-dump class having z-index 99999, so to fix the issue for toolbar menu added z-index to 100000.

  • Status changed to RTBC about 2 years ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom james.williams

    Works for me! That said, I wonder if a fix should really go into core, since core's own toolbar gets obscured by the output from the dumper too. admin_toolbar could probably then just make use of core's fix. Up to this module's maintainer really?

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada adriancid Montreal, Canada

    Thanks

  • Status changed to Fixed about 2 years ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium joevagyok

    The changed z-index causes a problem with media and other dialogues, because the toolbar z-index the closing of the dialogues is not clickable any more.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States butterwise

    The changed z-index also causes a problem with Ckeditor: When you maximize a formatted text field using Ckeditor, the admin toolbar shifts to the left-vertical position and overlays the editor window:

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom jessebaker

    This change to using an enormous z-index has also broken aspects of Acquia Site Studio which assumes the admin toolbar's z-index to be 502 which it has been for years.

    Will it be reverted because otherwise we will need to resolve the issues it causes.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium joevagyok

    I tried to contact the maintainers on slack but haven't received feedback on the issue so far. I found another issue created related to this.
    @jessebaker Just lock the version to 3.3.0 for now in composer until it gets resolved.

  • Status changed to Active about 2 years ago
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada adriancid Montreal, Canada

    I'm reverting this issue due to the problems described at ๐Ÿ› New z-index is breaking other functionalities Closed: duplicate

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom jessebaker

    Thanks @adriancid. However I think you also need to remove the duplicate style declaration on line 1 of the same file -

    #toolbar-bar {
      z-index: 100000;
    }

    was in that file twice.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada adriancid Montreal, Canada

    @jessebaker can you check the last changes? I can't see any duplicate code there.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada adriancid Montreal, Canada

    The patch here is causing this error ๐Ÿ› Top search results are hidden under the toolbar. Closed: works as designed

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom jessebaker

    Hmm I might be wrong, sorry!

    If you look here https://git.drupalcode.org/project/admin_toolbar/-/blob/3.3.2/css/admin.... you can see the following code twice (on lines 1 and again on 9)

    #toolbar-bar {
      z-index: 100000;
    }

    It looked like the commit linked in your comment only removed the second one - but looking at 3.x it looks like both are gone!

    https://git.drupalcode.org/project/admin_toolbar/-/blob/3.x/css/admin.to...

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Harish1688 India

    Hi

    To address the issue, the requirement states that the toolbar menu should be positioned above the status message.
    However, applying the CSS property z-index: 100000; to the #toolbar-bar class caused some unforeseen problems.

    In order to resolve this for admin toolbar module, creating a - patch menu-z-index-sucks-with-devel-module-3335841-17.patch. override the CSS (z-index:0;) of the devel module specifically for the admin toolbar by modifying the "admin.toolbar_search.css" file.

  • Status changed to Needs review about 2 years ago
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 8
    last update about 2 years ago
    17 pass
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Harish1688 India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Jaspreet-Kaur

    Verified! After applying patch #17 issue seems to be fixed. Thanks!

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada adriancid Montreal, Canada

    I will leave open the issue until the other user reporting problems write a comment saying is working for them.

  • ๐Ÿ‡น๐Ÿ‡ทTurkey Kartagis Istanbul

    The patch at #2 works for me, I don't know about the other issues mentioned.

  • Status changed to Active 4 months ago
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance dydave
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance dydave

    I've seen the previous history on this issue and it seems patch from #2 broke the module.

    However, if anybody would have a better suggestion or idea, for a CSS rule, it would be warmly welcome.

    It looks like this is still an issue, see screenshot below on latest 3.x:

    Although it is rather minor, since it only occurs when developing which is probably not "too" much of a problem for developers.
    But indeed, if this could be fixed, it would improve developers experience, therefore I've switched this over to Feature request.

    We're accepting merge requests if anyone is interested in working on this.
    Thanks in advance!

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sandip

    I am looking into it.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sandip

    Hi @everyone i observed this issue. As mentioned in #23 and #9 it is clear that we can not increase the z-index of #toolbar-bar. But some of the sections has z-index around 999 upto 99999. Thats why we are facing this issue. Here the patch at #17 i dont think it is properly solving the issue. The same issue is occuring for CK Editor also. Here is the the issue queue of CK Editor -> https://www.drupal.org/project/admin_toolbar/issues/3426402 ๐Ÿ› zindex issue between admin toolbar and ckeditor 5 Active

    So i think if we cant increase z-index of #toolbar-bar so can we apply the same approach to its parent div which is #toolbar-administration ?
    Like we can use position: relative and z-index around 99999.

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance dydave

    dydave โ†’ changed the visibility of the branch 3.x to hidden.

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance dydave

    Thanks Sandip (@sandip) for the recent feedback on this issue!

    So i think if we cant increase z-index of #toolbar-bar so can we apply the same approach to its parent div which is #toolbar-administration ?
    Like we can use position: relative and z-index around 99999.

    Could you please try translating these suggestions into actual changes in the following branch?
    https://git.drupalcode.org/issue/admin_toolbar-3335841/-/tree/3335841-fi...

    Then try testing a bit with different types of settings for the Admin Toolbar module, browsing on various pages of the site.

    That would definitely be very helpful ๐Ÿ™‚
     

    Otherwise, I personally think this issue is probably different from the one with the CKEditor5 Toolbar ( ๐Ÿ› zindex issue between admin toolbar and ckeditor 5 Active ).

    But, at this point, it is still unclear whether a CSS or JS solution should be preferred.

    It could still be worth exploring potential JS solutions as well:
    For example, adding/removing CSS classes, altering tags styles, etc... on initialization, when the devel module is enabled and/or a Kint or Symfony vardump is found on page.

    Any testing, feedback and reviews would be greatly appreciated.
    Thanks in advance!

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States justcaldwell Austin, Texas

    IMO, this is not an issue with admin_toolbar, and I wouldn't try to solve it here. The problem also occurs if you're just using the core toolbar module without admin_toolbar.

    The issue stems from devel's dumper using such a high z-index (99999). Actually, it's upstream from devel, as the (default) dumper functionality comes from Symfony var-dumper. I assume there's a reason for the high z-index, but maybe an issue should be opened in one or both of those projects.

    If you wanted to conditionally raise the toolbar z-index in the presence of a dump, you could use something like:

    body:has(.sf-dump) .toolbar-oriented .toolbar-bar {
      z-index: 100000;
    }

    Note: This might need other classes in addition to .sf-dump.

    That would still have the same conflicts with other modules mentioned above, but only when there's a dump on the page. Maybe it's worthwhile if devel is only temporarily enabled (?). As I said, I don't think this is amin_toolbar's problem to solve.

Production build 0.71.5 2024