Skip to content
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

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

Conversation

FatumaA
Copy link
Contributor

@FatumaA FatumaA commented Oct 15, 2024

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)

  • Tier 1
  • Tier 2
  • Tier 3
  • Tier 4

[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

Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for oss-insights ready!

Name Link
🔨 Latest commit d7dd6a0
🔍 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.

@FatumaA FatumaA marked this pull request as ready for review October 22, 2024 14:37
@nickytonline
Copy link
Member

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
Copy link
Contributor Author

FatumaA commented Oct 22, 2024

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
Copy link
Member

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
Copy link
Contributor Author

FatumaA commented Oct 22, 2024

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
Copy link
Member

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
Copy link
Contributor Author

FatumaA commented Oct 26, 2024

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

auto-merge was automatically disabled October 26, 2024 09:41

Head branch was pushed to by a user without write access

@FatumaA
Copy link
Contributor Author

FatumaA commented Oct 26, 2024

@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
Copy link
Member

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

Copy link
Member

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @FatumaA! 🚢

Copy link
Contributor

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit about the .env

.env Outdated Show resolved Hide resolved
Co-authored-by: Brandon Roberts <[email protected]>
@nickytonline nickytonline added this pull request to the merge queue Oct 31, 2024
Merged via the queue into open-sauced:beta with commit f86e76b Oct 31, 2024
11 checks passed
open-sauced bot pushed a commit that referenced this pull request Oct 31, 2024
## [2.64.1-beta.2](v2.64.1-beta.1...v2.64.1-beta.2) (2024-10-31)

### 🐛 Bug Fixes

* a11y fail   click events have key events and no static element interactions  ([#4169](#4169)) ([f86e76b](f86e76b))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants