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): improve show & hide people accessibility #10954

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

michelheusschen
Copy link
Contributor

  • Use focus trap
  • Add label for each toggle visibility state
  • Mark as modal dialog
  • Make toggling individual people accessible

@michelheusschen michelheusschen force-pushed the feat/web-improve-show-hide-people-a11y branch from 467b4e0 to 33007d9 Compare July 8, 2024 10:15
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.

This is better than what we have now. I was wondering what you think about just having a dedicated secondary menu instead though. Similar to the "add to album" action, we could just have it open with these options:

  • Show all
  • Hide all
  • Hide unnamed
    I think it would be more usable/intuitive. Clicking through a list of options is a bit more confusing imo.

@alextran1502
Copy link
Contributor

I agree with Jason that having a context menu with explicit text will help with UX

@alextran1502 alextran1502 merged commit a0f6d74 into main Jul 9, 2024
22 checks passed
@alextran1502 alextran1502 deleted the feat/web-improve-show-hide-people-a11y branch July 9, 2024 03:42
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