Skip to content

fix(Carousel): add aria-current attribute to active dot#4447

Merged
benjamincanac merged 2 commits intonuxt:v3from
J-Michalek:feat/carousel-aria-current-attr
Jul 7, 2025
Merged

fix(Carousel): add aria-current attribute to active dot#4447
benjamincanac merged 2 commits intonuxt:v3from
J-Michalek:feat/carousel-aria-current-attr

Conversation

@J-Michalek
Copy link
Contributor

@J-Michalek J-Michalek commented Jul 2, 2025 •
edited by benjamincanac
Loading

🔗 Linked issue

Resolves: #4403

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

I've added the aria-current attribute to the active dot in the Carousel component. This should improve the a11y when using assistive technologies.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@J-Michalek J-Michalek requested a review from benjamincanac as a code owner July 2, 2025 11:51
@benjamincanac benjamincanac changed the title feat(Carousel): add aria-current attribute to active dot fix(Carousel): add aria-current attribute to active dot Jul 2, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jul 2, 2025 •
edited
Loading 

npm i https://pkg.pr.new/@nuxt/ui@4447

commit: 5955423

@benjamincanac
Copy link
Member

benjamincanac commented Jul 2, 2025

Thanks @J-Michalek! I'm wondering why the tests are passing, we might be missing some if we don't cover this 😬

@J-Michalek
Copy link
Contributor Author

J-Michalek commented Jul 2, 2025

@benjamincanac Yes, that is weird, I've inspected the test snapshot and I can see that there are no dots being rendered, most likely due to scrollSnaps being an empty array. Most likely there is something wrong with useEmblaCarousel inside of the test. I'll look into it.

@benjamincanac
Copy link
Member

benjamincanac commented Jul 2, 2025

Yeah probably, if you can't make it work it's not a big deal!

@J-Michalek
Copy link
Contributor Author

J-Michalek commented Jul 2, 2025

Seems like it would be hard to debug and/or would require changes in the test structure. I was thinking, is it mandatory to use the scrollSnaps for this? If we could iterate over the items prop instead, then it would also work inside of the test, if its worth the hassle.

@benjamincanac
Copy link
Member

benjamincanac commented Jul 2, 2025

Yes we don't really have a choice, the number of dots is not necessarily equal to the number of items.

@J-Michalek
Copy link
Contributor Author

J-Michalek commented Jul 2, 2025

Oh yeah, it always displays like 5 of them even if there are more images got it, then I would take this PR as final.

@benjamincanac benjamincanac merged commit 1ba8a55 into nuxt:v3 Jul 7, 2025
6 checks passed
@benjamincanac benjamincanac added the v3 #1289 label Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamincanac benjamincanac benjamincanac approved these changes

Assignees

No one assigned

Labels

v3 #1289

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UCarousel selected dot has no attribute

2 participants

@J-Michalek @benjamincanac