calcite-design-system
calcite-design-system copied to clipboard
Esri
feat(dialog, modal, popover, input-date-picker, input-time-picker, sheet): support stacked component sequential closing with escape
Related Issue: #6456
Summary
Support component sequential closing with escape by having focus-trap handle the stacking order.
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.
To recap, this is where we are:
-
Cleanup modal and dialog by double checking for a single way to close both focus-trap enabled and disabled cases. That is, scrim has a click handler that does the closing, we want to make sure we don’t handle closing also with focus trap. If scim is meant to close everything maybe we also use it to close the focus trap. If not, lets keep it as is.
-
We confirmed that input-date-picker still closes on outside click, so comment regarding that is no longer relevant.
-
There is an outstanding
focus-trapPR to enhance so it will allow child elements to handle Escape key presses without causing the trap to close. We might have to wait for this to land to pick up the remaining comments.
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.
There is an https://github.com/focus-trap/focus-trap/pull/1253 to enhance so it will allow child elements to handle Escape key presses without causing the trap to close. We might have to wait for this to land to pick up the remaining comments.
We recently updated focus-trap, which now allows child elements to handle Escape key presses without closing the trap, but I noticed that dialog doesn't have logic to ignore canceled Escape key presses within focus-trap. Can you check that this is handled? Any code handling Escape like this should be changed to go through focus-trap, similar to this example.
Could you also coordinate input-date-picker changes w/ @anveshmekala? Additional coverage was added all date-picker-related test suites (see https://github.com/Esri/calcite-design-system/pull/8402/files).
Could you also coordinate
input-date-pickerchanges w/ @anveshmekala? Additional coverage was added all date-picker-related test suites (see #8402 (files)).
Checked with him and there are no conflicts so far.
This is the gist of the test @DitwanP, @geospatialem.
Whenever a focus-trapping component opens, it traps focus and gets added to the stack. Pressing escape should deactivate each returning focus to the next component in the stack.
I recommend setting up a stack of all focus-trap components as open, mimicking the setup in the e2e, but have 2 separate stacks with popover containing either input-time-picker or input-date-picker, as you can't have both open at the same time.
Click outside also needs to be tested. It's usually set to false. For input-date-picker and input-time-picker closing with click outside is delegated to popover: https://github.com/Esri/calcite-design-system/pull/9231/files#diff-42c23e5a429472ae2ecb636c2ba383162681d40c5ff70c3f89be55d376052e32R1002 So whatever the expected behavior for popover is in this case.