chore(docs): Update ModalWithDropdown example #12061
Conversation
rebeccaalpert
force-pushed
the
dropdown-focus
branch
from
1c94720 to
bd0895f
Compare
|
Preview: https://pf-react-pr-12061.surge.sh A11y report: https://pf-react-pr-12061-a11y.surge.sh |
rebeccaalpert
requested review from
a team,
dlabaj and
nicolethoen
and removed request for
a team
Example was using out-of-date PatternFly syntax from a prior version. It should now show how to properly add a dropdown to a modal that allows for keyboard accessibility.
rebeccaalpert
force-pushed
the
dropdown-focus
branch
from
bd0895f to
73868fb
Compare
There was a problem hiding this comment.
This looks good to me, some comments below that would probably be worth discussing/implementing, just because I believe consumers have raised issue with menus inside Modal's causing a scrollbar so it'd be good for us to convey how to avoid that with this example update.
| To present a menu of actions or links to a user, you can add a [dropdown](/components/menus/dropdown) to a modal. | ||
|
|
||
| To allow the dropdown to visually break out of the modal container, set the `menuAppendTo` property to “parent”. Handle the modal’s closing behavior by listening to the `onEscapePress` callback on the `<Modal>` component. This allows the "escape" key to collapse the dropdown without closing the entire modal. | ||
| To allow the dropdown to visually break out of the modal container, set the `popperProps appendTo` property to one of the parent content items in the modal. Otherwise you can use `inline` to allow it to scroll within the modal itself. Using the Dropdown's default append location will interfere with keyboard accessibility due to the modal's built-in focus trap. Handle the modal’s closing behavior by listening to the `onEscapePress` callback on the `<Modal>` component. This allows the "escape" key to collapse the dropdown without closing the entire modal. |
There was a problem hiding this comment.
I think the updates here look good for how we have the example currently setup, but it'd be worth mentioning that appending to the .pf-v6-l-bullseye element could be used for dropdowns/selects that have a lot of items that would cause that Modal scrollbar, if that isn't the desired result. Appending to the bullseye should allow the menu to expand visually outside the Modal (no scrollbar created in the Modal itself), and still allow focusing the content within that menu via keyboard.
| if (typeof document !== 'undefined' && document.getElementById) { | ||
| return document.getElementById('modal-box-body-with-dropdown'); |
There was a problem hiding this comment.
Just as a relation to my above comment, updating these lines to:
| if (typeof document !== 'undefined' && document.getElementById) { | |
| return document.getElementById('modal-box-body-with-dropdown'); | |
| if (typeof document !== 'undefined' && document.querySelector) { | |
| return document.querySelector('.pf-v6-l-bullseye'); |
Should result in behavior consumers might expect/want (the menu not creating a Modal scrollbar).
Going this far, though, maybe it'd be useful to add props within the ModalContent component to allow adding a unique ID to the FocusTrap wrapper that gets the bullseye class applied.
There was a problem hiding this comment.
I don't mind - pushing this since it's more specific.
rebeccaalpert
force-pushed
the
dropdown-focus
branch
from
43558a8 to
bca47fb
Compare
| /** | ||
| * This test was not generated | ||
| */ | ||
| test('Focus trap can have an id added', () => { |
There was a problem hiding this comment.
One last thing, if we can let's move this to a separate test file outside the Generated directory.
If you'd want to also add a snapshot test to that new file, we could probably delete the Generated test directory altogether
3 participants
Example was using out-of-date PatternFly syntax from a prior version. It should now show how to properly add a dropdown to a modal that allows for keyboard accessibility.
What: Closes #11847
Additional issues: