👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @ironprogrammer! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

Thank you for the PR, and thank you for the excellent description in the body, it was very helpful in understanding the problem you are trying to solve here.

As a quick test, this appears to be working well for me:

As you suggest, this is a somewhat sensitive part of the structure, so it would be great to have a few more eyes and testing on this one, to see we don't regress in other parts of the elevation. A couple of times the block toolbar would get too high an elevation and sit above the top toolbar, but that appears to be fine here:

Otherwise looks great, thanks!

Thanks for checking this out and providing some additional test feedback, @jasmussen!

A couple of times the block toolbar would get too high an elevation and sit above the top toolbar...

That brings up a test scenario where it might be interpreted that this fix allows the modal to appear too high on the z-axis. For example:

Link popover shown "above" editor header bar.

As mentioned in the PR description, from a UX perspective, a modal should be on top of everything else. If this popover was adjusted to flow "under" the top header, then it may not conform to this expectation.

Further Thoughts

If consensus goes the other way on this detail, then IMHO, the actions sidebar (re: Publish button vis in #32869) might be better positioned as a sibling to the editor's header, body, and footer, rather than as a descendant of the body, as it is today. If actions should always appear on top of everything else, then it would make sense to reflect that in the DOM for better readability and easier z-axis control.

Once the actions were no longer "under" the top header, then the static pinning of the content div's z-index could remain at 20 so that it plays nicely with its header. Anyways, that's just my thinking if this solution proves insufficient, or perhaps if this is viewed as a technical debt mitigation item.

Excellent context, thank you.

As mentioned in the PR description, from a UX perspective, a modal should be on top of everything else.

Do you have any additional context for this one? I'd like to learn, because from my experience that's not necessarily a requirement for proper modal behavior, and it would also be a principle that would have consequences for many other modal interfaces in the editor, such as the block inserter, or even dropdown menus from the block toolbar. Which incidentally led me to discover this bug we have at the moment 🙈 :

My instinct would be that any dialog that is invoked from the editing canvas is on top of anything inside that editing canvas. But that you can still have a top toolbar that is elevated above the entire editing canvas and its contents.

In response to @jasmussen:

Which incidentally led me to discover this bug we have at the moment 🙈 :

Thanks for this. It makes me reconsider the initial approach.

Looking more closely at the popovers that are NOT obscured by the sidebars -- e.g. block inserter, text alignment, block settings menu -- these remain visible because they are injected into a dedicated .popover-slot that is sibling to .interface-interface-skeleton. Compare this with the link popover, whose parent .popover-slot is a child to this element.

From what I have been able to gather, the link popover is the only one that is nested inside the editor, as opposed to being pushed to the external .popover-slot.

💡 Perhaps relocating the link popover's injection point would be a better fix to this issue, making it consistent with the other popovers. Thoughts?

Considerations

• The current location of the .popover-slot placeholder div outside the editor allows these to be modalized in the literal modal design pattern sense, where they are on top of the main window.
• Popovers are derived from the modal component, so this is consistent with the "focus on this one thing before moving on" purpose of a modal.
• This doesn't modify the placement or z-axis of other existing elements, and isolates the change to just the link popover.
• Moving the link popover may alleviate some technical debt, and support more consistency in popover placement for future features. I.e. keep them out of the editor body to ensure they are always accessible.

💡 Perhaps relocating the link popover's injection point would be a better fix to this issue, making it consistent with the other popovers. Thoughts?

Excellent detective work.

There's a bit of additional context, which is that the Site Editor puts the entire editing canvas inside an iframe. This iframe enables media queries to work correctly, and simplifies a bunch of things. For links, it also enables the link selection marquee to stay visible when editing inside the link popover. Compare post editor:

Site editor in iframe:

For those reasons alone, I would love to see us port the iframe to the post editor as well. That might require the link popover to be external to the editing canvas, as you suggest.

But to be clear, though, this is a small PR, and it seems good enough to land if it fixes an important issue (though I'd love additional testing). Just good to be mindful of any side effects if it introduces any! Thanks again for the PR.

...this is a small PR, and it seems good enough to land if it fixes an important issue (though I'd love additional testing).

Thanks, @jasmussen! If this were targeted to 12.7.1, would it get additional test love?

If this were targeted to 12.7.1, would it get additional test love?

It seems like this bugfix could land at any time, so I guess it just needs an additional dev to take a look and sanity check it! 👌

@ironprogrammer are there any other modals or full page overlays at all that this could potentially interfere with? If not, I think we're good to go – I think this UI element should be above pretty much anything other than a full-page overlay and things like navbars that conceptually live on the "glass pane" in front of everything else, and it seems like you already took care of the latter.

In response to @adamziel:

...are there any other modals or full page overlays at all that this could potentially interfere with?

My own (and @jasmussen's) testing involved one or both sidebars, setting the browser width narrow enough that the content area was too small for the width of the popover, activating the popover while scrolling the editor up/down, and checking on mobile (responsive below medium).

It has been noted that this new z-index setting only comes into effect for medium screens and up, where the sidebars are assigned z-index: 90. Whereas on smaller (mobile/tablet) screens, the sidebars default to 100,000 and the content area has no assigned z-axis value. Therefore this change shouldn't impact mobile, but of course we'd love some additional testing 🙏🏻.

Just testing the PR, and while it does raise the popover above the editor's sidebars, it still seems like the popover is under the wp-admin sidebar:

This is with Fullscreen mode turned off.

Just testing the PR, and while it does raise the popover above the editor's sidebars, it still seems like the popover is under the wp-admin sidebar...

Thanks, @talldan! Yes, I think the solution for that would be to give first class modalization to the popover, mentioned here. A popover is a modal, and modals are designed to focus the user's attention on a specific task, and the control's surface should be completely visible/usable for completing the task.

I feel that the PR in its current state, which only affects a z-axis change within the editor itself, still serves a purpose. It would:

• Resolve the reported issue (with the exception of your noted sidebars external to the editor).
• Ensure that other "inline" popovers pinned to the editor are not obscured by the editor sidebars going forward.
• Apply regardless of whether the link popover ever gained "first class modal" status.
• Remain compatible if the link popover's slot were changed, or if the block editor were iframed like the site editor.

This change affects the templates page, which uses the InterfaceSkeleton

A proposed workaround over here: #39331

Cheers!

Looks like this PR created a number of other smaller issues. See #38723 and #39582 for instance. Feels like we should consider reverting this one instead of patching all of these bits one by one. We also don't really know what impact could this have on third-party plugins.

So we're stuck with this forever? 😢

@draganescu -- No, we shouldn't be. Maybe I'm old school, but IMHO a modal should never be under other UI elements -- it defeats the intent of "User, look at this and complete this task before doing anything else" (even if the modal is simply dismissed).

Many other popovers are filled outside of the editor's control surfaces (i.e. outside of the editor skeleton, as referenced here). What are your thoughts about shifting the fill location for the link popover so that it follows those examples?