feat: replace anchored region for floating UI menu item

[chrisdholt]

Pull Request

📖 Description

This PR serves to initiate the migration of Menu Item anchored positioning to use floating-ui/dom. There are a few ways we can look at approaching this.

1. The initial work here (what is being proposed) is to start with a fairly minimal exposed API surface as it relates to floating-ui/dom. Floating UI provides a significant amount of configuration, but with each applied portion of middleware, etc, we likely add unshakeable code. The menu item provides a public function for setting the submenu position. This would provide the ability to configure the implementation by extending the component. Out of the box we have submenu positioning with a very familiar API as it relates to the previous implementation but with a more intuitive DOM structure. If you need more, you can extend the component and override the setSubmenuPosition method.

2. An alternative here could be to create a mixin which exposes either a full or partial configuration of Floating UI. This provides a more direct API to interact with, but also exposes a set of API's which we may want to move away from when native anchored positioning comes into play. Exposing so much of the floating-ui API as part of the baseline feature set of Foundation seems like overkill at this time. While it may be a convenience for some who want that full customization out of the box, in the most basic menu scenarios (without submenus) it would be extremely heavy. With that said, we could add a mixin and not bake it into the primary exported template, instead providing examples on how to implement and customize. With that said, again this feels like more of a convenience to a smaller subset of folks and I'm not sure there's a benefit to exporting the a mixin we don't intend to use.

3. Rather than provide a public function to override, we could make this pluggable and enable some method of passing the configuration and middleware to the component optionally.

One additional consideration while we're here is that if I don't need a menu item with submenus I'm still getting this code. In the case of a very simple menu, does it make sense to break out the submenu behavior and enable that to be inserted in a way that could support tree shaking? Alternatively, this should provide a reduction in complexity (no dependent components) and weight overall - do we mind if you get this for free?

🎫 Issues

addresses part of #6185

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

• [x] I have included a change request file using $ yarn change
• [ ] I have added tests for my changes.
• [x] I have tested my changes.
• [ ] I have updated the project documentation to reflect my changes.
• [ ] I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

• [ ] I have added a new component
• [x] I have modified an existing component

⏭ Next Steps

[scomea]

Are we able to keep the submenu in the viewport? ie. the thing that we fix in https://github.com/microsoft/fast/pull/6260

[chrisdholt]

Are we able to keep the submenu in the viewport? ie. the thing that we fix in #6260

Taking a deeper look here - it seems that it both "is" and "is not" doing this.

[scomea]

The thing we're trying to fix in v1 is allowing the submenu to overlap the menu (anchor) in order to remain in view:

[chrisdholt]

The thing we're trying to fix in v1 is allowing the submenu to overlap the menu (anchor) in order to remain in view:

Correct - which, as I saw with the comment above is that it's doing that overlap on the second menu item when colliding, but not on the first item w/ submenus. It's odd because collision is happening with both; you should be able to repro it fairly simply by resizing so the second menu is in view but the third would collide. In that scenario it is rightly resetting the 3rd submenu level. Oddly, it's not treating the first level submenu the same. Here's the same example above with a somewhat wider screen:

[scomea]

The second submenu is correctly flipping to the side of its parent submenu with the most room, and does overlap the root menu, but it doesn't overlap with its parent submenu and never needs to. That's the behavior I think is missing - where the flyout will overlap the immediate parent element (its anchor) in order to remain in the viewport as much as possible.

[chrisdholt]

The second submenu is correctly flipping to the side of its parent submenu with the most room, and does overlap the root menu, but it doesn't overlap with its parent submenu and never needs to. That's the behavior I think is missing - where the flyout will overlap the immediate parent element (its anchor) in order to remain in the viewport as much as possible.

Ah - I see that as well now. I'll look to create a repro and see if I need to open a bug or feature request for this. An alternative is that we could create custom middleware for this (@kingoftac mentioned other opportunities as well). I do wonder though if this is a common enough scenario where it should "just work".

[scomea]

Odd z-index issue that only shows up when keyboarding, but ok with mouse.

[chrisdholt]

Odd z-index issue that only shows up when keyboarding, but ok with mouse.

I'll take a look, likely an issue w/ the test CSS...

