Link popover does not scroll with page

[ellatrix]

Describe the bug

Introduced by #14938. Cc @aduth.
Noticed while testing #15035.

When the link popover is visible, and you scroll the page, the link popover doesn't move. That's because it have a fixed position. Previously the popover element would be positioned in the RichText wrapper element, and would be positioned relative to that.

Suggested fix: add an option to Popover to allow inline positioning with relative coordinates.

[aduth]

Suggested fix: add an option to Popover to allow inline positioning with relative coordinates.

A challenge to consider here is that Popover is implementing using portals (i.e. non-relative) in part to avoid dealing with CSS style cascade effects. Reintroducing relative positioning reintroduces this potential conflict.

[ellatrix]

@aduth Previously [the component] also used Popover, but wrapped in an extra relatively positioned div. Not sure how exactly it worked then. :)

[aduth]

@aduth Previously also used Popover, but wrapped in an extra relatively positioned div. Not sure how exactly it worked then. :)

Did GitHub eat your markup between words "Previously" and "also" ? Unsure if your comment was meant to include a reference to a specific example.

If I recall correctly, Popover was originally positioned relatively, and then refactored to be separate in the DOM when Slot/Fill was first introduced. Maybe that's the previous implementation relied upon.

Actually, just now looking at the implementation, worth pointing out that Popover does render in-place if there is no Slot context for it to render in to:

gutenberg/packages/components/src/popover/index.js

Lines 393 to 397 in c0ac81a

	// In case there is no slot context in which to render,
	// default to an in-place rendering.
	if ( getSlot && getSlot( SLOT_NAME ) ) {
	content = <Fill name={ SLOT_NAME }>{ content }</Fill>;
	}

(Unrelated: cc @jorgefilipecosta you had mentioned some issues with Popover in Widgets screen, I wonder if this might be the cause)

[jorgefilipecosta]

(Unrelated: cc @jorgefilipecosta you had mentioned some issues with Popover in Widgets screen, I wonder if this might be the cause)

Thank you for bringing this up, I think the popovers problems in the widgets screen were related to not having a working PopoverProvider there.

[ellatrix]

Did GitHub eat your markup between words "Previously" and "also" ? Unsure if your comment was meant to include a reference to a specific example.

Sorry, corrected. :) Just meant to refer to the component affected.

What I meant is that, previously the link container also used Popover, but somehow it was placed relatively to the rich text container. I'm not sure how this was done exactly.

[ellatrix]

Fixed by #17867.