Background overlay on theme page has layout issues

[zodiac1978]

In a fresh 6.2 installation (and with a big monitor) I see some layout issues if open the theme details modal.

When the modal is open, the fly out-menus are below the background overlay.

Additionally the overlay is not ending at the end of the page but earlier.

[zodiac1978]

Fly-out menu is hidden below overlay, but not for the complete page. because overlay is ending earlier (added red border for visual help)

[ironprogrammer]

Thank you for the report, @zodiac1978!

Would you please provide ​more information on how to reproduce this issue? For example, these questions come to mind:

• What are the dimensions of the browser when this occurs?
• What admin page is being viewed when it occurs?
• Have all plugins been disabled?

[zodiac1978]

Replying to ironprogrammer:

• What are the dimensions of the browser when this occurs?

"with a big monitor" was not very specific. Sorry for this!

On screens equal or lower 780 pixel width the theme details overlay is fullscreen, so there is no problem.

With 781 to 782 pixel width the layout is completely broken, because the menu isn't collapsed (see new screenshot).

With 783 pixel width and above the menu is under the theme layer.

• What admin page is being viewed when it occurs?

As already mentioned: "if open the theme details modal", so I'm on /wp-admin/themes.php?theme=twentytwentyone for example.

• Have all plugins been disabled?

This is already mentioned too. This happens "In a fresh 6.2 installation", so no plugins active.

Testings steps:

0. Set the viewport of your browser window to at least 783 pixel width
1. Install WP and login to dashboard
2. Visit Design -> Themes
3. Click on any of the default themes to open the themes details modal
4. Try to open one of the submenus of WordPress by hovering over the main menu entry
5. See that the flyout submenu is under the theme details modal

Not sure if the modal should be on top of everything or the menu. Then maybe this is a WONTFIX/WORKSFORME, but it looks broken. Especially if the overlay is not fullscreen and the submenu is partly not grayed out, as seen on the screenshot. I couldn't provide specific pixel widths for this, because this is locale specific (text is breaking differently, creating different heights of the modal).

[zodiac1978]

With 781 to 782 pixel width the layout is completely broken, because the menu isn't collapsed

[ironprogrammer]

Thanks for the detailed feedback, @zodiac1978! I can confirm this behavior. I'd like to put this into the 6.3 milestone for consideration. Regardless of the question relating to the menu being above/under the modal, this 2px display gap seems like an addressable fix.

Reproduction Report

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 13.4
• Browser: Safari 16.5
• Server: nginx/1.25.0
• PHP: 7.4.33
• WordPress: 6.3-alpha-55505-src
• Theme: twentytwentythree v1.1
• Active Plugins:
  • gutenberg v15.9.1

Actual Results

• ✅ At 781-782px width, the mobile menu is active, and the theme previewer shifts from full-screen to modal.

Supplemental Artifacts

Figure 1: Menu display behavior between 780px and 783px+

[orestissam]

Updates the media query for the theme details overlay, so it matches the breakpoints of the navigation menu.

[monzuralam]

Test Report

Environment

• Hardware: HP 11th Gen Intel(R) Core(TM) i5-1155G7 @ 2.50GHz 2.50 GHz
• OS: Windows 11
• Browser: Chrome 114.0.5735.110 & Firefox 114.0.1 (64-bit)
• Server: Apache/2.4.56
• PHP: 8.0.29
• WordPress: 6.3-alpha-55905
• Theme: twentytwentythree v1.1
• Active Plugins: gutenberg v16.0.0-rc.4

Steps to Reproduce

1. Navigate Appearance > Themes
2. Themes > Click twentytwentythree theme & open the popup
3. Testing Breakpoint

Expected Results

When testing a patch to validate it works as expected:

• ✅ Got Expected results.
• ✅ https://core.trac.wordpress.org/attachment/ticket/58164/58164.patch working as well.

Supplemental Artifacts

Image - ​https://drive.google.com/file/d/1rbQPOEBJri8Q2_M4WzlJ290DKVuJ7zov/view?usp=sharing
Video - ​https://www.awesomescreenshot.com/video/18238716?key=7b4e65b1e0144258430acf2a78738621

[oglekler]

I am adding needs-testing to highlight that this patch is ready, and testing is also not limited by one test.

[audrasjb]

Before patch, at 782px ❌

[audrasjb]

After patch at 782px ✌️✅

[audrasjb]

Patch also successfully tested on my side (see screenshots above).
Marking for commit.

[audrasjb]

In 56104:

Themes: Fix layout issue on the Themes page background overlay.

This changeset replaces the 780px media query max-width value with 782px for better consistency and to fix a responsive layout issue.