[scomea]

Anchored region while resizing with menus open w. screen lock and auto-update on. We currently have auto-update off, turning-it on worsens perf but updates menu pos as the viewport resizes as the floating ui version does:

Same scenario with this floating ui:

Anchored region looks to be significantly better perf, like half the scripting time. Just a rough comparison.

[chrisdholt]

Anchored region while resizing with menus open w. screen lock and auto-update on. We currently have auto-update off, turning-it on worsens perf but updates menu pos as the viewport resizes as the floating ui version does:

Same scenario with this floating ui:

Anchored region looks to be significantly better perf, like half the scripting time. Just a rough comparison.

Totally down to have you pick up something like this and come up with an alternative.

Ultimately where we need to strike a balance:

1. Move towards aligning with emerging standards such as CSS Anchored positioning, popup attribute, etc. This means an approach which doesn't add DOM to solve the problem as neither of the above rely on it.
2. Performance - We are a perf conscious team so the above certainly raises eyebrows.

I'm open to alternatives to @floating-ui here, but I don't think we can move towards emerging standards with Anchored Region as it is because it's so prescriptive in its approach and implementation. The additional DOM is a hurdle here for me in terms of the API, the composition model, and as much of a separation of concerns as we can have.

So - I'm open here. I'm not married to @floating-ui as a package and a solution here (and I'm not seeing a way outside writing middleware or opening a feature request to overlay the reference element during collision), but I don't think we can wait another major version to position ourselves for what's coming, and I don't think that Anchored Region is currently setup to get us closer to that due to how it's currently constructed. I think @EisenbergEffect and I are on a similar page that these major version breaking changes are significant enough that we'd really like to avoid another major version for a year if we can. Given that, would you want to look at either:

1. improving on my implementation w/ @floating-ui (it could just be poor implementaiton 😄 )?
2. creating our own mixin that handles the baseline scenarios we care about
3. something else?

[scomea]

1. Move towards aligning with emerging standards such as CSS Anchored positioning, popup attribute, etc. This means an approach which doesn't add DOM to solve the problem as neither of the above rely on it.

So in my mind one of the "features" of shadow dom is that it can hide DOM based constructs within it that are part of the mechanism that makes a component work and not part of the public api. I'm not sure what the practical issue ends up being here. When the new platform tech becomes available we implement it in the least breaking way possible against our existing components, and then cleanup any breaking irregularities on the next major version.

Note that the extra DOM wrapper in anchored region is how it completely stays out of the business of figuring out padding and margins as it does its thing and lets the internal elements use CSS to add margins in a straightforward way. JS that queries styles is expensive perf wise.

I don't think the perf issue with floating-ui has to do with your implementation, its just slow compared to anchored-region which was very much optimized for runtime performance and to address concerns about how much time was being spent in getBoundingClientRect calls.

[KingOfTac]

One of the walls I have run into with floating-ui is it doesn't seem to have a concept of inset positioning which I think is the root cause of the issue above where a nested menu won't overlap its immediate parent in order to stay within view.

It's not very straightforward to tell the floating element to flip over the edge of the reference element instead of the entire reference element, which is one area that anchored-region excels in my opinion.

[KingOfTac]

Since AnchoredRegion is better on performance as well as having some additional options for positioning, would it be worth it to migrate AnchoredRegion's declarative API to something like a HTMLDirective?

For a Directive something like this comes to mind:

const template = html`
  <button id="tooltip-anchor"></button>
  <!-- the anchor could either be an ElementRef, or an IdRef, whatever is best for performance -->
  <tooltip  ${position({ anchor: 'tooltip-anchor', inset: true, autoUpdate: true })}></tooltip>
`;

Basically turning AnchoredRegion into a HTMLDirective and its attributes into configuration options for the directive. Something like this could easily be transitioned to the native CSS anchored positioning when that becomes available.

Something like this could also be used as a polyfill for the native behavior if also paired with an ElementStyles Behavior. The styles behavior could conditionally attach the styles for anchored positioning if the browser supports it and the Directive could skip running any js positioning in that case.

[chrisdholt]

@radium-v is investigating some additional approaches given the feedback that Floating UI doesn't solve one key scenario for us. Closing this for now.