open-sauced / app

🍕 Insights into your entire open source ecosystem.
https://pizza.new
Apache License 2.0
429 stars 228 forks source link

fix: a11y fail click events have key events and no static element interactions #4169

Closed FatumaA closed 3 weeks ago

FatumaA commented 1 month ago

Description

Addresses the accessibility problems in the files listed in the linked issue.

Related Tickets & Documents

Fixes #4130

Mobile & Desktop Screenshots/Recordings

AuthSection: authsection - after authsection -before

CardRepoList: cardrepolist - after cardrepolist - before

ContributorFilterDropdown: contributorFilterDropdown - after contributorFilterDropdwon - before

ContributorHighLightCard: Neutral and hiver states remain same - contributorHighlightCard

contributorHighloghtCard - hover

ContributorlistTableRow: No diff, except it now has a cursor on hover: contributorlistTableRow - no diff except cursor

DevCard: devcard - no diff

HighlightFilterCard: highlight filtercard - no diff

HighlightInputForm close button: highlightinputform - close button

HighlightInputForm - suggested links: highlightinputform - suggested highlight link

PillSelector: pill selector

pill selector after

RepositoryCartItem: repositorycartitem - no diff

Search: search search - focussed

Search-dialog: search-dialog - no diff

SuperlativeSelector: Unselected - superlative selector - no diff Selected - superlative selctor - selectec - no diff

TextInput: textinput - before

textinput- after

ToggleOption: toggleoption - after toggleoption - before

With Icon toggleoption with icon - after toggleoption with icon - before

Card in pages/u/[username]/Card.tsx: u:username card - no diff

Radio and Radio Check: Screenshot 2024-10-23 at 01 02 01

Screenshot 2024-10-23 at 01 03 37

Steps to QA

Run npx eslint . in the root folder, and see that those errors are no longer occurring

Tier (staff will fill in)

[optional] What gif best describes this PR or how it makes you feel?

A picture of a black woman chewing with her eyes darting nervously

netlify[bot] commented 1 month ago

Deploy Preview for oss-insights ready!

Name Link
Latest commit d7dd6a0e5388c397e38a09532492ae7e481c5627
Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/6723d456febeb800088dffb1
Deploy Preview https://deploy-preview-4169--oss-insights.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 1 month ago

Deploy Preview for design-insights ready!

Name Link
Latest commit d7dd6a0e5388c397e38a09532492ae7e481c5627
Latest deploy log https://app.netlify.com/sites/design-insights/deploys/6723d4566affee00085b984d
Deploy Preview https://deploy-preview-4169--design-insights.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

FatumaA commented 1 month ago

Hi @nickytonline, This is ready for review.

Also, should I replace all the HTML buttons with the Button component in components/shared/Button/button.tsx?

FatumaA commented 1 month ago

A few more change requests. Can you provide before and after screen shots of where these changes are?

In the case where it's a component that is used in multiple components, just have a before and after screenshots for a single place it's used.

Thanks @FatumaA and looking forward to getting these changes into the code base to make the app more accessible!

Can I do this from the storybook instance? That's what I was using

nickytonline commented 1 month ago

A few more change requests. Can you provide before and after screen shots of where these changes are? In the case where it's a component that is used in multiple components, just have a before and after screenshots for a single place it's used. Thanks @FatumaA and looking forward to getting these changes into the code base to make the app more accessible!

Can I do this from the storybook instance? That's what I was using

Yeah, just point to the Storybook stories.

nickytonline commented 1 month ago

Looks like there are some conflicts that need to be resolved @FatumaA

FatumaA commented 1 month ago

This PR touches 20 components. Of these, the radio and radio check components no longer span 100% of their width, and the dev card wall component is totally broken by switching its surrounding div to a button.

Radio: Before - radio - checked - before radio - hover- before

After - radio radio - hover-after radio - checked - after

RadioCheck: Before - Neutral state - radiocheck - before - neutral radiocheck - checked

Hover state - radiocheck - hover - before radiocheck - hover- checked - before

After -

radiocheck - on hover - after radiocheck-hover-checked-after

DevWallCard: Before: devcardwall - before

After: devcardwall -after - broken

FatumaA commented 1 month ago

As for the other components, I did not notice any difference in their look or function with the changes

AuthSection: authsection - after authsection -before

CardRepoList: cardrepolist - after cardrepolist - before

ContributorFilterDropdown: contributorFilterDropdown - after contributorFilterDropdwon - before

ContributorHighLightCard: Neutral and hiver states remain same - contributorHighlightCard

contributorHighloghtCard - hover

ContributorlistTableRow: No diff, except it now has a cursor on hover: contributorlistTableRow - no diff except cursor

DevCard: devcard - no diff

HighlightFilterCard: highlight filtercard - no diff

HighlightInputForm close button: highlightinputform - close button

HighlightInputForm - suggested links: highlightinputform - suggested highlight link

PillSelector: pill selector

pill selector after

RepositoryCartItem: repositorycartitem - no diff

Search: search search - focussed

Search-dialog: search-dialog - no diff

SuperlativeSelector: Unselected - superlative selector - no diff Selected - superlative selctor - selectec - no diff

TextInput: textinput - before

textinput- after

ToggleOption: toggleoption - after toggleoption - before

With Icon toggleoption with icon - after toggleoption with icon - before

Card in pages/u/[username]/Card.tsx: u:username card - no diff

nickytonline commented 1 month ago

Thanks for all the screenshots for before and after @FatumaA. So we can move this along, can you exclude changes that currently break the UI and we can sort those out in a follow up PR?

Also, thanks for your patience!

FatumaA commented 1 month ago

Thanks for all the screenshots for before and after @FatumaA. So we can move this along, can you exclude changes that currently break the UI and we can sort those out in a follow up PR?

Also, thanks for your patience!

No problem, thank you for yours!

I think the radio and radio check should be a simple fix on the button classes, we should keep the fix in this PR; what do you think?

As for the devwallcard component, I will undo the changes and leave it out as you've suggested.

nickytonline commented 1 month ago

For the radio and radio check feel free to put the fix in here, but also, if you want to take a bit of time, like I said, it can go in a follow up PR.

FatumaA commented 1 month ago

Fixed the radio and radio check. And undid the devcardwall change

Radio and Radio Check: Screenshot 2024-10-23 at 01 02 01

Screenshot 2024-10-23 at 01 03 37

nickytonline commented 1 month ago

This seems to all be working great. The only thing I noticed is the search no longer auto focuses when you click on the search button or press CMD + K (macOS) or CTRL + K (other OSes).

FatumaA commented 1 month ago

This seems to all be working great. The only thing I noticed is the search no longer auto focuses when you click on the search button or press CMD + K (macOS) or CTRL + K (other OSes).

I removed the auto focus on the components, let me look into it

FatumaA commented 1 month ago

@nickytonline I pushed a fix for that, but I noticed the component uses global document methods. Is there any particular reason, or is this up for a refactor?

nickytonline commented 3 weeks ago

@FatumaA, I'll take a peek at this tomorrow.