[Select] Add ability to position as popper #1853
Conversation
|
Does this also contain a fix for non-modal mode? |
There was a problem hiding this comment.
@benoitgrelard This looks great! These are the exact CSS vars I need to constrain the height of my dropdown. What more is remaining on this PR? I see it's still a draft. Is there any way I can help out? (tests needed? etc?)
|
@Andrii-Antoniuk, it might be possible in the new positioning mode now, but will be in another PR. We'll prioritise releasing this change for now. |
packages/react/select/src/Select.tsx
Outdated
| */ | ||
| onPointerDownOutside?: DismissableLayerProps['onPointerDownOutside']; | ||
|
|
||
| position?: 'outside' | 'above'; |
There was a problem hiding this comment.
I'm not sold on above, it feels too similar to the other popper settings and possibly be confused with side="top".
Quick thought?
positioning: 'outside' | 'integrated'
There was a problem hiding this comment.
I'm not sold on integrated. I thought about this quite a lot, but it's quite hard to find a balance between:
- something that describes it opens outside like a popper, but can't be called "dropdown", "bottom", "below", etc as we don't know which direction it will open (hence why I went with "outside")
- something that describes it opens "on top" (felt too similar to "top") so went with "above"
Maybe "layered"? What were your thoughts for "integrated"?
There was a problem hiding this comment.
Haha cheers for chiming in @atomiks!
It's tough! "over" also could be confused with "above"/"top" although it does carry a bit more meaning.
What do you think @andy-hook?
There was a problem hiding this comment.
I was thinking integrated because the content is "integrated" with the trigger and has strong opinions on the position and layout.
I do like over the most though, it adds a bit more meaning (to my mental model at least)
I'd be happy to move forward with that.
There was a problem hiding this comment.
Is dropdown too vague? <Select dropdown />
There was a problem hiding this comment.
Maybe asDropdown? That might not be a bad shout as the way most people would ask for this specific feature was: "How do I make the Select position like the DropdownMenu one?"
There was a problem hiding this comment.
My problem with asXX is that we've established it as a convention for modifying the underlying node (asChild), we also have a DropdownMenu primitive which could be confusing? I think our naming should have relation to other positioning type props e.g. hideWhenDetached, avoidCollisions.
There was a problem hiding this comment.
Yep, I agree with the asXX sentiment there. We could consider popper. It's inline with modal on Dialog. Component vs Behaviour kinda thing.
Oooooor... position="dropdown" | "overlay"? 😬
There was a problem hiding this comment.
@andy-hook have done a little bit more discussion on this and decided to go with:
position?: 'item-aligned' | 'popper';with 'item-aligned' being the default still.
No it hasn't, if you look at each commit you should be able to see it's just moving stuff around. |
|
@andy-hook I've just realised also that I made Do you think it's worth adding to the public API? Or should I omit it from the other primitives too? |
We should omit it until needed |
Branches in `SelectPopperOutside` and `SelectPopperAbove`
benoitgrelard
force-pushed
the
select-popper
branch
from
bf6f79e
to
5c8cc4f
Compare
|
When do you have a release date? :) Thanks for your work! ❤️ |
|
It should be released tomorrow if all goes to plan! |
Fixes #1247