Problem/Motivation

When the parent menu is expanded, the navigation bar should automatically scroll to ensure visibility of the child menus. Presently, although the menu expands, users must manually scroll to determine if it contains any items. Otherwise, there is a misleading impression that the parent menu lacks children.

Steps to reproduce

  1. Reduce the browser height to a size where the Configuration menu is shown, and the People is hidden below the navigation footer.
  2. Navigate to admin/content
  3. Click on Configuration
  4. The sub-menu children of Configuration will open below the footer, when it should actually scroll the show the sub-menu.

You can see the correct behavior by:

  1. Navigate to /admin/structure/block
  2. Resize the browser to a height that hides People
  3. Click on Configuration: it will scroll to fit the sub-menu

Parent Menu expanded

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#25 collapsed_scroll_grandchild.mp4133.24 KBm4olivei
#21 collapsed_scroll.mp4605.25 KBm4olivei
#21 level_2_issue.mp41.05 MBm4olivei
#20 admin _ DrupalPod (7).mp4461.64 KBdishakatariya
#20 admin _ DrupalPod (6).mp4308.6 KBdishakatariya
#19 admin _ DrupalPod (1) (1).mp44.7 MBkanchan bhogade
#19 admin _ DrupalPod (1).mp42.36 MBkanchan bhogade
#16 Monosnap screencast 2024-02-23 10-56-55.gif1.25 MBm4olivei
#15 admin _ DrupalPod.mp41.22 MBdishakatariya
#11 screencast 2024-02-20 09-27-12.gif814.65 KBckrina
#10 after_patch.mp41.12 MBdishakatariya
#10 after.mp42.21 MBdishakatariya
#10 before.mp4499.36 KBdishakatariya
parent-expanded-2023-12-5.gif13.05 MBprashant.c

Issue fork navigation-3406162

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

Prashant.c created an issue. See original summary.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented

I can reproduce this bug: it only happens with 1st menu items that don't have children, like People or Appearance. Thanks @Prashant.c!

pragati_kanade’s picture

pragati_kanade commented

Hi @Prashant.c I'm not able to reproduce this issue.My screen resolution is 1920X1080. I have also tried to reproduce on 1366X768,1440X900,1280,700.
Please guide me further to produce this issue so that I can fix this.
Thanks

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes

Yes, I still can reproduce this. I've added step by step instructions on the issue summary.

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

jatingupta40’s picture

jatingupta40 commented
Assigned: Unassigned » jatingupta40

JatinGupta40 opened merge request !177

jatingupta40’s picture

jatingupta40 commented
Status: Active » Needs review
dishakatariya’s picture

dishakatariya commented
Assigned: jatingupta40 » dishakatariya
dishakatariya’s picture

dishakatariya commented
Assigned: dishakatariya » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new before.mp4499.36 KB
new after.mp42.21 MB
new after_patch.mp41.12 MB

Hi, Verified and tested Merge request !177 on the Drupal 11.x version. Patch applied successfully and working fine.
Testing steps:
1. Install the Drupal 11.x version
2. Install the navigation module
3. Reduce the browser height to a size where the Configuration menu is shown, and the People is hidden below the navigation footer.
4. Navigate to admin/content
5. Click on Configuration
6. The sub-menu children of Configuration will open below the footer, when it should actually scroll the show the sub-menu.
7. You can see the correct behavior by:
8. Navigate to /admin/structure/block
9. Resize the browser to a height that hides People
10. Click on Configuration: it will scroll to fit the sub-menu
Testing Results
When the parent menu is expanded, the navigation bar should be automatically scrolling to ensure visibility of the child menus.
Attaching the recording below.
Can be move to RTBC.
RTBC++

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new screencast 2024-02-20 09-27-12.gif814.65 KB

Thanks all for your work here! After testing this I've found an usability problem this is introducing: if it scrolls too much to fit as much of the item on the screen the 1st menu item label is lost. This label is what actually gives you the context to know which menu item you're in, and if it gets moved beyond the visible area that context is lost.

The ideal behavior would be to place the whole submenu on the visual space, but always ensuring the label of the parent menu item being opened is visible. That could be done in several ways, like calculating its height in the current implementation or rethinking if the current approach of showing "the nearest" item is the best one.

jatingupta40’s picture

