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(skip-link): fix skip-link styles with new design #607

Merged
merged 6 commits into from
Apr 5, 2022

Conversation

straker
Copy link
Contributor

@straker straker commented Mar 30, 2022

Closes issue: #455

The whitespace between the spans isn't needed as the display: block causes a whitespace.

VoiceOver output of link reading 'Skip to Main Content'

Chrome Accessibility Tree output of link reading 'Skip to Main Content'

@straker straker requested a review from a team as a code owner March 30, 2022 22:54
scurker
scurker previously requested changes Apr 4, 2022
Copy link
Member

@scurker scurker left a comment

Choose a reason for hiding this comment

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

A few small things

@@ -1,19 +1,17 @@
:root {
--skip-link-background-color: rgba(11, 14, 17, 0.9);
--skip-link-background-color: rgba(255, 255, 255, 0.95);
Copy link
Member

Choose a reason for hiding this comment

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

Did you check with @awpearlm on this? The UX isn't clear, the notes say it's 90% opacity but the actual spec shows 95%.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know if anything needs to change where, just want to validate that 95% is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just going off the uxpin notes.

@straker straker merged commit f766f9d into develop Apr 5, 2022
@straker straker deleted the skiplink-styles branch April 5, 2022 21:41
@scurker scurker mentioned this pull request Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants