Popover: polish component after recent refactor

[ciampo]

The recent refactor of the Popover component to use floating ui has caused a few regressions, and still feels partially incomplete.

Known regressions (either issues, or PRs containing a fix):

• Fix customizer block toolbar after popover refactor #41312
• CustomGradientPicker: ColorPicker popover is glitchy (size and position) #41353
• Unable to select colors on small screens #41894
• Tooltip: Fix positioning by anchoring to child element #41268
• LinkControl UI goes off screen #42003
• Popover: pass missing anchor ref to the getAnchorRect callback prop #42076
• Popover: expandOnMobile styling is broken #42631
• Popover arrow not working properly in the latest versions #42820
• Popover: make sure that ownerDocument is always defined #42886
• Popover: anchor correctly to parent node when no explicit anchor is passed #42971
• ColorPalette, GradientPicker: fix color picker popover positioning #42989
• Fix block toolbar offset in site editor when toggling sidebars #43172
• Popover: fix and improve opening animation, use framer motion #43186
• Fix popover glitch that results in incorrect toolbar positioning in site editor #43267
• Popover: make sure offset middleware always applies the latest frame offset values #43329
• Site editor: do not show scrollbar when toolbar overflows the editor wrapper #43332
• Dropdown: anchor popover to the dropdown wrapper (instead of the toggle) #43377
• Fix legacy widget form positioning in customizer #43297
• Block Toolbar Regression - Toolbar obstructs selected blocks. #41575 : related PRs:
  • Popover: improve iframe offset computation #42417
  • Ensure block toolbar doesn't overlap block by modifying forcePosition and shift popover props #42887
  • WIP: Try to fix Popover obstructs blocks #43162
  • Block Popover: use placement instead of position, move to useBlockToolbarPopoverProps hook #44323
• Site editor: Block settings dropdown remains in place when scrolling #42725
  • Fix block settings dropdown positioning when scrolling in the site editor #43544
  • Popover: enable auto-updating every animation frame #43617
• Block toolbar positioned under block in the site editor #43541
  • Refactor popover to use separate flip and resize props instead of forcePosition #43546
  • Fix site editor toolbar when block is bigger than viewport #43548
• Popover: make more reactive to prop changes, avoid unnecessary updates #43335
• User mention suggestions can't be exited from within a list block #43063
• Padding visualizers do not work properly in the Site Editor #41823
  • Site editor: fix padding and margin visualizer positioning #43971
• Customize widgets: Link Popover not visible in full (Entire toolbar not visible) #41739
  • Fix: Fix popover stacking in the customize widgets editor #44282
• Mover control: Block toolbar no longer sticks to block that's being moved until you scroll #44220
  • Block Toolbar: update position when moving blocks #44301
• Popover: fix limitShift logic by adding iframe offset correctly (and a custom shift limiter) #42950
• Block toolbar sometimes positioned incorrectly after dragging and dropping a block #44281
• Popover component overlay not fully visible in Customizer #43678

Improvements wish list:

• Improve code readability by adding more inline comments (Popover: tidy up code 🧹 , add more comments #42944)
• Improve Storybook examples (Popover: rewrite Storybook examples using controls #42903)
• Popover: rework props around shift, filp and resize behavior #43191
  • Refactor popover to use separate flip and resize props instead of forcePosition #43546
  • Popover: stabilize __unstableShift prop to shift #43845
• There are currently many ways of passing an anchor, we should settle on a few, refactor all components and mark the other legacy approaches as deprecated
  • Popover: add anchor prop #43691
• Legacy position prop:
  • Popover: assess all position to placement conversions, add unit tests #44339
    • Popover: refine position-to-placement conversion logic, add tests #44377
  • Popover: refactor all usages of the legacy position prop to the placement prop #44401
  • Popover: deprecate the position prop #44386
• We should untangle the different onClose-like callbacks and unify them to just one callback (link)
• We should refactor all usages of the Popover using CSS margins and try to refactor them to use the offset prop instead
• Similarly, we should move away from the two other __unstable props:
  • __unstableObserveElement (included in Popover: enable auto-updating every animation frame #43617)
  • __unstableSlotName
• Add a popover variant prop and refactor popovers to use it, deprecate isAlternate #45137 (see convo)
• Find a better way to make widget editor popovers use position: fixed. (see Fix popover stacking in the customize widgets editor #44282 for context)
• Support for RTL opening animations
• In the context of all these changes, we could:
  • rewrite the component to TypeScript and update the README
    • Popover: refactor to TypeScript #43823
    • Popover: convert unit tests to TypeScript and modern RTL assertions #44373
  • rewrite the component's styles to use Emotion
  • rewrite the current unit tests with modern testing library features
  • Add more tests (both via RTL and via experimental viz reg)

[renatho]

Hey! I imagine it's probably related to what I'm facing with the Popover arrow.

Using WP 6.0 with Gutenberg plugin deactivated (working):

With Gutenberg (trunk) plugin activated (not working):

[ciampo]

Hey! I imagine it's probably related to what I'm facing with the Popover arrow.

Hey @renatho , it could be related — could you open a separate issue for this regression (where you can add more details and we can discuss further), so that we keep this issue as a master tracking issue? Thank you!

[renatho]

Hey! I imagine it's probably related to what I'm facing with the Popover arrow.

Hey @renatho , it could be related — could you open a separate issue for this regression (where you can add more details and we can discuss further), so that we keep this issue as a master tracking issue? Thank you!

Sure! Here is the issue: #42820

[talldan]

I've added #42725 to the list. Hope that's ok!

[ciampo]

I've added #42725 to the list. Hope that's ok!

Of course it is, thank you!

I must flag that I currently don't really have the capacity to work on this. I can try to squeeze some time and help, but we definitely need to understand who is going to work on these fixes

[talldan]

The popover's offset doesn't update when the block/pattern inserter is closed (#42417 (comment)) (needs to be confirmed if the regression is caused by the refactor to floating UI)

I think I know what's happening here. There's no 'resize' listener for frameOffset, so the value isn't updated. It looks like it works ok when opening the sidebar, but this is actually the 'shift' middleware being triggered by the block's position changing.

The bug seems to specifically happen when inserting a new block and closing the sidebar, and I think that happens because the inserted block's toolbar is newly mounted, and a fresh frameOffset is calculated while the sidebar is open. Then, when closing the sidebar the frameOffset still retains the incorrect value.

I can work on a fix for this (edit: fix in #43172).

Interestingly, I also just noticed this in the floating-ui docs:
https://floating-ui.com/docs/react-dom#variables-inside-middleware-functions

It suggests that any variable accessed through a closure by middleware functions needs to be a ref. That would also apply to frameOffset.

[tellthemachines]

👋 I just created #43541 for what seems to be a related issue; may be related to the recently merged #42887.

[talldan]

There are a couple of other issues here in the widgets customizer - Popover component overlay not fully visible in Customizer