feat(DropdownMenu): Add responsive menu alignment #5307
Conversation
Add deprecation notice for alignRight in DropdownMenu
src/DropdownMenu.tsx
Outdated
| DEVICE_SIZES.forEach((brkPoint) => { | ||
| const direction = align[brkPoint]; | ||
| if (direction) { | ||
| alignRight = alignRight || direction === 'left'; |
There was a problem hiding this comment.
question: i'm confused by this – we set alignRight to true if any direction is left? that seems a little odd to me
There was a problem hiding this comment.
Sorry should've left a comment here. In order to responsively align left, the dropdown-menu-right class needs to be added in addition to dropdown-menu-**-left. I reused this bool to control toggling of that class.
https://getbootstrap.com/docs/4.5/components/dropdowns/#responsive-alignment
|
wait a sec... per #5304 (comment) – should we then have some sort of check that only a single key is specified in the object? i.e. it wouldn't make sense to specify more than one align class, i think? |
|
e.g. if i'm reading the bootstrap docs correctly, the type should be more like AlignDirection
| { sm: AlignDirection }
| { md: AlignDirection }
| { lg: AlignDirection }
| { xl: AlignDirection } |
|
cc @jquense – was that what you meant by "can't specify bad combos"? |
|
Oops, was thinking multiple aligns were allowed. I'll fix that up |
|
well, are they? the upstream docs at least suggest that it makes little sense |
|
Adding the combination of classes do allow you to flip flop on diff screen sizes. But yeah prob not a practical thing. |
|
Fixed code to only handle 1 breakpoint in the |
|
I think you should be allows to specify combos together no? At least thats supported for other cases of responsive variants. |
|
@jquense upstream doesn't show it https://getbootstrap.com/docs/4.5/components/dropdowns/#responsive-alignment would a user really want to flip back and forth on direction? the way the "left" thing is set up doesn't quite enable that... e.g. if you want it go to "left" at ">= md", we'd need to also add the "align right" as the default... otherwise we'd need a slightly different/clunkier API here? e.g. we'd have to force users to specify |
#5304