Color Controls: Allow popovers to remain within smaller viewports #41929
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
aaronrobertshaw
added
[Type] Bug
|
Size Change: +9 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
This was referenced Jun 24, 2022
|
Thanks, this is definitely an improvement: popover.mp4One small adjustment I would suggest if possible – place the popover beneath the associated button so that you can always see which color you're picking:
|
|
Thanks for giving this a run @jameskoster 👍
Agreed. This is essentially what I was referring to by the following note in the PR description.
I don't think we have the means just yet to tweak the automatic positioning of Popovers. I believe there is some extra work we'll need to do to expose more of the underlying floatingUI library functionality. Given that, the aim of this PR is to get the popovers at least back into the viewport so they can be used, then improve the positioning as soon as we can. |
glendaviesnz
approved these changes
Jun 27, 2022
There was a problem hiding this comment.
This worked as advertised for me. I think it is worth merging this fix in ahead of the changes to allow better placement as it at least allows the color picker to be used again on smaller screens, and the placement to the left of the panel is still fine on larger screens.
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.
Fixes: #41894
What?
Allows color control popovers from the Inspector Controls sidebar to remain within smaller viewports.
Note: This PR only restores the visibility and usability of the color control popovers. There is some ongoing work around better positioning after the floatingUI changes. Once that settles we can improve the color popover positioning as a follow-up.
Why?
After the recent switch of popovers to use the floatingUI library (#40740) the color controls were offset to outside the sidebar. This placement and offset meant the popover was outside the viewport when the sidebar becomes a full-width overlay on smaller viewports.
How?
Toggles on the
__unstableShiftPopover prop so that if the popover falls outside the viewport it is automatically repositioned.Testing Instructions
Screenshots or screencast
Screen.Recording.2022-06-24.at.5.06.20.pm.mp4
Screen.Recording.2022-06-24.at.5.00.22.pm.mp4