Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert: Fix/editor content zindex stack context #39620

Merged

Conversation

ironprogrammer
Copy link
Contributor

@ironprogrammer ironprogrammer commented Mar 21, 2022

What?

Reverts #38893, "Raise z-index of content div relative to sidebars" (commit b90f46f). This change was introduced to allow the editor's main content area to display link popover modals above the sidebars.

Also reverts a subsequent corrective change in #39331, which was opened due to the original content area z-index change (commit 2a18443).

Why?

While #38893 addressed the original issue (#38723), it introduced other downstream issues that rely on the content area appearing at a lower z-axis than the sidebars.

These issues include:

Because of the outsized impact of addressing the original minor issue, there was consensus that the z-axis change should be reverted, and addressed differently. Here are the requests, for reference:

How?

Reversion of the change restores .interface-interface-skeleton__content to z-index: 20. Also rolls back a separate and subsequent adjustment applied for visibility of the Templates "Add New" dropdown menu in #39331.

Testing Instructions

Check out PR or apply the patch to gutenberg:trunk and re-build the plugin. Make sure the plugin is activated.

Test 1: Original Issue #38723 (un-fix)

  1. On medium and up screen (tablet, laptop, desktop):
  2. Create new or edit a post.
  3. Open the List View and/or Settings sidebars.
  4. Enter test paragraph content, if needed, and highlight some text -- text on the extreme left or right of the paragraph works best to replicate the issue.
  5. Click the "link" option from the inline toolbar, and the Link popover (modal) will activate.
  6. Observe that the popover appears "under" either sidebar (⚠️ this un-fixes the original issue, Block Toolbar: Link Tooltip overlapping #38723).

Screenshots

Before Patch

Before patch
Original fix shows link popover "above" sidebars.

After Patch

After patch
Link popover appears "under" sidebars, restoring original behavior.

Regression for #32869

Regression for PR 32869
Actions panel retains stacking priority, and popover appears "under" sidebars.

Test 2: Templates "Add New" Dropdown Visibility #39331 (revert)

  1. With a block theme activated, navigate to Appearance > Editor (/wp-admin/site-editor.php).
  2. Click the WordPress icon in the upper-left corner to toggle navigation.
  3. Click Templates to open the template selector.
  4. Click the "Add New" button to activate the "Add New Template" dropdown menu.
  5. Observe that the dropdown is visible and appears "above" the template list.

Screenshots

Regression for #39331

Regression for PR 39331
"Add New" dropdown menu remains visible.

@ramonjd
Copy link
Member

ramonjd commented Mar 21, 2022

Thanks for the revert PR!

I guess #39331 will have to be reverted along with these changes as well? Would it be possible to bundle that into this PR for ease of testing?

@ironprogrammer ironprogrammer force-pushed the fix/editor-content-zindex-stack-context branch from fec80ed to 945ce7d Compare March 21, 2022 21:58
Restores previous `z-index` for editor content area.

Reverts b90f46f.
@ironprogrammer
Copy link
Contributor Author

I guess #39331 will have to be reverted along with these changes as well? Would it be possible to bundle that into this PR for ease of testing?

Hi, @ramonjd -- absolutely. I'll slurp in those revert changes as a separate commit.

@ironprogrammer ironprogrammer marked this pull request as ready for review March 21, 2022 23:35
Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Original behaviour is restored for the link and template popovers.

Screen Shot 2022-03-22 at 11 01 43 am

Screen Shot 2022-03-22 at 11 01 10 am

@mtias mtias merged commit b24f943 into WordPress:trunk Mar 22, 2022
@mtias
Copy link
Member

mtias commented Mar 22, 2022

Thanks!

@github-actions github-actions bot added this to the Gutenberg 12.9 milestone Mar 22, 2022
@ironprogrammer ironprogrammer deleted the fix/editor-content-zindex-stack-context branch March 22, 2022 18:01
jostnes pushed a commit to jostnes/gutenberg that referenced this pull request Mar 23, 2022
* Revert PR 38893

Restores previous `z-index` for editor content area.

Reverts b90f46f.

* Revert PR 39331

Reverts 2a18443.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants