Automatic Scroll for Visibility in Expanded Parent Menus

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

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

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

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++

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.

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

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++

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:

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

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

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

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.

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 →

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

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...

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

@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... →

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.