Fix block settings dropdown positioning when scrolling in the site editor #43544
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d update the popover on scroll of that document
talldan
added
[Feature] Blocks
|
Size Change: +191 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
talldan
requested review from
ciampo and
ntsekouras
and removed request for
ajitbohra
67 tasks
ntsekouras
reviewed
Aug 24, 2022
ciampo
added
[Type] Bug
|
This is unexpected — I thought that, for the "second gen" popover, all that mattered was the position of its anchor (and therefore it would be unaffected by what happens in the Also, if the anchor and the popover are within the same document (which is the case for the settings dropdown), I also expected that |
Ah, I imagine it's a slot/fill thing. 🤔 |
|
I gave this PR a spin and noticed that:
Kapture.2022-08-24.at.11.17.28.mp4
I was hoping that some of these issues could be fixed by the different approach to
Maybe we need to tinker with the |
|
With #43617 merged, I believe we can close this PR. Feel free to re-open in case I was mistaken! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes #42725
(best reviewed using the 'Hide whitespace' option)
Why?
To describe the bug requires a bit of background.
The site editor uses an iframe for its content, and all blocks are within the iframe.
Popovers are outside the iframe in the main document. Whenever the iframe is scrolled, popovers that are anchored to blocks (like the the block toolbar) listen for the scroll event and update their position on the screen to match where the block is. You have this situation:
This bug happens because the block settings menu dropdown is a nested popover. It's anchored to another popover (the block toolbar) that isn't in an iframe, so it doesn't know to listen for scroll events.
How?
I could only think of one feasible way to solve this bug, which is to add a context provider that stores the root (reference) document for nested popovers, allowing them to also listen for scroll events.
Testing Instructions
Expected: the toolbar and settings menu should stay anchored to the block
Screenshots or screencast
Before
Kapture.2022-08-24.at.15.00.34.mp4
After
Kapture.2022-08-24.at.14.58.18.mp4