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

[Modal] Disable Focus Trap does not work if assigned later #11527

Closed
2 of 6 tasks
minhiu opened this issue Feb 12, 2025 · 7 comments
Closed
2 of 6 tasks

[Modal] Disable Focus Trap does not work if assigned later #11527

minhiu opened this issue Feb 12, 2025 · 7 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Business/Community Analyst Issues logged by ArcGIS Business/Community Analyst team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 5 A few days of work, definitely requires updates to tests. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality regression Issues that are caused by changes in a release, but were working before that.

Comments

@minhiu
Copy link

minhiu commented Feb 12, 2025

Check existing issues

Actual Behavior

If I assign focusTrapDisabled to a modal after it was created, the focus trap is not disabled.

Expected Behavior

The focus trap should be disabled anytime afterfocusTrapDisabled is assigned to a modal

Reproduction Sample

https://codepen.io/Hieu-Pham-the-animator/pen/jENMBXy?editors=1111

Reproduction Steps

  1. Create a Calcite Modal
  2. Assign the focusTrapDisabled property after the modal is created
  3. Notice that the focus trap is not disabled

Note:

  • If focusTrapDisabled was assigned during Modal creation, it worked correctly
  • allowOutsideClick in focusTrapOptions does not work for Input. We still cannot click on any input despite setting this to true.

Reproduction Version

https://codepen.io/Hieu-Pham-the-animator/pen/jENMBXy?editors=1111

Relevant Info

Calcite Version: 3.0.0

Regression?

3.0.0-next.119

Priority impact

impact - p1 - need for current milestone

Impact

In BA Web App, we need to disable the focus trap if users click outside of a Modal.

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/calcite-ui-icons
  • @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Business/Community Analyst

@minhiu minhiu added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Feb 12, 2025
@github-actions github-actions bot added ArcGIS Business/Community Analyst Issues logged by ArcGIS Business/Community Analyst team members. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone labels Feb 12, 2025
@jcfranco
Copy link
Member

@minhiu Could you share more about your use case? From the looks of it, you don't need focus trapping if you want to allow interaction outside of the modal. If this is the case, you need to set focusTrapDisabled prior to opening the modal.

@jcfranco jcfranco added the need more info Issues that are missing information and/or a clear, actionable description. label Feb 13, 2025
@minhiu
Copy link
Author

minhiu commented Feb 13, 2025

@jcfranco

Our use case: We would like to optimize the app for both mouse and keyboard users. By default, all of our dialogs are focusTrapDisabled: false, to make sure all keyboard users will be trapped inside that dialog. For mouse users, every time they click outside of the dialog, we will turn focusTrapDisabled: true to let them interact freely outside of the dialog.

@jcfranco
Copy link
Member

Interesting. Do you restore the trap if a user starts tabbing?

@minhiu
Copy link
Author

minhiu commented Feb 13, 2025

@jcfranco That would be a great to have, but not needed feature from our end! Could be a great enhancement to pass in focusTrapOptions in the future!

@geospatialem geospatialem added p - high Issue should be addressed in the current milestone, impacts component or core functionality regression Issues that are caused by changes in a release, but were working before that. and removed needs triage Planning workflow - pending design/dev review. need more info Issues that are missing information and/or a clear, actionable description. labels Feb 14, 2025
@geospatialem geospatialem added this to the 3.0.0 Patch - 2025-02-19 milestone Feb 14, 2025
@geospatialem geospatialem added the estimate - 5 A few days of work, definitely requires updates to tests. label Feb 14, 2025
@jcfranco
Copy link
Member

We inadvertently removed focusTrapDisabled’s reactiveness, so we’ll look into this for next week’s patch. Thanks for reporting this!

@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Feb 18, 2025
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Feb 18, 2025
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Feb 19, 2025
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco Feb 19, 2025
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem removed the 3 - installed Issues that have been merged to master branch and are ready for final confirmation. label Feb 19, 2025
@geospatialem geospatialem added the 4 - verified Issues that have been released and confirmed resolved. label Feb 19, 2025
@geospatialem
Copy link
Member

Verified for the upcoming patch with 3.1.0-next.10

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Business/Community Analyst Issues logged by ArcGIS Business/Community Analyst team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 5 A few days of work, definitely requires updates to tests. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality regression Issues that are caused by changes in a release, but were working before that.
Projects
None yet
Development

No branches or pull requests

4 participants