Size Change: +32 B (0%)

Total Size: 1.19 MB

_{compressed-size-action}

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.

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 div. At some point the div was removed, so that was no longer the case. Then when the div was added back due to a back-compat issue, it looks like the popover was left outside of the div.

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.

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)?

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.

Related #24592, #24532.

@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 master and this PR.

I don't see any PRs for those issues though, so I might take a look at them next week.