Box Control: Ensure Unlink tooltip position stays within the viewport #41176
+7
−3
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
andrewserong
added
[Type] Bug
|
Size Change: +22 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for tidying up these tooltips @andrewserong 👍
The issue was introduced when Popovers were refactored to use the floatingUI library (#40740).
✅ Tooltips appear correctly positioned for LTR
✅ Tooltips appear correctly positioned for RTL
❓ There are a lot of other buttons that have tooltips that are possibly displayed in a sidebar. These will also all need updating unless we can find a more generic way to solve this positioning problem. Perhaps via a better default position on the Button component?
Some examples in my limited testing so far include:
- Block editor
- Editor more / options menu
- FontSizePicker - Custom size toggle button
- ToolsPanel - Vertical ellipsis menu buttons
- InspectorControls sidebar's close button
- Site Editor
- Editor more / options menu
- Sidebar close and more buttons
- Color palette vertical ellipsis menus
- Create color palette add button
- Block list search field's clear button
I'm out of time today but can help look further into this tomorrow.
|
Thanks for testing @aaronrobertshaw!
That's a great point, it does look like it'd be worth looking to a more generalised solution for this, so that (somehow) if the button is close to the edge of the viewport, we make sure that the tooltip doesn't go off the side. So maybe there's a fix we can look into for the Tooltip component itself, potentially, too, if the Button component doesn't wind up being the right place? I'll do a bit more digging and 🤔 too! |
My naive guess (without exploring the code yet) is that we are now setting a default position on Tooltips whereas it was automatic placement previously which might have ensured rendering within the current viewport. |
|
I'll close out this PR now — I believe I've come up with a more thorough way to fix this directly in |
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?
Ensure that when you hover over the Unlink button in BoxControl components (e.g. Padding, Border, or Border Radius controls) that the Tooltip position does not go off the edge of the viewport.
Why?
We currently aren't setting the position on the wrapping
Tooltipcomponents, so the tooltip flows over the edge of the viewport. If we set the position tobottom left, given that the Unlink button is as the right edge of the screen, this should ensure that the Tooltip is always within the viewport.How?
"bottom left"to each of theTooltipcomponents used forLink sidesbuttons in the design tools.Testing Instructions
Screenshots or screencast