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

feat(web,a11y): standardize base modal #8388

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

ben-basten
Copy link
Member

Description

Standardized BaseModal header with the following:

  • left aligned
  • sentence case
  • consistent sizing
  • immich blue color
  • optional logo/icon to the left of the header

Improved accessibility:

  • added aria-modal and aria-labelledby, so the screen reader now announces the header when the modal is opened
  • removed escape event, so any modal that handles closing a modal also automatically handles the escape key
  • removed autofocus attribute from the album selection modal, which was stealing focus and preventing the screen reader from announcing the modal header

Screenshots

Expand for screenshots

Immich logo

modal-logo

Icon

modal-icon

No icon

modal-no-icon

Dark mode logo

modal-dark-mode

Dark mode icon

modal-dark-mode-icon

How Has This Been Tested?

  • MacOS + VoiceOver
    • Chrome
    • Firefox
    • Safari
  • Windows + NVDA - I don't have a Windows VM setup yet...
    • Chrome
    • Firefox

Affected modals:

  • Add to album
  • Add to shared album
  • Account Settings > Partner Sharing > Add partner
  • Link sharing (from asset viewer or album detail views)
  • Set profile picture
  • Create shared link
  • Albums > Album ID > Sharing "Options" modal

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)

* consistent headings
* remove escape key handler
* add aria attributes
Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you

@jrasm91 jrasm91 merged commit f7afc03 into immich-app:main Apr 2, 2024
23 checks passed
@ben-basten ben-basten deleted the feat/standard-modal-header branch April 2, 2024 17:06
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

3 participants