To reproduce

  1. Create a menu link to an anchor on a page
  2. Navigate to the page.
  3. In a mobile view, open the menu and click the link to the page
  4. I expect the menu to close and the anchor to be scrolled to. However nothing happens.

This can also be reproduced on Tugboat at https://tugboat-aqrmztryfqsezpvnghut1cszck2wwasr.tugboat.qa/node/7#comments. The menu link is "Nested Comments" and is under the "Features" top level menu item

Note this came from this slack conversation: https://drupal.slack.com/archives/CJT807H7T/p1625154993136300

CommentFileSizeAuthor
#11 Screen Recording 2021-08-12 at 10.21.53 AM.mov3.57 MBgauravvvv
#10 3221871-10-test-only-FAIL.patch1.3 KBmherchel
#10 interdiff-9-10.txt611 bytesmherchel
#10 3221871-10.patch2.68 KBmherchel
#9 3221871-9-test-only-FAIL.patch1.3 KBmherchel
#9 interdiff-2-9.txt2.59 KBmherchel
#9 3221871-9.patch2.68 KBmherchel
#2 3221871.patch1.31 KBmherchel

Comments

mherchel created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Active » Needs review
StatusFileSize
new 3221871.patch1.31 KB

Patch attached.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

This work as expected, pretty elegant too.

w01f’s picture

w01f commented

Catching up on this from the slack chat. The patch applied cleanly, but something is caching the site I haven't found yet so I will retest tomorrow and follow up. I don't see anything in the patch about removing js-fixed, but I'm assuming somehow it does that as well?

Thanks so much for looking into this and will follow up this weekend!

w01f’s picture

w01f commented

Also tested and works great!

mherchel’s picture

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

lauriii
he/him
Primary language Finnish
Location Finland
commented 
+++ b/core/themes/olivero/js/navigation.es6.js
@@ -104,6 +104,18 @@
+    // If hyperlink links to an anchor in the current page, close the
+    // mobile menu after the click.

"mobile" should be on the previous line. This can be fixed on commit. Other than that, all looks good!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs tests

Coming back at this now, I'm wondering if we could add tests for this?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
StatusFileSize
new 3221871-9.patch2.68 KB
new interdiff-2-9.txt2.59 KB
new 3221871-9-test-only-FAIL.patch1.3 KB

Tests added. The tests actually caught something, too. If the link didn't have a leading forward slash /, it would not work properly. So, the JavaScript is updated too.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
StatusFileSize
new 3221871-10.patch2.68 KB
new interdiff-9-10.txt611 bytes
new 3221871-10-test-only-FAIL.patch1.3 KB

Gah. Forgot a trailing semicolon in the test. New patches attached.

gauravvvv’s picture

gauravvvv commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screen Recording 2021-08-12 at 10.21.53 AM.mov3.57 MB

Patch #10, fixes the issue. Now on clicking to the menu link having id on same works in mobile too. Menu is closing and page scroll down the element having same id as in menu link.

RTBC+1

Attached after patch screen recording for reference.

  • lauriii committed 4f8c468 on 9.3.x 
    Issue #3221871 by mherchel, Gauravmahlawat, W01F, nod_: Olivero: Mobile...

  • lauriii committed d831b94 on 9.2.x 
    Issue #3221871 by mherchel, Gauravmahlawat, W01F, nod_: Olivero: Mobile...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs tests

Committed 4f8c468 and pushed to 9.3.x. Cherry-picked to 9.2.x because Olivero is experimental. Thanks!

Status: Fixed » Closed (fixed)

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