Split from #3191682: Ensure that Olivero's focus states are visible and accessible in Windows High Contrast mode

In IE11, when the submenu has focus, a horizontal scrollbar is present.

CommentFileSizeAuthor
#10 IE11_-_Win7__before_upgrading_ie___Running_-6.png331.92 KBmherchel
#8 3223268-8.patch1.25 KBjavi-er
#6 3223268-6.patch1.22 KBdhirendra.mishra
#6 interdiff_4-6.txt510 bytesdhirendra.mishra
#4 3223268-4-primary-menu-submenus-scrollbar-focus.patch1.22 KBjavi-er
#4 IE11_-_after_patch_4.png1.05 MBjavi-er
IE11_-_Win7__before_upgrading_ie___Running_.png340.47 KBmherchel

Comments

mherchel created an issue. See original summary.

mherchel credited javi-er.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Crediting @javi-er, who found the bug

javi-er’s picture

javi-er commented
Status: Active » Needs review
StatusFileSize
new IE11_-_after_patch_4.png1.05 MB
new 3223268-4-primary-menu-submenus-scrollbar-focus.patch1.22 KB

The problem seems to be that in IE11 the position absolute + translate + width of submenu items :focus:before element is triggering the scrollbars.

I simplified the :focus:before logic for second level items which seems to have solved the issue, I tested this on Chrome, Firefox, Safari (mac), IE11 on Win10 and Edge 91 on Win10

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

This is looking good! There's a couple of issues. The patch isn't passing core's CSS linting. You can run this locally by running yarn lint:css. It's probably just an order of properties thing. 

+++ b/core/themes/olivero/css/components/navigation/nav-primary-wide.pcss.css
@@ -148,6 +148,9 @@ body:not(.is-always-mobile-nav) {
+       left: calc(var(--sp) * -1);

This should be left: calc(var(--sp0-5) * -1);. On line 34, we're setting width: calc(100% + var(--sp));, and we only want to evenly distribute the spacing on both the left and right side (right now, it's all distributed on the left).

dhirendra.mishra’s picture

dhirendra.mishra commented
Status: Needs work » Needs review
StatusFileSize
new interdiff_4-6.txt510 bytes
new 3223268-6.patch1.22 KB

Here is the fix from #5 comment.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

Last patch didn't pass tests

javi-er’s picture

javi-er commented
StatusFileSize
new 3223268-8.patch1.25 KB

Thanks @mherchel, here's the patch at #6 with css linting applied, it's passing the tests on my local now.

javi-er’s picture

javi-er commented
Status: Needs work » Needs review
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new IE11_-_Win7__before_upgrading_ie___Running_-6.png331.92 KB

Looks perfect! Thanks for working on it!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

This is ready to be committed after the code freeze 👍

  • lauriii committed 0c9dfc7 on 9.3.x 
    Issue #3223268 by javi-er, dhirendra.mishra, mherchel: Olivero: IE11...

  • lauriii committed f671866 on 9.2.x 
    Issue #3223268 by javi-er, dhirendra.mishra, mherchel: Olivero: IE11...
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

Committed 0c9dfc7 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.