pharos icon indicating copy to clipboard operation
pharos copied to clipboard

Card: Improve accessibility by moving `overlay` slot outside of `<a>` tag

Open michael-iden opened this issue 1 year ago • 5 comments

This change: (check at least one)

  • [ ] Adds a new feature
  • [X] Fixes a bug
  • [ ] Improves maintainability
  • [ ] Improves documentation
  • [ ] Is a release activity

Is this a breaking change? (check one)

  • [ ] Yes
  • [X] No - Although I think its pretty close to a breaking change

Is the: (complete all)

  • [X] Title of this pull request clear, concise, and indicative of the issue number it addresses, if any?
  • [ ] Test suite(s) passing?
  • [ ] Code coverage maximal?
  • [ ] Changeset added?
  • [X] Component status page up to date?

What does this change address? Fixes a problem where the button inside of the overlay slot would not be picked up by the browser when tabbing through clickable elements. I believe this is because the button within the overlay was slotted inside of an anchor tag

How does this change work? Move overlay slot outside of the link tag

Additional context I am very nervous about this change. This component is far too complex and takes in too many variable elements through slots. Even with tests and the sandbox I have little confidence that it's not actually going to break the UX in at least some browsers.

Also I am struggling to get the hooks to run properly with the shimmed yarn version. My global yarn in 4.0.1 in the node envs 18+ managed by asdf. I might look to update the application setup to a later yarn if we can get a moment (#635).

michael-iden avatar Nov 20 '23 19:11 michael-iden

🦋 Changeset detected

Latest commit: 00fdddcbfadb7d30f77f5fe6e192a3650c9175fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@ithaka/pharos Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Nov 20 '23 19:11 changeset-bot[bot]

size-limit report 📦

Path Size
packages/pharos/lib/index.js 52.79 KB (+0.01% 🔺)

github-actions[bot] avatar Nov 20 '23 19:11 github-actions[bot]

@michael-iden Wondering what the status of this is, I see it has been open a few months. Is there work needed here? Should it be closed as no longer needed?

brentswisher avatar Apr 24 '24 12:04 brentswisher

@brentswisher this is still an issue but was unfortunately deprioritized and I haven't gotten back around to updating things. I'll try to get an estimate on when I might be able to circle back on this fix and I'll send you a thread directly where we were tangentially talking about this problem.

michael-iden avatar Apr 24 '24 12:04 michael-iden

@michael-iden No worries at all, appreciate the work in this, just trying to do a little GitHub cleanup where I can! 😄

brentswisher avatar Apr 24 '24 13:04 brentswisher