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

fix(web): focus escaping from modals #8730

Merged
merged 6 commits into from
Apr 14, 2024

Conversation

ben-basten
Copy link
Member

@ben-basten ben-basten commented Apr 11, 2024

Description

Fixes included in this change:

  1. On initial load, the external library "Add exclusion pattern" modal was incorrectly showing the "Add" button as actionable, even without any path input.
  2. Update the focus trap to exclude any elements that are disabled, since they do not accept focus. This was causing focus to escape out of the External Library "Add exclusion pattern" and "Add import path" modals.

Screenshots

N/A

How Has This Been Tested?

Scenario 1: "Add import path" modal

  1. Open "External Library" admin settings: https://demo.immich.app/admin/library-management
  2. On a library, click the ellipses "..." > Edit Import Paths > Add path
  3. Press the Tab key to move focus through the modal. Observe that it moves outside of the modal. Shift+Tab also gets stuck going backwards in focus.

Scenario 2: "Add exclusion pattern" modal

  1. Open "External Library" admin settings: https://demo.immich.app/admin/library-management
  2. On a library, click the ellipses "..." > Scan Settings > Add Exclusion Pattern
  3. Press the Tab key to move focus through the modal. Observe that it moves outside of the modal. Shift+Tab also gets stuck going backwards in focus.

Checklist:

  • npm run lint (linting via ESLint)
  • npm run format (formatting via Prettier)
  • npm run check:svelte (Type checking via SvelteKit)
  • npm test (unit tests)

@alextran1502 alextran1502 merged commit 25e1887 into immich-app:main Apr 14, 2024
23 checks passed
@ben-basten ben-basten deleted the fix/focus-trap-leak branch April 14, 2024 15:28
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.

None yet

4 participants