fix: show copy buttons on touch devices and fix aria-label mismatch#2203
fix: show copy buttons on touch devices and fix aria-label mismatch#2203thealxlabs wants to merge 7 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated CopyToClipboardButton.vue ARIA-label fallback logic: Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c13f9f9-c27b-4c3a-8f2e-310f4e119844
📒 Files selected for processing (2)
app/components/CopyToClipboardButton.vuetest/nuxt/components/CopyToClipboardButton.spec.ts
| const ariaLabel = button.attributes('aria-label') ?? '' | ||
| const visibleText = button.text() | ||
| // The aria-label should equal the visible text (not some other string) | ||
| expect(visibleText).toContain(ariaLabel) | ||
| }) |
There was a problem hiding this comment.
Final mismatch test uses a weak/reversed assertion for its stated intent.
This can pass when aria-label is only a substring of visible text, so it does not reliably guard against label/content-name mismatch.
Proposed test tightening
- const ariaLabel = button.attributes('aria-label') ?? ''
- const visibleText = button.text()
- // The aria-label should equal the visible text (not some other string)
- expect(visibleText).toContain(ariaLabel)
+ const ariaLabel = (button.attributes('aria-label') ?? '').trim()
+ const visibleText = button.text().trim()
+ // The aria-label should equal the visible text (not some other string)
+ expect(ariaLabel).toBe(visibleText)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e violation The @media (hover: none) block that made copy buttons always visible on touch caused the absolutely-positioned button (top: 100%) to appear between the package header and the sticky subheader, landing within 24px of the provenance shield link and failing the target-size audit. Revert to display: none on touch devices to eliminate the overlap.
|
Hey there, there already is an open PR (#998) to resolve this issue, even though it wasn’t linked (since it came before the issue was filed). We’re actively working on that one—just waiting for a dependency upgrade, so I will be closing this one. Thank you anyway! |
🔗 Linked issue
resolves #1771
🧭 Context
Two related issues with
CopyToClipboardButton: (1) copy buttons were hidden on touch devices because they relied on a CSS hover state that never fires on mobile, and (2) the button'saria-labelcould differ from its visible text, causing an accessibility label/content mismatch for screen readers.📚 Description
@media (hover: none)CSS block to make copy buttons visible and interactable on touch-only devices, where hover is unavailable.aria-labelcomputation so it defaults tocopyText(the same string shown to sighted users) rather than a separate value, eliminating the WCAG label-content-name mismatch.A regression test (
test/nuxt/components/CopyToClipboardButton.spec.ts) verifies that:aria-labelmatches the visiblecopyTextwhen no explicit override is givenariaLabelCopyprop takes precedencecopiedTextin both label and visible text