Skip to content

Conversation

@mattaebersold
Copy link
Contributor

Level access audit for COR flagged this as a high priority issue. Basically they are saying that aria-disabled shouldn't be how we set the active slide dot state. They suggest aria-current.

I tested this simple change locally, and things seem to be fine, but I wanted to show you in case you think of some other reasons why we shouldn't do this.

One ramification is that if other sites update the carousel package, they will need to update any custom styling that hooks into the disabled attribute...

image

From level access:
image

@mattaebersold mattaebersold requested a review from weotch April 10, 2024 20:04
@codesandbox
Copy link

codesandbox bot commented Apr 10, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@cypress
Copy link

cypress bot commented Apr 10, 2024

Passing run #118 ↗︎

0 18 0 0 Flakiness 0

Details:

trying out aria-current for active slide
Project: vue-ssr-carousel Commit: 4cf56b00f6
Status: Passed Duration: 01:15 💡
Started: Apr 10, 2024 8:03 PM Ended: Apr 10, 2024 8:04 PM

Review all test suite changes for PR #144 ↗︎

@netlify
Copy link

netlify bot commented Apr 10, 2024

Deploy Preview for vue-ssr-carousel ready!

Name Link
🔨 Latest commit 4cf56b0
🔍 Latest deploy log https://app.netlify.com/sites/vue-ssr-carousel/deploys/6616f0dde76161000824d3f6
😎 Deploy Preview https://deploy-preview-144--vue-ssr-carousel.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mattaebersold mattaebersold self-assigned this Apr 10, 2024
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.

1 participant