-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Establish a verification checklist #198
Comments
@natalia-fitzgerald Looks great!
|
This is looking great @natalia-fitzgerald!
I have mixed feelings if the component needs to be used before it can be validated? When I've integrated new components from component libraries, I often do have to make small tweaks, but I don't think that should prevent it from being validated?
How have people been verifying this so far? I usually use VoiceOver on my mac, but I've seen significant differences when testing things out in JAWS or Narrator on windows at previous gigs. It wouldn't have to be part of this more high level definition of done document, but it might be worth breaking this task down into smaller tasks when we do our code reviews (Verify that meaningful visible text is conveyed, links have descriptive text, etc.). Maybe just by linking to a generic accessibility checklist that includes a screen reader section like this one or just the WCAG docs?
Could we add something about testing coverage here to this section? I saw in sbl-project's definition of done a generic >85% code coverage requirement, which while an arbitrary benchmark, might be nice to have here. Thanks Natalia! |
Thanks for your feedback @billhimmelsbach @meissadia
|
Great WCAG guidelines! Sounds good to me for whatever manual testing (screen reader / tab navigation) we want to do.
I'm down for including lighthouse, but I think we already run Storybook's A11y extension which is based on axe-core's accessibility tooling? It's the one that pops up in the accessibility tab on the bottom of storybook components. Might be worth just tossing whatever we decide into a GitHub action too, so that it just runs on PRs.
Probably the component level for me? For example, best to avoid things like creating a
Yeah, when building out the components, devs have been writing automated tests to verify the functionality of the component, ensure future devs don't mess things up when doing refactoring, etc. Here's some good tests @meissadia wrote for pagination, for example. For the code coverage metric, >85% would mean that at least that amount of the lines of code should be covered by some kind of automated test. For a lot of the simpler components in the library so far, rendering the component alone is enough to reach this metric. For more complicated components, it's an arbitrary but nice benchmark to set, just so we make sure our code is well tested for future refactors and what not. I'm happy to move the percentage up or down depending on how people feel. 👍 |
I haven't used Chromatic much either @meissadia, but I put up a PR this morning to hopefully at least get the integration working again. 🤞 I have used other visual regression tooling, so might be worth fixing up once our components are more stable. |
Verifying DSR component
|
We are closing this one out since we are using the checklist. |
@meissadia @shindigira @billhimmelsbach
I would like to establish a verification checklist for our components so that we can share in the responsibilities of reviewing and verifying. Please review provide feedback or additions.
Verification checklist
1. Verify the CFPB Design System (React) component against the CFPB Design System
For existing components
For new components
2. Run accessibility checks
3. Verify component unit tests
yarn vitest Button
)4. Prepare a PR for Code Review
design(XXXX): final design review for XXXX component
5. Conduct Code PR review
6. Conduct Design PR review
Draft
toVerified
in Storybook7. Verify component
The text was updated successfully, but these errors were encountered: