Skip to content
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

Add container option to our Dropdowns #24257

Closed
wants to merge 4 commits into from

Conversation

Johann-S
Copy link
Member

@Johann-S Johann-S commented Oct 5, 2017

Add container option to our Dropdowns

For those who wants to try : https://codepen.io/Johann-S/pen/EwoPYo

Fixes : #24251

@danijelGombac
Copy link

danijelGombac commented Oct 5, 2017

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: translate3d(1625px, 406px, 0px)
Second click: translate3d(1642px, 406px, 0px)

@Johann-S
Copy link
Member Author

Johann-S commented Oct 5, 2017

Seems related to Popper.js 😟
/CC @FezVrasta

EDIT:
I'm able to reproduce what's described by @danijelGombac on this CodePen : https://codepen.io/Johann-S/pen/WZdrKj

@tmorehouse
Copy link
Contributor

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.

@Johann-S
Copy link
Member Author

Johann-S commented Oct 5, 2017

Good catch @tmorehouse ! 👍
I'll enable that feature only for dropdowns not in a navbar

@Johann-S Johann-S force-pushed the v4-dev-johann-dropdown-container branch from 391e107 to 7a74d82 Compare October 5, 2017 13:39
@danijelGombac
Copy link

If you use this option with dropdown inside modal, the dorpdown position is wrong when scrolling modal.

@Johann-S
Copy link
Member Author

You can call update of our Dropdown plugin it will update the position of your Dropdown 👍

@danijelGombac
Copy link

This happen when dropdown is inside overflow. Try open modal and dropdown.

https://codepen.io/danijelGombac/pen/OxZxZM

@Johann-S
Copy link
Member Author

Not sure it's related to Bootstrap here @danijelGombac because it's Popper.js which handle the position

@danijelGombac
Copy link

Yes the problem is in Popper.js. It can't handle two overflow container if the first container is not position static.

https://codepen.io/danijelGombac/pen/OxZxZM

@Johann-S
Copy link
Member Author

maybe @FezVrasta will have time to take look at this issue 😄

@danijelGombac
Copy link

I hope. There could be also problems with tooltips and popover inside modal and table-responsive.

@Johann-S
Copy link
Member Author

I made an issue to track that : floating-ui/floating-ui#459

@Johann-S
Copy link
Member Author

I found a solution to fix that @danijelGombac 👍

@Johann-S Johann-S force-pushed the v4-dev-johann-dropdown-container branch from 7a74d82 to 6cd0204 Compare October 16, 2017 12:12
@danijelGombac
Copy link

danijelGombac commented Oct 16, 2017

@Johann-S

Why not apply this config option only when config.container was used?

@Johann-S
Copy link
Member Author

Johann-S commented Oct 16, 2017

yep you're right seems better with that way !

EDIT :
done @danijelGombac 👍

@Johann-S Johann-S force-pushed the v4-dev-johann-dropdown-container branch 2 times, most recently from 2295bca to 61d1650 Compare October 20, 2017 07:53
@tmorehouse
Copy link
Contributor

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?

@patrickhlauke
Copy link
Member

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...

@tmorehouse
Copy link
Contributor

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.

@tmorehouse
Copy link
Contributor

tmorehouse commented Nov 1, 2017

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)

@patrickhlauke
Copy link
Member

WAI-ARIA recommends...

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

@tmorehouse
Copy link
Contributor

@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 menu to role popup when the dropdown contains form elements (other than standard acceptable menu items), and change from keyboard navigation (up/down) to pure tab only navigation. Although this switch might be confusing to keyboard users (but not to screen-reader users).

@tmorehouse
Copy link
Contributor

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).

@patrickhlauke
Copy link
Member

We are thinking of adding n option to change from role menu to role popup

assuming you mean dialog, that was actually something i considered not long ago...making the trigger elements aria-haspopup="dialog" and treating the dropdown itself as a form of non-modal dialog (including managing the focus to remain inside it, possibly)

@tmorehouse
Copy link
Contributor

tmorehouse commented Nov 1, 2017

Yep, meant to say 'dialog' (had aria-haspopup on the mind)

@Johann-S Johann-S force-pushed the v4-dev-johann-dropdown-container branch from 61d1650 to 8d97095 Compare November 2, 2017 09:30
@Johann-S
Copy link
Member Author

Johann-S commented Nov 2, 2017

I updated my Codepen : https://codepen.io/Johann-S/pen/EwoPYo
with the keyboard navigation fixed 👍

@Johann-S Johann-S force-pushed the v4-dev-johann-dropdown-container branch 2 times, most recently from 6a9e8ad to 070144e Compare November 2, 2017 11:28
@Johann-S Johann-S force-pushed the v4-dev-johann-dropdown-container branch from 070144e to 17d64f4 Compare November 2, 2017 11:33
@danijelGombac
Copy link

@Johann-S The tab key not working how it should. When tab is pressed the menu close.

@patrickhlauke
Copy link
Member

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)

@Johann-S
Copy link
Member Author

Johann-S commented Nov 2, 2017

Yep and changing the behavior on how our Dropdowns work on TAB adds a lot of complexicity 😟
So we are stuck for this issue #24251

@FezVrasta
Copy link
Contributor

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

@tmorehouse
Copy link
Contributor

Would some of the Popper.js overflow modifiers fix this? https://popper.js.org/popper-documentation.html#modifiers..preventOverflow

@tmorehouse
Copy link
Contributor

I think PR #24976 provides a better solution without the need for re-parenting the .dropdown-menu, taking @FezVrasta 's idea (position: static) suggestion, and adding in Popper's boundariesElement modifier.

@Johann-S
Copy link
Member Author

closed because #24976 is better 👍

@Johann-S Johann-S closed this Dec 11, 2017
@Johann-S Johann-S deleted the v4-dev-johann-dropdown-container branch December 11, 2017 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants