pharos
pharos copied to clipboard
Card: Improve accessibility by moving `overlay` slot outside of `<a>` tag
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).
🦋 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
size-limit report 📦
Path | Size |
---|---|
packages/pharos/lib/index.js | 52.79 KB (+0.01% 🔺) |
@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 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 No worries at all, appreciate the work in this, just trying to do a little GitHub cleanup where I can! 😄