This is probably a regression from when we converted to CSS logical properties.

The primary menu's hover states are not correct at wide widths.

  • The width of the underline is too small
  • It grows from the left instead of the center.

Current state

Approved Proof of Concept mockup (https://olivero-poc.netlify.app/)

CommentFileSizeAuthor
#10 Screen Recording 2021-08-12 at 10.55.13 AM.mov3.61 MBGauravvvv
#9 3226704-9.patch2.34 KBmherchel
#9 interdiff-2-9.patch1.46 KBmherchel
#8 hover-current-2-level.gif162.51 KBmherchel
#6 hover-current.gif290.39 KBmherchel
#5 Screen Shot 2021-08-09 at 16.55.27.png9.4 KBlauriii
#3 3226704-after_patch.gif3.79 MBAbhijith S
#3 3226704-before_patch.gif4.89 MBAbhijith S
#2 3226704.patch1.54 KBmherchel
hover-poc.gif31.69 KBmherchel
hover-current.gif97.56 KBmherchel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mherchel created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Active » Needs review
FileSize
3226704.patch1.54 KB
Abhijith S’s picture

Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
FileSize
3226704-before_patch.gif4.89 MB
3226704-after_patch.gif3.79 MB

Applied patch #2 and it works fine.The width of underline on hover is increased and also the line grows from the center now. Adding screen recordings below.

Before patch:
before

After patch:
after

RTBC +1

IndrajithKB’s picture

IndrajithKB CreditAttribution: IndrajithKB at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs review » Reviewed & tested by the community

Hi @mherchel thanks for the patch, and thanks @Abhijith S for the review.
"border-top-width increased from 5px to 9px, and line grows changed from left to center" which is the expected result so am just moving the status of the issue from NR to RTBC.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
FileSize
Screen Shot 2021-08-09 at 16.55.27.png9.4 KB

It's a bit strange that the border hides most of the arrow from 2nd level menu items. Any thoughts on this?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
FileSize
hover-current.gif290.39 KB

The submenu should be placed above the hover state. Is this not the case for you?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented

That seems to be caused by having one menu item that is wrapped on two lines. It's working as expected when there aren't menu items wrapped on multiple lines.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
FileSize
hover-current-2-level.gif162.51 KB

@lauriii I'm still unable to reproduce this. Does this issue happen without the patch? Animated gif attached.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
FileSize
interdiff-2-9.patch1.46 KB
3226704-9.patch2.34 KB

Ahh I see that happens if another menu item has two lines.

IMO I'm indifferent about this, as I don't expect to see this in the real world. That being said, it's easy enough to fix.

This patch removes the hover state underline for menu items that have children.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at OpenSense Labs for Drupal India Association commented
Status: Needs review » Reviewed & tested by the community
FileSize
Screen Recording 2021-08-12 at 10.55.13 AM.mov3.61 MB

Patch #9, Fixes the issue. Border have been removed for the menu items having submenu's.

Added before and after patch recording.

RTBC+1

  • lauriii committed fa0b720 on 9.3.x 
    Issue #3226704 by mherchel, Abhijith S, lauriii, Gauravmahlawat,...

  • lauriii committed d64fb54 on 9.2.x 
    Issue #3226704 by mherchel, Abhijith S, lauriii, Gauravmahlawat,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed fa0b720 and pushed to 9.3.x. Also 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.