Props zodiac1978, ironprogrammer, orestissam, audrasjb, monzuralam.
Fixes #58164.

[zodiac1978]

Thanks for merging!

But this was only a side effect that was found in the research. The two main point from this ticket are still not solved (maybe a wontfix, but this should be documented then):

These are the open issues:

When the modal is open, the fly out-menus are below the background overlay.

Additionally the overlay is not ending at the end of the page but earlier.

[ironprogrammer]

Regarding the first issue:

When the modal is open, the fly out-menus are below the background overlay.

The preview backdrop is z-index: 10000:

.theme-overlay .theme-backdrop {
        z-index: 10000;
}

Perhaps #adminmenuwrap could be changed from 9990 to 10001? Would this pose issues above the collapsed menu breakpoint? (At lower breakpoints, there are overrides in place.)

#adminmenuwrap {
        z-index: 10001;
}

[chaion07]

Thanks @zodiac1978 for reporting this. We reviewed this ticket during a recent bug-scrub session. Here's the summary of the discussion:

1. There are open issues in the ticket which JB as the owner of the ticket will check and punt if we can't fix it in this release.
2. Since the ticket already has a commit in this milestone, punting would potentially mean creating a follow-up ticket IIRC.
3. This can altogether be a sensible decision (would it entail) having the option to revert the changes and delay any further action. It would always make sense to get feedback and accumulated thoughts from everyone involved.

Cheers!

Props to @mukesh27 & @swissspidy

[oglekler]

Patch is not ready and RC1 is tomorrow, so I am moving this ticket into 6.4 milestone.

[oglekler]

@orestissam can you please follow the suggestions above? It looks like "it is nearly there" :)

[orestissam]

Hi @oglekler

Will provide a new patch in the following days.
Thanks

[orestissam]

Fixes issue with menu z-index and theme details backdrop height

[orestissam]

A note on reproducing the issue with the overlay not covering the whole page.
This is only visible when page content is "shorter" than the viewport height. If you 're testing against a default fresh installation, delete a few of the installed themes first so that the height of the page's content is shorter (i.e. leave only 3-4 themes installed).

Background: The .theme_backdrop div is correctly absolutely positioned and takes up the whole available space, but its relatively positioned parent (#wpbody-content) doesn't have a minimum height and follows the height of its inner content, which if short enough causes the issue with the overlay that is smaller than the screen.

[sabernhardt]

punting would potentially mean creating a follow-up ticket

We did not need to create a follow-up ticket to close this one with 6.3. The z-index issue was already reported in #41155, and that ticket probably could address the height issue too. Should we continue the discussion (and move the patch) there?

[Boniu91]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58164/58164-2.patch

Environment

• OS: Windows 10
• Web Server: Apache
• PHP: 8.1
• WordPress: 6.3-alpha-55505-src
• Browser: Chrome
• Theme: Twenty Twenty-Two

Actual Results

• Issue partially resolved with patch.

Additional Notes

• The second patch doesn't contain a fix mentioned here:

https://core.trac.wordpress.org/ticket/58164#comment:15

Supplemental Artifacts

https://core.trac.wordpress.org/attachment/ticket/58164/58164.png

[oglekler]

This ticket was discussed during a bug scrub.
@audrasjb this looks good, please take a look to consider if it is ready to be committed.

Add props: @mukesh27

[nicolefurlan]

@audrasjb with just a week until 6.4 RC1, does this seem ready for commit?

[nicolefurlan]

@audrasjb does this seem ready for committing before RC1 tomorrow?

[nicolefurlan]

This ticket was discussed in today's bug scrub.

@hellofromTonya is reviewing this. Will commit or punt depending on review.

[hellofromTonya]

I'm not sure 58164-2.patch​ is ready for commit this late in the cycle. Why?

Though there is a test report for TT2, what about other scenarios such as other themes, different submenus, and different viewports (devices)?

I also tend to agree with @sabernhardt comment:27:

We did not need to create a follow-up ticket to close this one with 6.3. The z-index issue was already reported in #41155, and that ticket probably could address the height issue too. Should we continue the discussion (and move the patch) there?

#41155 predates this ticket. Rather than solving the z-index and height issue here, might be better to move the discussion and 58164-2.patch​ to #41155.

Moving this ticket to 6.5. But if there's high confidence that the patch does indeed fix the z-index and height issues across all themes and viewport sizes, then it could be considered for 6.4, though RC1 is ~16(ish) hours.

[swissspidy]

@audrasjb Is this on your radar still for 6.5? Looks like a punt candidate given the lack of activity.

[sabernhardt]

I'm closing this ticket because it has the breakpoint correction in [56104], and any further work can be part of #41155.