Fix misplaced Popover when using Floating UI (#566)

[atmelmicro]

Closes #566

[mbohal]

👍

[bedrich-schindler]

In the end, after Monday's discussion, I do not see this solution satisfying.

Floating UI is just example we use just because Floating UI is currently trending library that solves positioning. But we are open to every implementation user decides to use. This means that we are unable to determine attributes user would need to pass, position, x, y are attributes that are necessary for our example but might not be for production use.

I want @adamkudrna to decide this together with us. I would personally suggest to implement something like style. I would call it e.g. placementStyle.

placementStyle could be either just the way how to bypass styles (style={placementStyle}) or it can have black or white list or style attributes to pass.

I know that React 19 will get rid of forwardRef, but I see this concept of forward-ref (respectively forward-styles) useful as we can control it for the component we know it use useful.

This is my proposal.

I can also imagine that we would require to pass either placement or placementStyle, so that only one can be set at the time. But this is just question arising in my head.

[mbohal]

But we are open to every implementation user decides to use.

I agree with @bedrich-schindler that we do not want to embrace Floating UI to closely.

I think the question is if some other placement implementation is likely to express position differently then x, y and position. I don't feel versed enough in CSS to decide this. I would think it less likely, but I'm don't really know.

I can also imagine that we would require to pass either placement or placementStyle, so that only one can be set at the time. But this is just question arising in my head.

So you mean to have one new prop:

placement: PropTypes.shape({
    /**
   * This sets the CSS property `position` of the popover. This is reserved for use with Floating UI.
   */
  position: PropTypes.string,
  /**
   * This sets the CSS property `top` of the popover. This is reserved for use with Floating UI.
   */
  x: PropTypes.string,
  /**
   * This sets the CSS property `left` of the popover. This is reserved for use with Floating UI.
   */
  y: PropTypes.string,
})

so we can then easily switch between placement formats?

[adamkudrna]

• I can imagine positioning solved via CSS transforms (translateX, translateY properties). I think the new drag-and-drop library we use does it this way as it is more performant and fluent.
• Can we have something like adapters for supported positioning libraries? We would provide the Floating UI adapter and anyone could plug-in their own.

[mbohal]

Can we have something like adapters for supported positioning libraries? We would provide the Floating UI adapter and anyone could plug-in their own.

So something like:

getPlacement: PropTypes.func,

where the signature of Floating UI implementation would be:

getPlacement(x: number, y: number, position: string): { x: number, y: number, position: string }

with the return value being the style HTML attribute value?
I like that.

[mbohal]

On call we agreed that the best solution is from @bedrich-schindler

placementStyle could be either just the way how to bypass styles (style={placementStyle}) or it can have black or white list or style attributes to pass.

We would like to filter the style attributes to allow all properties that are neede for:

• absolute positioning
• CSS transform

We need to reflect this in the docs.

[adamkudrna]

In case a general positioning white-list of relevant CSS properties is needed, this is what I can think of:

position
inset
inset-inline-start
inset-inline-end
inset-block-start
inset-block-end
top
right
bottom
left
translate
transform-origin

Notes (could be mentioned in the docs):

• ⚠️ inset is a shorthand for top right bottom left, not for inset-* properties.
• As opposed to top right bottom left and the inset shorthand, inset-* properties are writing-direction aware.

https://developer.mozilla.org/en-US/docs/Web/CSS/inset

I am leaving out these:

• inset-inline, inset-block: if a shorthand is needed, I would go with inset (although it's not writing-direction aware).
• transform: it allows to set any transforms including rotation and distortion, not only x/y(/z) translation. That seems to be too powerful to me.

[mbohal]

I think we should reword:

This prop is filtered and should only …

to:

This prop should only …

The code does not filter anything. It merely shows warning and error when in dev mode. I find this sufficient, but the docs should match the code.

[mbohal]

In dev mode the allowed styles are already handled by propType validation. I dont think we need this explicit check at all.

If we do, the errors in console caused by bject.keys(placementStyle) when placementStyle === null need to be fixed,