react
react copied to clipboard
Add accessible column in component statuses
- [x] Add the
a11yReviewed
frontmatter variable to theSelect
andActionMenu
components - [x] Add Accessible column in component statuses
References
- https://github.com/github/primer/issues/1212
- https://github.com/primer/doctocat/pull/468
- https://github.com/primer/primer.style/pull/326
Merge checklist
- [ ] Added/updated tests
- [ ] Added/updated documentation
- [x] Tested in Chrome
- [x] Tested in Firefox
- [x] Tested in Safari
- [ ] Tested in Edge
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.
🦋 Changeset detected
Latest commit: 67c378b0557add5d3f60eecaf718f64e81e1b1e4
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
Name | Type |
---|---|
@primer/react | 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 |
---|---|
dist/browser.esm.js | 77.1 KB (0%) |
dist/browser.umd.js | 77.71 KB (0%) |
Wouldn't alpha/beta status mean that it is accessible? I guess it could be accessible but not satisfy other alpha/beta criteria. 🤔
Hey @pksjce! 👋 that will be the ideal state in the future and eventually this signal/variable might become redundant, but at this point, there are only a couple of React components that have been reviewed by the accessibility team and have no major issues. The idea in the short-mid term is to give more visibility to those signals and components.
(not a blocker, tiny duplication)
is there a way to expose this value from the component checklist? https://github.com/primer/react/blob/main/docs/content/ActionMenu.mdx?plain=1#L366 cc @colebemis
(not a blocker, tiny duplication)
is there a way to expose this value from the component checklist? https://github.com/primer/react/blob/main/docs/content/ActionMenu.mdx?plain=1#L366 cc @colebemis
That's a good point, didn't know about that property in the CheckList component, which makes me think that it might be better to align the name - a11yReviewed
instead of accessbile
- for consistency and clarity.
Shouldn't the CheckList
use the value that comes from the frontmatter instead of the other way around? @siddharthkp
Shouldn't the CheckList use the value that comes from the frontmatter instead of the other way around? @siddharthkp
I'm okay with either direction. I think we wanted to use the checklist in a react component and didn't need to publish it outside of the docs when it was built.
(scope creep, feel free to ignore) I remember someone mentioned it would be nice to record a11yReviewedAt
with a date instead of a boolean so that we can show if a review happened a long time ago and shouldn't be trusted.