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

Fix: New block editor z-index issue. Link options are hidden at the bottom of the page #63809

Closed
wants to merge 3 commits into from

Conversation

Rishit30G
Copy link
Contributor

What?

PR is intended to fix this issue: #63795

Why?

When you are editing text in the new block editor if your text is on the bottom of the page and you are trying to add/edit a link, the options are hidden if you have some options for example seo options

How?

Added z-index: 5 to .editor-visual-editor as per the suggestion

Testing Instructions

Works as expected, the results are now no more blocked by the box below the text

Screenshots or screencast

image

Copy link

github-actions bot commented Jul 22, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Rishit30G <[email protected]>
Co-authored-by: stokesman <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: talldan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jul 22, 2024
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Rishit30G! In case you missed it, we'd love to have you join us in our Slack community.

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

Copy link
Contributor

@stokesman stokesman left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I see this change is per the suggestion in #63795. This works but the z-index isn’t required in all contexts so applying it in the component’s own styles may necessitate overriding it in other contexts.

In the earlier issue on the bug #62998, I had made this suggestion:

.edit-post-layout.has-metaboxes .edit-post-visual-editor {
    z-index: 1;
}

This way the z-index is applied by a parent context. That would go in this file: https://github.com/WordPress/gutenberg/blob/trunk/packages/edit-post/src/components/layout/style.scss

@Rishit30G
Copy link
Contributor Author

Hey @stokesman,
Thanks for sharing feedback on my PR, however on setting z-index = 1 the results are still hiding behind the metabox content:

image

On setting z-index: 99 it's working fine:

image

@stokesman stokesman linked an issue Jul 23, 2024 that may be closed by this pull request
@stokesman
Copy link
Contributor

stokesman commented Jul 23, 2024

however on setting z-index = 1 the results are still hiding behind the metabox content:

That’s a great catch. Setting the z-index to a greater value like 99 works for Yoast’s metabox however it’s unknown if other plugins use larger values. So alternatively, the metabox area could be made its own stacking context. The most explicit way I know how is to add isolation: isolate to it.

Here’s a screen capture demo:

yoast-metabox-v-isolation.mp4

Still, I suppose there is some potential issue with that if there is some plugin creating a sort of popover like UI from within the metabox area that overlays the editor canvas or other parts of the editor interface. By making the metabox area a stacking context all elements within it can’t be in front of anything the metabox area is not in front of.

So I'm not sure what the best idea is here. Perhaps @t-hamano or @talldan have a helpful thought on this.


I pushed a commit that applies the z-index per the project’s convention around z-indexes. Oh, I also made it 1 but of course we may end up changing that.

@t-hamano
Copy link
Contributor

I think the ideal approach would be to use isolation: isolate;, as we did in #62681, i.e. CSS like this:

.edit-post-layout__metaboxes {
	isolation: isolate;
}

.edit-post-layout.has-metaboxes .edit-post-visual-editor {
	z-index: z-index(".edit-post-layout.has-metaboxes .edit-post-visual-editor");
}

By making the metabox area a stacking context all elements within it can’t be in front of anything the metabox area is not in front of.

Does this mean that, for example, if we render a popover inside the metabox, any part that extends above the metabox will be clipped?

@stokesman
Copy link
Contributor

if we render a popover inside the metabox, any part that extends above the metabox will be clipped?

Yes. Though only when the element is inside the DOM tree of the metabox. If a plugin were using Popover from the components package in a default fashion it wouldn’t be a child of the metabox container so should be fine. Still, who knows what plugins are doing. Also, to say clipped is precise as long as you mean generally not visible. It would be behind the visual editor but not clipped by the metabox container.

I suppose the potential issue could be guarded against with :focus-within that puts the metabox container back in front:

.edit-post-layout__metaboxes:focus-within {
	z-index: 1;
}

Though, now I'm thinking Aki’s initial idea #62998 (comment) for resolving this may be the best. That is to make a shared ancestor of the visual editor and the metabox area a stacking context instead of the visual editor.

@t-hamano
Copy link
Contributor

Though, now I'm thinking Aki’s initial idea #62998 (comment) for resolving this may be the best. That is to make a shared ancestor of the visual editor and the metabox area a stacking context instead of the visual editor.

I tried this approach in #63939.

@t-hamano
Copy link
Contributor

Hi @Rishit30G,

This issue has been resolved in #63939. Of course, you have been added as a co-author to the props.

I would like to close this PR, but thank you for your efforts!

@t-hamano t-hamano closed this Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline popover hides behind metabox
3 participants