Problem/Motivation

At mobile widths, secondary tabs are re-ordered so the active tab appears first, regardless of its position within the group of tabs. This violates Success Criterion 3.2.3: Consistent Navigation. The tab's position amongst its siblings needs to be the same on every page & viewport width.

This was identified in this Olivero issue: #3129257: Olivero: Mobile tabs can become out of order if browser is resized, which was requesting that Olivero reproduce the Claro behavior reported here.

Steps to reproduce

Go to any page that offers multiple secondary tabs. Visit a tab that isn't the first. On mobile, it is displayed as the first tab despite not actually being the first

Proposed resolution

Find an approach that offers the benefits of showing the active tab without re-ordering it to the first tab on mobile

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#20 3210435-20-d94.patch2.88 KBlauriii
#20 3210435-20-d10.patch2.9 KBlauriii
#18 3210435-after_patch.png30.9 KBabhijith s
#18 3210435-before_patch.png42.4 KBabhijith s
#7 with patch.png87.22 KBmherchel
#7 before.png104.19 KBmherchel
#6 Captura de Pantalla 2021-06-29 a la(s) 17.22.11.png183.8 KBjavi-er

Issue fork drupal-3210435

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

javi-er made their first commit to this issue’s fork.

javi-er opened merge request !869

javi-er’s picture

javi-er CreditAttribution: javi-er at Lullabot commented
Status: Active » Needs review

Hi! I made a change to address this, I added a line to `Drupal.behaviors.navTab` to ignore secondary tabs, so these aren't re ordered to show the active element first. Since secondary tabs aren't collapsible, this could probably work regarding a11y issues.

javi-er’s picture

javi-er CreditAttribution: javi-er at Lullabot commented
StatusFileSize
new Captura de Pantalla 2021-06-29 a la(s) 17.22.11.png183.8 KB

This is how it looks after the patch, notice the active secondary tab being second instead of first:

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new before.png104.19 KB
new with patch.png87.22 KB

Confirmed that this looks good and ensures that the secondary navigation items do not reposition themselves. Great fix!

saschaeggi’s picture

saschaeggi
he/him
CreditAttribution: saschaeggi at Unic commented

Sounds like a solid and obvious change to me. So I'd give that a go from a design point of view. Also I didn't had it present anymore that we did define it to behave this way as discussed with @lauriii in Slack today.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina at Lullabot commented

+1 for me too. I've tested it and works perfect, and the changes make a lot of sense.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work

Posted some feedback on the MR.

sagarchauhan made their first commit to this issue’s fork.

sagarchauhan’s picture

sagarchauhan CreditAttribution: sagarchauhan at Material for Drupal India Association commented
Status: Needs work » Needs review

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Issue tags: +stable blocker
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: kristen pol at Itty Bitty Byte for Salsa Digital commented
Status: Needs review » Needs work
Issue tags: +Bug Smash Initiative

Hmm... MR shows 1000+ changes and The source branch is 286 commits behind the target branch... needs to be fixed for 9.4 properly so moving back to needs work.

kostyashupenko made their first commit to this issue’s fork.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
CreditAttribution: kostyashupenko at Skilld commented
Status: Needs work » Needs review

Rebased & ready for review

abhijith s’s picture

abhijith s CreditAttribution: abhijith s as a volunteer and at Zyxware Technologies commented
StatusFileSize
new 3210435-before_patch.png42.4 KB
new 3210435-after_patch.png30.9 KB

Applied MR !869 on d9.4.x and its working fine.The reordering of secondary tabs are removed in this patch.

Before patch:
before

After patch:
after

RTBC +1

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: kristen pol at Itty Bitty Byte for Salsa Digital commented
Status: Needs review » Reviewed & tested by the community

Thanks for the MR update and the testing. Marking RTBC based on:

1. Patch CSS changes is fairly straight forward though I'm not sure it's the approach the Claro team would approve.

2. Patch appears to only address the issue in the issue summary.

3. Issue title and summary are fairly clear.

4. Tests pass.

5. Manual testing passes.

6. Unclear if specific tests can be added for this.

7. Patch applies cleanly to 9.3 and 9.4 and with offsets for 10:

[drupal-10.0.x-dev/10.0.x] [drupal-10.0.x-dev]$ patch -p1 < 869.diff 
patching file core/themes/claro/css/components/tabs.css
Hunk #1 succeeded at 204 (offset 10 lines).
Hunk #2 succeeded at 254 with fuzz 2 (offset 10 lines).
patching file core/themes/claro/css/components/tabs.pcss.css
Hunk #1 succeeded at 174 (offset -2 lines).
Hunk #2 succeeded at 224 (offset -2 lines).
patching file core/themes/claro/templates/menu-local-tasks.html.twig
patching file core/themes/claro/templates/navigation/menu-local-task.html.twig
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
StatusFileSize
new 3210435-20-d10.patch2.9 KB
new 3210435-20-d94.patch2.88 KB

  • lauriii committed 36275db on 10.0.x 
    Issue #3210435 by javi-er, kostyashupenko, sagarchauhan, mherchel,...

  • lauriii committed 6e58ed8 on 9.4.x 
    Issue #3210435 by javi-er, kostyashupenko, sagarchauhan, mherchel,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed 36275db and pushed to 10.0.x. Also committed the 9.x patch to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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