In the current state of the PoC when tabbing through the primary and secondary nav the tabbing visual order isn't correct. The tab jumps to the the visual last element and then comes back to the previous. This is produced by some flexbox styling which changes the item order.

Current behavior:

focus order olivero false

Expected behavior:

focus order olivero expected

Also, the order of the desktop version and the mobile version aren't the same. This could also be fixed by design or if that's intentional, we have to change the order of the actual items and not changing the order by CSS.

CommentFileSizeAuthor
olivero-focus-order.gif2.27 MBfhaeberle
olivero-focus-expected.gif2.14 MBfhaeberle
#7 127.0.0.1_5500_index.html_(iPad).png667.27 KBfhaeberle

Comments

fhaeberle created an issue. See original summary.

MaddieC’s picture

MaddieC commented
Assigned: Unassigned » MaddieC
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Assigned: MaddieC » Unassigned

@MaddieC Unassigning you, as I don't want to hold up this issue if other people want to work on this. If you have work, please create a PR within the proof of concept repo on github. Also ping me in Drupal Slack (#d9-theme channel) if you need assistance!

MaddieC’s picture

MaddieC commented
Status: Active » Needs review

Hello @mherchel,

Sorry for the late pull with the code. I have made a PR on the proof of concept repo: https://github.com/Lullabot/olivero-poc/pull/7

mherchel’s picture

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

Looks great. Merged PR. Thank you!

andrewmacpherson’s picture

andrewmacpherson commented
Title: Fix the DOM order for the Login / Sign Up links within the secondary nav » Fix the tabbing order for the Login / Sign Up links within the secondary nav
Status: Fixed » Needs work
Issue tags: +Accessibility

This is still broken. The problem is you're trying to treat the visual reading order differently in the narrow and wide breakpoints.

The screenshots in the issue summary show the problem in the wide breakpoint. It was a failure of WCAG "Focus order".

The solution in #4 wasn't very robust though. This needs to work at both breakpoints.

I Just checked the tab order in the menus, on the current PoC. The tabbing order of the login and signup buttons is mixed between the different breakpoints.

Wide breakpoint: the tabbing order is good. Contact > searchbutton > login > signup
Narrow breakpoint: the tabbing order is wrong. It should go: contact > signup > login. The culprit is .secondary-nav ul {flex-direction: row-reverse;}

Recommend: Make the tabbing order match the visual reading order in both cases. The safest way is to make both of these follow the DOM order. Beware of flexbox direction overrides when there are interactive children; it's a very fast route to a WCAG level A failure. Use of tabindex attributes is definitely not appropriate here.

The problem stems from designs which show the login and sign-up buttons in the opposite reading order at different breakpoints. I can't see a good reason why this is necessary. The PoC has just 2 links in the secondary nav. This will bite you when CMS authors start adding more links.

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Status: Needs work » Needs review
StatusFileSize
new 127.0.0.1_5500_index.html_(iPad).png667.27 KB

I outlined a solution in one of my commits.
The visual solution of this looks like this:


olivero secondary menu poc


I took into account what @andrewmacpherson said and tried to find a good solution. Aligning the button to the right and fixing the focus order to follow the dom order is in my opinion a really good fix for the problem.

https://github.com/Lullabot/olivero-poc/pull/13

mherchel’s picture

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

Thanks for this, requested changes at https://github.com/Lullabot/olivero-poc/pull/13#issuecomment-552447228

markconroy’s picture

markconroy commented
Related issues: +#2983568: Audit and improve focus styles across the Umami theme for logged out users, +#2977510: Refactor/improve Umami demo's search form CSS for better responsive support

Here's some links to Umami issues where we had similar issues to solve
#2983568: Audit and improve focus styles across the Umami theme for logged out users

#2977510: Refactor/improve Umami demo's search form CSS for better responsive support

shaal’s picture

shaal
he/him
Primary language English
Location Boca Raton, FL
commented

Similarly to what I wrote in #3090563: Convey behaviour of navigation submenu to assisitive tech.
In OOTB Umami, we used 2 separate markups for desktop/mobile menus. Would that resolve the issue here as well ?

andrewmacpherson’s picture

andrewmacpherson commented

Umami isn't wrapping 2 menus and a search block.

MaddieC’s picture

MaddieC commented

Seeing that Jen said that we can keep the order on the mobile as on the desktop. Can't we just do that? Without the need for 2 separate markups for desktop/mobile?

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Status: Needs work » Needs review

Updated my pull request. But in the meantime the conversation went to like restructuring the menu? We now simply show the secondary nav on desktop and mobile in the same order.

mherchel’s picture

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

Thanks! Added one more comment on the changes :)

https://github.com/Lullabot/olivero-poc/pull/12#pullrequestreview-315621997

mherchel’s picture

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

disregard the previous comment (it was for another issue). This looks good! Merging!

Status: Fixed » Closed (fixed)

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