Once the number of menu items height more than screen height, they overflow outside the page.

When this happen, we are not able to see last number of navigation links. You can seen this issue in the attached GIF.

CommentFileSizeAuthor
#18 after_patch_17.mp4455.71 KBsulfikar_s
#18 before_patch_17.mp4525.49 KBsulfikar_s
#17 interdiff_4_17.txt5.89 KBhinal05
#17 3201370_17.patch6.17 KBhinal05
#17 3201370_17_after.gif4.8 MBhinal05
#17 3201370_4_after.gif1.92 MBhinal05
#17 3201370_before.gif2.19 MBhinal05
#11 after_patch.png263.18 KBMadhu kumar
#11 before_patch.png266.92 KBMadhu kumar
#10 interdiff_09_10.txt554 byteshinal05
#10 3201370_10.patch5.95 KBhinal05
#9 3201370_09.patch5.94 KBhinal05
#9 3201370_09_after.gif4.63 MBhinal05
#9 3201370_09_before.gif3.77 MBhinal05
#7 after_patch.gif746.69 KBbogdan.rizescu
#7 before_patch.gif792.27 KBbogdan.rizescu
#6 3201370-after.gif9.64 MBabhijith s
#6 3201370-before.gif11.84 MBabhijith s
#5 Screenshot 2021-03-04 at 16.49.31.png36.52 KBgauravvvv
#5 Screenshot 2021-03-04 at 16.25.28.png137.79 KBgauravvvv
#4 interdiff_3_4.txt1.26 KBanmolgoyal74
#4 3201370_4.patch1.17 KBanmolgoyal74
#3 3201370_1.patch1.11 KBhinal05
#3 3201370_after.png973.1 KBhinal05
#3 3201370_before.png924.24 KBhinal05
navigation_issue.gif4.24 MBhinal05

Comments

hinal05 created an issue. See original summary.

hinal05’s picture

hinal05 commented
Assigned: Unassigned » hinal05
Status: Active » Needs work
hinal05’s picture

hinal05 commented
Assigned: hinal05 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3201370_before.png924.24 KB
new 3201370_after.png973.1 KB
new 3201370_1.patch1.11 KB

I have created patch for this issue. Please review it.

anmolgoyal74’s picture

anmolgoyal74 commented
StatusFileSize
new 3201370_4.patch1.17 KB
new interdiff_3_4.txt1.26 KB
gauravvvv’s picture

gauravvvv commented
StatusFileSize
new Screenshot 2021-03-04 at 16.25.28.png137.79 KB
new Screenshot 2021-03-04 at 16.49.31.png36.52 KB

Patch #4, Looks good. Can we add a scroll track here, that is not visible. Need your input here @mherchel.

abhijith s’s picture

abhijith s commented
StatusFileSize
new 3201370-before.gif11.84 MB
new 3201370-after.gif9.64 MB

Applied patch #4 on 9.1.x.Scrollbar appeared after applying this patch.

Before patch:

before

After patch:

after

RTBC +1

bogdan.rizescu’s picture

bogdan.rizescu commented
Status: Needs review » Needs work
StatusFileSize
new before_patch.gif792.27 KB
new after_patch.gif746.69 KB

I have applied patch #4 and it partially solves the issue. As you can see in after_patch.gif when hovering inside-outside on Home Menu, the list with submenus tends to redimension to its natural size, without the scrollbar.

    Patch tested in:
  • - Chrome + Firefox
  • - 9.1.x + 9.2.x development branches of Drupal Core


Hovering inside-outside the Home Menu:
After applying patch #4

hinal05’s picture

hinal05 commented
Assigned: Unassigned » hinal05
hinal05’s picture

hinal05 commented
Assigned: hinal05 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3201370_09_before.gif3.77 MB
new 3201370_09_after.gif4.63 MB
new 3201370_09.patch5.94 KB

I have remove extra spacing between menus links for sub menu dropdown. Please review the patch and the GIF.

hinal05’s picture

hinal05 commented
StatusFileSize
new 3201370_10.patch5.95 KB
new interdiff_09_10.txt554 bytes

Please review updated patch.

Madhu kumar’s picture

Madhu kumar commented
StatusFileSize
new before_patch.png266.92 KB
new after_patch.png263.18 KB

Patch #4 applied cleanly and it is working well. Now we are able to see the scroll bar.

Before patch:

before-patch

After patch:

after

Madhu kumar’s picture

Madhu kumar commented
Status: Needs review » Reviewed & tested by the community
gauravvvv’s picture

gauravvvv commented

CI failed. Moving to NW

gauravvvv’s picture

gauravvvv commented
Status: Reviewed & tested by the community » Needs work
abhisekmazumdar’s picture

abhisekmazumdar
he/him
Primary language English
Location India
commented

Hi @hinal05

Thanks for the patch.

+++ b/core/themes/olivero/css/components/navigation/nav-primary-button.css
@@ -90,19 +90,14 @@
+      //margin-inline-end: -2.25rem

I would like to know is there any reason for commenting out this? Can't we remove this right away?

hinal05’s picture

hinal05 commented

Hi @abhisekmazumdar

Thanks for the reply.

The issue is that the hovering on inside or outside on Menu link the list with submenus is open. we just need to open dropdown on the hovering on inside the link text don't need to open outside the menu link.

As per this issue, need to remove menu link padding or set spacing properly because it was affecting on sub menu dropdown.

I have remove "margin-inline-end: -2.25rem" because After applied new spacing css we don't need this style any more.

I will create new issue for this.

hinal05’s picture

hinal05 commented
Status: Needs work » Needs review
StatusFileSize
new 3201370_before.gif2.19 MB
new 3201370_4_after.gif1.92 MB
new 3201370_17_after.gif4.8 MB
new 3201370_17.patch6.17 KB
new interdiff_4_17.txt5.89 KB

I have applied patch #4 and the sub navigation scrolling is working fine but after the implementing this patch getting one more issue regarding dropdown arrow. Arrow was hide after applied this patch So, I have created one more patch for this issue.

Please review the patch and screenshots and make sure after applying this patch need to clear the catch (command: drush cr).

sulfikar_s’s picture

sulfikar_s commented
StatusFileSize
new before_patch_17.mp4525.49 KB
new after_patch_17.mp4455.71 KB

Hi I've tested the #17 patch, and it fixes the main issue of the ticket as well as the arrow issue (mentioned on #17) & the issue mentioned on #7.

Attaching the before & after of the patch.

RTBC+1

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Closed (outdated)

This was fixed in #3190120: Olivero: Focused level-2 nav items should always be in viewport during keyboard navigation.