jatingupta40 commented
Assigned: Unassigned » jatingupta40

Thank you for noticing this issue @ckrina. I will work on it and update the code.

jatingupta40’s picture

jatingupta40 commented
Assigned: jatingupta40 » Unassigned
Status: Needs work » Needs review
dishakatariya’s picture

dishakatariya commented
Status: Needs review » Reviewed & tested by the community

HI Verified and tested as per #11 comment and Merge request !177. It looks fine to me now.
1st item label is coming properly now and fit to the screen.
attached the recording below.
Hence moving this to RTBC
RTBC++

dishakatariya’s picture

dishakatariya commented
StatusFileSize
new admin _ DrupalPod.mp41.22 MB
m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new Monosnap screencast 2024-02-23 10-56-55.gif1.25 MB

Found some issues from a code and functionality perspective. I'm seeing a "snap-back" behavior sometimes due to the callback thats meant to fix the top position:

jatingupta40’s picture

jatingupta40 commented
Assigned: Unassigned » jatingupta40

Thank @m4olivei for the suggestions you have provided. Will definitely work on it.

jatingupta40’s picture

jatingupta40 commented
Assigned: jatingupta40 » Unassigned
Status: Needs work » Needs review

Hello @m4olivei I have made the changes regarding the suggestion you provided. Can you please again have a look at it? Thanks

kanchan bhogade’s picture

kanchan bhogade commented
StatusFileSize
new admin _ DrupalPod (1).mp42.36 MB
new admin _ DrupalPod (1) (1).mp44.7 MB

Hi
I've tested Merge request !177 on Drupal 11.x for standard and Umami
The patch was applied successfully...
It looks fine to me now.

RTBC+1
attaching videos

Keeping in "needs review" for code verification

dishakatariya’s picture

dishakatariya commented
StatusFileSize
new admin _ DrupalPod (6).mp4308.6 KB
new admin _ DrupalPod (7).mp4461.64 KB

Hi Verified and tested the Merge request !177 on the Drupal 11.x for standard and Umami. This is working as expected now.
Attaching the recording below.
RTBC+1
Keeping it in review as per above comment to review the code.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs review » Needs work
StatusFileSize
new level_2_issue.mp41.05 MB
new collapsed_scroll.mp4605.25 KB

Thanks for the continued work on this! Code is looking better. While testing this time around, I noticed that we have the same reported issue at the second level of links as well. It would be good to handle that here too, eg.

https://www.drupal.org/files/issues/2024-02-28/level_2_issue.mp4

Also, I don't think that we want the auto-scroll behavior at all when the menu is collapsed. The sub-links in this case appear in a pop-over which is visible, so there is no need to scroll the navigation container. When you do that, it feels unexpected and at desktop, you loose hover over the parent item, which closes the popover.

https://www.drupal.org/files/issues/2024-02-28/collapsed_scroll.mp4

jatingupta40’s picture

jatingupta40 commented
Assigned: Unassigned » jatingupta40

Sure @m4olivei. Appreciate your suggestion, Will work on that.

finnsky’s picture

finnsky commented

Thank you for work!
Added one comment.

Large part of that Navigation JS works with custom events.
https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent

So all of things already defined. you only need to catch this event like it happends here

https://git.drupalcode.org/project/navigation/-/blob/1.x/js/sidebar.js#L...

jatingupta40’s picture

jatingupta40 commented
Assigned: jatingupta40 » Unassigned
Status: Needs work » Needs review

Thank you for the review and for providing the suggestions.
I have made the suggested changes. Please review. @finnsky @m4olivei

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs review » Needs work
StatusFileSize
new collapsed_scroll_grandchild.mp4133.24 KB

@JatinGupta40 the latest updates are not taking advantage of the custom event that @finnsky referenced in #23. That is still to do, and can potentially simplify the code here. Have you looked at that?

Also, still seeing some odd behavior when collapsed. If I expand a child menu item to reveal the grandchildren, the navigation scrolls, which is odd.

https://www.drupal.org/files/issues/2024-03-07/collapsed_scroll_grandchi...

KeyboardCowboy’s picture

KeyboardCowboy
Primary language English
Location Denver, CO, USA
commented
Status: Needs work » Closed (outdated)

Thanks for all the work on this! Since we switched to the Drawer designs, though, this is no longer applicable so we're going to close this now.