Skip to content

fix(web): asset viewer navigation buttons - #7888

Merged
jrasm91 merged 1 commit into
mainfrom
fix/web-asset-viewer-navigation-buttons
Mar 12, 2024
Merged

jrasm91 merged 1 commit into
mainfrom
fix/web-asset-viewer-navigation-buttons

Conversation

@michelheusschen

Copy link
Copy Markdown
Collaborator
  • Show cursor as pointer when hovering over clickable navigation area
  • Improve accessibility
  • Remove unused keydown event

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: bf3f832
Status: ✅  Deploy successful!
Preview URL: https://a5382996.immich.pages.dev
Branch Preview URL: https://fix-web-asset-viewer-navigat.immich.pages.dev

View logs

@jrasm91 jrasm91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need to wrap the button in a div and have the click handler be in the div and not the button?

@michelheusschen

Copy link
Copy Markdown
Collaborator Author

That allows focusing the button and not the whole clickable area (white border in image for illustration). The click event from the button bubbles up to the div where it's handled, that's also why we can safely ignore these a11y warnings.

asset-viewer-button-focus

@jrasm91
jrasm91 merged commit 41504b9 into main Mar 12, 2024
@jrasm91
jrasm91 deleted the fix/web-asset-viewer-navigation-buttons branch March 12, 2024 12:09
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.

2 participants