Skip to content

Fix carousel dot focus loss with VoiceOver activation#127

Open
deepakpra wants to merge 2 commits intortCamp:developfrom
deepakpra:fix/126-carousel-dot-focus-voiceover
Open

Fix carousel dot focus loss with VoiceOver activation#127
deepakpra wants to merge 2 commits intortCamp:developfrom
deepakpra:fix/126-carousel-dot-focus-voiceover

Conversation

@deepakpra
Copy link
Copy Markdown

Summary

Fixes an accessibility issue where carousel pagination dots lose focus after being activated with VoiceOver keyboard interaction. The dot list is now preserved when the scroll snap count has not changed, preventing unnecessary re-rendering of the focused dot element.

Type of change

  • Bug fix
  • New feature
  • Enhancement/refactor
  • Documentation update
  • Test update
  • Build/CI/tooling

Related issue(s)

Closes #126

What changed

  • Prevented context.scrollSnaps from being recreated on every carousel selection.
  • Kept pagination dot elements stable when the scroll snap count remains the same.
  • Preserved keyboard/screen-reader focus after activating a carousel dot.

Breaking changes

Does this introduce a breaking change? If yes, describe the impact and migration path below.

  • Yes — migration path:
  • No

Testing

Describe how this was tested.

  • Unit tests
  • Manual testing
  • Cross-browser testing (if UI changes)

Test details:

  • Added a carousel with pagination dots on the frontend.
  • Enabled VoiceOver on macOS.
  • Navigated to a carousel pagination dot using keyboard navigation.
  • Activated the focused dot using Control + Option + Space.
  • Verified focus remains on the activated pagination dot after the slide changes.

Screenshots / recordings

Screen.Recording.2026-04-24.at.11.40.14.AM.mov

Checklist

  • I have self-reviewed this PR
  • I have added/updated tests where needed
  • I have updated docs where needed
  • I have checked for breaking changes

@deepakpra deepakpra changed the title fix: preserve carousel dot focus after VoiceOver activation Fix carousel dot focus loss with VoiceOver activation Apr 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an accessibility regression where activating carousel pagination dots via VoiceOver keyboard interaction causes focus to be lost, by keeping the pagination dots’ backing array stable when the number of scroll snaps hasn’t changed.

Changes:

  • Avoid recreating context.scrollSnaps on every Embla select update; only rebuild when the snap count changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/blocks/carousel/view.ts Outdated
Comment thread src/blocks/carousel/view.ts Outdated
Copy link
Copy Markdown
Collaborator

@theMasudRana theMasudRana left a comment

Choose a reason for hiding this comment

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

@deepakpra,
Thanks for addressing the scrollSnapList() reuse feedback.

@theMasudRana theMasudRana self-requested a review April 29, 2026 05:50
Copy link
Copy Markdown
Collaborator

@theMasudRana theMasudRana left a comment

Choose a reason for hiding this comment

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

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Carousel dots lose focus after keyboard activation with VoiceOver

3 participants