Automattic / _s

Hi. I'm a starter theme called _s, or underscores, if you like. I'm a theme meant for hacking so don't use me as a Parent Theme. Instead try turning me into the next, most awesome, WordPress theme out there. That's what I'm here for.
http://underscores.me/
GNU General Public License v2.0
10.93k stars 3.12k forks source link

Navigation focus not working in Safari (macOS); dropdowns only open on hover #1441

Open stevygee opened 4 years ago

stevygee commented 4 years ago

It seems the .focus class isn't being set on menu items in Safari on desktop (works fine on mobile). This becomes apparent when you don't want to open submenus on hover, just on click, by commenting out these lines in the CSS: https://github.com/Automattic/_s/blob/e78a8088975b868715eb084831d0ac7c6cec2571/sass/components/navigation/_navigation.scss#L26 https://github.com/Automattic/_s/blob/e78a8088975b868715eb084831d0ac7c6cec2571/sass/components/navigation/_navigation.scss#L37 Now the submenus won't open at all, even when clicking on the parent item.

Found out that Safari doesn't set focus on links, so these events never fire: https://github.com/Automattic/_s/blob/e78a8088975b868715eb084831d0ac7c6cec2571/js/navigation.js#L63-L64

I've spent a couple of hours today trying to come up with a workaround involving an additional click listener. But it's tricky getting it to work without removing the focus/blur event handling in other browsers. Maybe emulating focus isn't a good idea at all? Also tried to use :focus-within, but this also doesn't work with links in Safari.

I might take another stab at it, so in case I'll find a good solution, I'll open a pull request.

Ismail-elkorchi commented 4 years ago

Hi @stevygee, and thank you for the issue report.

I just tried out the instructions you described, and I am unable to reproduce this issue. Using Option+Tab, I can set focus on links as illustrated in the screenshot below :

Capture d’écran 2020-07-13 à 23 29 49

Even when commenting the lines you indicated in _navigation.scss I still can open the submenu by hover.

stevygee commented 4 years ago

@Ismail-elkorchi sorry it took this long to get back to you! Thank you for trying to reproduce my issue. You're right, focus by tabbing works. However, I want to open the submenu by click instead of hover, so I need to focus the parent element by clicking. I was able to come up with a fix that seems to do the job: https://github.com/stevygee/_s/commit/f716bed856f32ee309466fde33d48a790f0395e2

elsmore commented 2 years ago

@stevygee I'm having the same issue - the focus class is not set on desktop Safari when a subnav is clicked. Would you mind sharing your fix again? The link doesn't work.

stevygee commented 2 years ago

@elsmore Had to do some digging, but apparently this was the fix: https://www.diffchecker.com/fOAD1lQ8

elsmore commented 2 years ago

@stevygee Thank you so much, that has helped me a lot!

stevygee commented 2 years ago

@elsmore Glad I could help!