Buttons block: Change position of the link popover #27408
Conversation
david-szabo97
added
[Type] Enhancement
|
Size Change: +32 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
Good catch. Looking back at the history, I think this is an unexpected consequence of other changes made to the block. Popovers by default anchor to their parent element, and the component used to be inside the wrapping The anchor ref is at least a more explicit way that would avoid that happening in future, so I think this is a good option. Looks like there are CI tests failing for some reason. Looks quite unusual, each check had the tests run twice. Might be worth rebasing and force pushing to see if that clears the failed tests. |
david-szabo97
force-pushed
the
fix/button-link-popover-position
branch
from
9a2eb5b to
174ae97
Compare
|
If I recall correctly, there was an issue (later fixed) where the link popover (when it was not centered) was appearing off of the screen, particularly on small screens. did you check whether your change would work on small screens (<600px width)? |
Yep! It works fine! Popover repositions itself if it can't position itself to the specified anchor. If it can't fit below the button then it positions itself above the button. Though we might think about anchoring the link popover to the toolbar 🤔 It would look better here on small screens: What do you think? |
|
FYI, this is the issue that I was referring to #20146 |
|
Hmm, I'm still noticing that the popover is offscreen on small screens (testing in Firefox/Chrome dev tools using the mobile presets): There's also a bit of thrashing where it looks like the popover continually tries to reposition itself. |
|
@skorasaurus @talldan Gotcha! Thanks for the links, now I understand what you were referring to. On the other hand, those are different issues totally unrelated to this change. So I don't think those should be a blocker for this PR. Since the issue linked above is affecting both the I don't see any PRs for those issues though, so I might take a look at them next week. |
david-szabo97
requested review from
vindl,
noahtallen,
mattwiebe,
Addison-Stavlo,
Copons and
jeyip
Description
Link popover in the Buttons block opens up in the bottom center relative to the Buttons block. It feels strange that it pops up so far from the button we are editing. To improve, this PR changes the position of the toolbar to be relative to the button component.
Alternatively, we could make it relative to the toolbar button (link button). So it appears right below the link toggle. Though I prefer the one above because we can see the text of the button that way.
How has this been tested?
ButtonsblockButtonsblockScreenshots
before
after
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: