-
-
Notifications
You must be signed in to change notification settings - Fork 79.1k
Add container option to our Dropdowns #24257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I was try this changes and it works fine, only the position of the dropdown is wrong on the first click. When you click again the position is ok. First click: |
Seems related to Popper.js 😟 EDIT: |
One caveat... if someone specifies a container when the dropdown is inside a collapsible navbar. Would need to have a guard in there to prevent the dropdown from being re-parented in that case. |
Good catch @tmorehouse ! 👍 |
391e107
to
7a74d82
Compare
If you use this option with dropdown inside modal, the dorpdown position is wrong when scrolling modal. |
You can call |
This happen when dropdown is inside overflow. Try open modal and dropdown. |
Not sure it's related to Bootstrap here @danijelGombac because it's Popper.js which handle the position |
Yes the problem is in Popper.js. It can't handle two overflow container if the first container is not position static. |
maybe @FezVrasta will have time to take look at this issue 😄 |
I hope. There could be also problems with tooltips and popover inside modal and table-responsive. |
I made an issue to track that : floating-ui/floating-ui#459 |
I found a solution to fix that @danijelGombac 👍 |
7a74d82
to
6cd0204
Compare
Why not apply this config option only when config.container was used? |
yep you're right seems better with that way ! EDIT : |
2295bca
to
61d1650
Compare
Just thinking ARIA and tab order. Will re-parenting to the body change the tab order of the menu items, since the menu items are no longer siblings of the menu trigger button? |
indeed. unless focus is moved into the dropdown when it's opened and then kept there, this would be quite a broken experience for kbd users. (in the example in codepen, simply add some other focusable elements to the page to see the problem). so (just going by the example posted in the starter, not tried the actual code in this PR) i'd say no to this... |
The only way would be to test the focus order (simulating tab presses) before opening/reparenting and trapping tab/shift-tab to move focus to the correct items, but that would be really tricky and probably really buggy. |
WAI-ARIA recommends focusing the first item in dropdown menus when first opened, so that screen reader users are "inside" the dropdown, and that tabbing while inside the menu would close the menu and tab to the next document focusable item. Moving between menu items while opened is only handled by cursor up/down/left/right (although this doesnt work well for form controls (especially textarea) This is what we have done in the Bootstrap-Vue implementation of Bootstrap V4 dropdowns (at the moment, as we don't support form in dropdowns at the moment) |
but these are not menus in the WAI-ARIA sense, merely generic expand/collapse dropdowns (as they can contain more than simply menu items). see https://getbootstrap.com/docs/4.0/components/dropdowns/#accessibility |
@patrickhlauke yeah, I've seen that section before, and it is why we haven't implemented forms inside dropdowns (yet), as the native Bootstrap V4 implementation is not as intuitive to some screen reader users. We are thinking of adding n option to change from role |
But back to the re-parenting discussion, I think moving the menu to somewhere else in the body will be confusing to keyboard users (due to the possibility of getting "lost" in the document when tabbing around). |
assuming you mean |
Yep, meant to say 'dialog' (had aria-haspopup on the mind) |
61d1650
to
8d97095
Compare
I updated my Codepen : https://codepen.io/Johann-S/pen/EwoPYo |
6a9e8ad
to
070144e
Compare
070144e
to
17d64f4
Compare
@Johann-S The tab key not working how it should. When tab is pressed the menu close. |
keyboard navigation fixed how? see https://codepen.io/patrickhlauke/pen/ZaQzMR ... tab to open the dropdown, then tab again - focus moved to the "foo" links. and yes as noted, once user eventually gets to the dropdown itself, tab closes the menu. i think the reparenting of the dropdown is not a good idea, as it just adds too many complications (in our current implementation) |
Yep and changing the behavior on how our Dropdowns work on TAB adds a lot of complexicity 😟 |
I added some information useful to provide a workaround for #24251 and that could possibly replace the most basic usages of this very own PR |
Would some of the Popper.js overflow modifiers fix this? https://popper.js.org/popper-documentation.html#modifiers..preventOverflow |
I think PR #24976 provides a better solution without the need for re-parenting the |
closed because #24976 is better 👍 |
Add container option to our Dropdowns
For those who wants to try : https://codepen.io/Johann-S/pen/EwoPYo
Fixes : #24251