Problem/Motivation

The "skip to main content" link style looks unfinished at the narrow breakpoints.

Some of the styles are currently restricted by this selector:

body:not(.is-always-mobile-nav) .skip-link {}

Proposed resolution

A simple approach would be to use the same style at all breakpoints.

Remaining tasks

Update CSS

CommentFileSizeAuthor
#17 Снимок экрана 2020-04-10 в 13.40.35.png156.01 KBkostyashupenko
#16 3119789-16.patch1.84 KBkostyashupenko
#11 Screen Shot 2020-04-04 at 10.06.00 AM.png857.96 KBproeung
#11 3119789-10.patch2.67 KBproeung
#10 3119789-9.patch1 KBkostyashupenko
#5 3119789-skip-link.png27.39 KBmherchel
#4 3119789-skip-link.patch4.65 KBmherchel
#3 Снимок экрана 2020-04-01 в 13.52.02.png27.16 KBkostyashupenko
#3 Снимок экрана 2020-04-01 в 13.51.40.png85.9 KBkostyashupenko
#3 3119789-3.patch5.59 KBkostyashupenko

Comments

andrewmacpherson created an issue. See original summary.

andrewmacpherson’s picture

andrewmacpherson commented

I'm wondering if this is a regression. The class came from here #3115430: Create CSS body class that enables mobile menu at all widths. What was the skip link like at narrow breakpoints before that change was merged?

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Active » Needs review
StatusFileSize
new 3119789-3.patch5.59 KB
new Снимок экрана 2020-04-01 в 13.51.40.png85.9 KB
new Снимок экрана 2020-04-01 в 13.52.02.png27.16 KB

1. Removed utility.css
2. Moved skip-link component from utility.css to skip-link.css
3. Reworked skip-link a little bit. Added :hover state (underline), also --color--blue-50 background was added, since sometimes we had white text above white background. On my side with high-contrast mode skip-link looks readable.

Screenshots:
1. Hover state:
Skip link hover

2. Mobile styles
Mobile skip link

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
StatusFileSize
new 3119789-skip-link.patch4.65 KB

Made a couple minor changes:

1) Changed the background color to ensure WCAG compliance (see #3119790: Insufficient contrast on the skip-link)
2) Added border, made font larger for accessibility reasons (see #3112482: Stronger focus style for skip-to-main link)

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 3119789-skip-link.png27.39 KB

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Fixed

Committed!

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
andrewmacpherson’s picture

andrewmacpherson commented
Status: Fixed » Needs work
Issue tags: +Accessibility

Thanks for working on these skip-link issues.

Re. #3.2 - I really like that mobile appearance.

Re. #4.2

made font larger for accessibility reasons (see #3112482: Stronger focus style for skip-to-main link)

What accessibility reason? The other issue didn't mention font size, because there wasn't anything wrong with that.

Patch #4:

+.skip-link {
+  background-color: var(--color--blue-20);
+  border: solid 1px rgba(255, 255, 255, 0.5)

Why use the alpha component in the border colour here? It means the border is effectively a pale blue which has a contrast ratio of just 2.3:1 versus the skip link background of blue-20, and even less against the surrounding header gradient. That's not a very strong focus indicator; there's no point in an outline which is hard to see.

I recommend removing the alpha component from the skip link border colour, and using solid white.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Needs work » Needs review
kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
StatusFileSize
new 3119789-9.patch1 KB
proeung’s picture

proeung
she/her
commented
StatusFileSize
new 3119789-10.patch2.67 KB
new Screen Shot 2020-04-04 at 10.06.00 AM.png857.96 KB

I'm purposing a patch that changes the position of the "focus" state to relative and extending the bar across the width of the page wrapper. Let me know what you all think.

Olivero skip link

andrewmacpherson’s picture

andrewmacpherson commented

#10: this is good, fixes the low-contrast problem from #8.

#11: I think #10 is RTBC-worthy. What's the thinking behind the design in #11?

andrewmacpherson’s picture

andrewmacpherson commented

When the toolbar (module) is shown for a ligged-in user, the skip-link doesn't appear. It's positioned at the top of the viewport, and is obscured by the toolbar.

Bartik, Seven, Umami (via Classy?) take this into account when positioning it.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

That's a good catch.

@proeung I've fixed some similar issues at https://git.drupalcode.org/project/olivero/-/blob/8.x-1.x-dev/css/src/co..., where you can make use of the same styles.

Note you'll not want to use the var(--sp4) or :not(.is-always-mobile-nav)

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Assigned: Unassigned » kostyashupenko
kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Assigned: kostyashupenko » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3119789-16.patch1.84 KB

Reroll, also about:

When the toolbar (module) is shown for a ligged-in user, the skip-link doesn't appear. It's positioned at the top of the viewport, and is obscured by the toolbar.

This issue related to toolbar.js Cause of it is wrong padding-top calculation value when toolbar exist (it happens on my side only on page-load), so i guess it is https://www.drupal.org/project/drupal/issues/2958478), we don't need to fix it here. For now after page was loaded you can just click on any toolbar-tab, then calculation of padding-top for body will be adapted and all will be fine with skip-link.
Btw this bug happens on my side only on user/ pages. For example on node/# all fine by default. Anyway :)

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
StatusFileSize
new Снимок экрана 2020-04-10 в 13.40.35.png156.01 KB

This is how it looks currently on my side:
skip link

Well, maybe make sense to show skip-link above toolbar? Idk :)

brianperry’s picture

brianperry
he/him
commented
Status: Needs review » Reviewed & tested by the community

Reviewed and tested the latest patch and the skip link functions correctly and the styles appear consistent.

I was unable to duplicate the issues with the skip link being obscured by the admin toolbar for some reason. Based on the fact that there is a related core issue for this that could solve this problem, I've created https://www.drupal.org/project/olivero/issues/3133458 to track the toolbar issue specifically. Helping the core issue through would be ideal. If we do have to make specific fixes in Olivero, I'd imagine they would have to be adjusted once the root cause is solved.

Assuming that the admin toolbar overlap is no longer part of this issue, marking it RTBC.

proeung’s picture

proeung
she/her
commented
Status: Reviewed & tested by the community » Fixed

I committed the patch that @kostyashupenko (#16) submitted as it's good enough to be included in our Alpha release. If there are additional feedback and bug found related to the admin toolbar, let's open a follow-up ticket on this.

@brianperry Thanks for reviewing and testing this patch!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.