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

Ignoring a11y false positives violations related to li elements inside DatePicker with Time Component. #1581

Merged
merged 8 commits into from
Sep 21, 2023

Conversation

siddhantrawal
Copy link
Contributor

@siddhantrawal siddhantrawal commented Sep 18, 2023

Two of the a11y violations related to DatePicker Component were failing locally but passing correctly on Cypress GUI. The issue was caused due to all the li elements inside the DatePickerWithTimeExample and DatePickerWithCombinedTimeExample Component not meeting color-contrast criteria locally.

Screenshot 2023-09-18 111002
Screenshot 2023-09-20 104806

Electron Browser ( DatePickerWithCombinedTimeExample):

FailureSummary: Element has insufficient color contrast of 1.38 (foreground color: #dbdbdb, background color: #ffffff, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1"

image

Electron Browser ( DatePickerWithTimeExample):

FailureSummary: Element has insufficient color contrast of 1.38 (foreground color: #dbdbdb, background color: #ffffff, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1"
image

Locally (Terminal) :
Screenshot 2023-09-05 114508

Cypress GUI:
Screenshot 2023-09-18 105202

As discussed during the call, they were false positives.

Change

Thanks to Rohan for this. Workaround for this was to ignore these violations as shown in this config.

Testing

Ran it locally and on Cypress GUI and no longer got the failure result.

Locally (Terminal) :
Screenshot 2023-09-18 111559

Cypress GUI:
Screenshot 2023-09-18 130712

Docs

"N/A".

@siddhantrawal siddhantrawal added the a11y Accessibility issues (keyboard navigation, color contrast, assistive technologies, semantics, etc) label Sep 18, 2023
@siddhantrawal siddhantrawal requested a review from a team as a code owner September 18, 2023 17:08
@siddhantrawal siddhantrawal self-assigned this Sep 18, 2023
@siddhantrawal siddhantrawal requested a review from a team as a code owner September 18, 2023 17:08
@siddhantrawal siddhantrawal requested review from gretanausedaite and r100-stack and removed request for a team September 18, 2023 17:08
Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

suggestions to improve maintainability:
(1) in the PR description, add an explanation with screenshot of exactly what part is triggering the false positive.
(2) in the code, include a comment linking to this PR.

@siddhantrawal siddhantrawal changed the title Ignoring false positives a11y DatePicker Component violations. Ignoring a11y false positives violations related to li elements inside DatePicker with Time Component. Sep 18, 2023
Copy link
Contributor

@gretanausedaite gretanausedaite left a comment

Choose a reason for hiding this comment

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

No comments from my side.
There are couple more contrast failures in a11y test results. They are not false positive?

@siddhantrawal
Copy link
Contributor Author

No comments from my side. There are couple more contrast failures in a11y test results. They are not false positive?

I have that in my mind and will plan to double-check if they are false positives or not. I plan to handle those in different PR if that's okay?

@siddhantrawal
Copy link
Contributor Author

siddhantrawal commented Sep 20, 2023

suggestions to improve maintainability: (1) in the PR description, add an explanation with screenshot of exactly what part is triggering the false positive. (2) in the code, include a comment linking to this PR.

Updated. Please Verify.

@mayank99
Copy link
Contributor

Screenshots within cypress browser with highlights would better:

Combined with the full message (from the cypress browser console):

"Element has insufficient color contrast of 1.38 (foreground color: #dbdbdb, background color: #ffffff, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1"

@siddhantrawal
Copy link
Contributor Author

Screenshots within cypress browser with highlights would better:

Combined with the full message (from the cypress browser console):

"Element has insufficient color contrast of 1.38 (foreground color: #dbdbdb, background color: #ffffff, font size: 10.5pt (14px), font weight: normal). Expected contrast ratio of 4.5:1"

Updated. Thanks for the help.

Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM for now.

Since we discovered this issue was specific to Electron browser (which is what gets used in CI), we might want to consider switching to Chromium in a future PR.

@siddhantrawal siddhantrawal enabled auto-merge (squash) September 21, 2023 13:11
@siddhantrawal siddhantrawal merged commit a903f9b into dev Sep 21, 2023
15 of 16 checks passed
@siddhantrawal siddhantrawal deleted the siddhant/datepicker-a11y-fix branch September 21, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility issues (keyboard navigation, color contrast, assistive technologies, semantics, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants