Make combobox and select dropdowns float above other UI #1234
Conversation
m-akinc
requested review from
rajsite,
jattasNI,
atmgrifter00 and
msmithNI
as code owners
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
Was discussed at stand-up to explore the cost of using anchored-region for this use-case:
Anchored Region
Pros:
- Aligns with our existing pattern across controls (menu, menu-button, tooltip, is that all?)
- We aren't mixing popover implementations across components
- Has some more usage in the wild, i.e. the floating-ui library is part of FAST beta that hasn't released yet
Cons:
- Requires template forking
- Anchored region was proprietary to FAST and is deprecated, have to switch eventually.
- Anchored region probably has a much smaller net user base than floating-ui
If it's true that only a few of our controls use anchored region and two of three are already templated owned by us (tooltip and menu-button), then maybe we should be looking at the cost of migrating to floating ui for all of them
|
Edit: moved to threaded comment |
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
Moving to threaded comment:
@atmgrifter00
So, I've noticed in the Combobox Storybook Doc page that the dropdown is still clipped within its parent container. Were we not expecting this to be fixed with this change?
There was a problem hiding this comment.
looks like in chrome it pops over but firefox it's still stuck with the same storybook
There was a problem hiding this comment.
I've run into enough problems and uncertainties that I'm planning on using anchored region for the select and combobox, instead. When that is ready, I'll close this PR and open a new one.
|
Closed but created another PR to address this issue: #1244 |
## 🤨 Rationale Fixes: #606 Fixes: #1227 ## 👩💻 Implementation I initially looked at using [Floating-UI](https://floating-ui.com/), as that is what FAST 2.0 is using to solve this problem. (see #1234) However, for consistency, that would require adding support to both the Select/Combobox as well as updating the menu button, the menu item (for submenu support), and the tooltip. Each case was a significant change, and I ran into several problems, including with the action menu in the table. It turned into a 40+ file change with a lot of risk. I sunk more time into it than I wanted, and still didn't feel close to done. I'm instead taking a much more scoped approach of extending the existing pattern and using the anchored region in the Select and Combobox. This required forking the templates for both controls from FAST, so that we could add the anchored region element. ## 🧪 Testing I added some tests to cover certain expectations about the templates, e.g. attribute forwarding. I'm relying mostly on existing Chromatic tests to ensure the listbox pops up in the same place given the position options. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
Pull Request
🤨 Rationale
Fixes: #606
Fixes: #1227
👩💻 Implementation
FAST fixed this issue with their combobox and select in their master branch, but we are still on FAST 1.0. In the PR for that change, they suggest that we essentially copy their implementation. See: microsoft/fast#6452 (comment). This PR does just that. We're using the package floating-ui to float and position the dropdown of the combobox and select. Because floating-ui now manages the position/size of the dropdown, the
positionandmaxHeightproperties are no longer functional. We had only exposed/documented thepositionproperty, and we had neglected to expose it from Angular. I've removed the properties from tests, Storybook, etc.🧪 Testing
Some manual testing in Storybook. Existing unit tests pass.
✅ Checklist