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(tab-button): use darker text to pass a11y #27355

Merged
merged 16 commits into from
May 11, 2023
Merged

fix(tab-button): use darker text to pass a11y #27355

merged 16 commits into from
May 11, 2023

Conversation

brandyscarney
Copy link
Member

Issue number: N/A


  • Changes the default Material Design tab color to $text-color-step-350 or #a6a6a6
  • Changes the default iOS tab color to $text-color-step-400 or #999999
  • Removes the axe skip in the e2e test

These were the minimum color changes needed to pass axe.

@brandyscarney brandyscarney requested a review from a team as a code owner May 2, 2023 20:47
@stackblitz
Copy link

stackblitz bot commented May 2, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions bot added the package: core @ionic/core package label May 2, 2023
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM, but I would like to review the screenshots too. There shouldn't be any issues. I've just learned to not trust screenshots.

@brandyscarney brandyscarney requested review from thetaPC and a team May 3, 2023 18:12
@liamdebeasi
Copy link
Contributor

I updated this branch with main now that it has the new screenshot threshold of 0.1. New screenshots were captured in e08d9e6.

@brandyscarney brandyscarney requested a review from a team May 10, 2023 14:18
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@liamdebeasi
Copy link
Contributor

Discussed with Brandy offline, but I manually removed the MD screenshots in 4c40f18 so they can be regenerated. The diff for the MD screenshots is too small to be detected with our current diff threshold of 0.1. However, I am hesitant to lower the threshold below 0.1 because we risk getting false positives with our screenshots.

Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Good to merge once tests pass/screenshots are updated.

@brandyscarney brandyscarney enabled auto-merge May 11, 2023 20:21
@brandyscarney brandyscarney added this pull request to the merge queue May 11, 2023
Merged via the queue into main with commit 0b23814 May 11, 2023
@brandyscarney brandyscarney deleted the FW-3604 branch May 11, 2023 20:53
brandyscarney added a commit that referenced this pull request May 11, 2023
Issue number: N/A

---------

- Changes the default Material Design tab color to
`$text-color-step-350` or `#a6a6a6`
- Changes the default iOS tab color to `$text-color-step-400` or
`#999999`
- Removes the axe skip in the e2e test

These were the minimum color changes needed to pass axe.

---------

Co-authored-by: ionitron <[email protected]>
Co-authored-by: Liam DeBeasi <[email protected]>
brandyscarney added a commit that referenced this pull request May 11, 2023
brandyscarney added a commit that referenced this pull request May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants