alphagov / govuk_publishing_components

A gem to document and distribute frontend components for GOV.UK applications
https://components.publishing.service.gov.uk
MIT License
66 stars 20 forks source link

Bizarre behaviour on super navigation header when switching from tablet to desktop on safari #2430

Closed owenatgov closed 3 years ago

owenatgov commented 3 years ago

When on safari and swapping from tablet to desktop views, a bizarre behaviour is exhibited by the super navigation header:

https://user-images.githubusercontent.com/64783893/141271839-791dcc40-7f2e-4d6e-81cc-933544c9cd84.mov

Summary of screen recording:

injms commented 3 years ago

I've tried in Safari v15.1 and v15.4 - and can't get it to do this in either of those versions, whether I'm resizing the devtools or the window itself.

Which version of Safari did you find the bug in?

owenatgov commented 3 years ago

@injms I'm on 14.1.2 Which makes me think this might be a version problem. What are the stats on safari usage/do you know where I can check out some stats?

owenatgov commented 3 years ago

From Ian via slack:

Safari 14.1.2 on macOS (because google analytics helpfully lumps iOS and macOS Safari together) saw 1,139,741 tracked users in October (~1.8% of total users)

owenatgov commented 3 years ago

I've been able to get this to happen consistently on my local safari. I'm not able to replicate this on the same version on browser stack. Have updated the description of the issue.

injms commented 3 years ago

So after quite a bit of head scratching, @owenatgov and I have managed to find out what cause this layout bug.

When using a non-Apple mouse and if the system preferences are set to "Show scoll bars: automatically based on mouse or trackpad" Safari will always show the scroll bar. This causes a 15px difference between the width that triggers the CSS media query and the width reported by window.innerWidth.

As the navigation header uses window.innerWidth, this means that the CSS triggers a layout change at a different screen width to when the JavaScript hides or shows any elements - which causes the mismatch seen in the video.

https://github.com/alphagov/govuk_publishing_components/blob/c576f385edd75112c65ea5bfaa2d15721a9f35ce/app/assets/javascripts/govuk_publishing_components/components/layout-super-navigation-header.js#L104-L106

A bit of investigation is needed to find a robust way of reporting the screen width to ensure that this mismatch doesn't occur.

injms commented 3 years ago

MDN has a good explanation of window.innerWIdth - and this bit is especially interesting:

If you need to obtain the width of the window minus the scrollbar and borders, use the root element's clientWidth property instead.

Using clientWidth on the root html element is a special case in the spec - it returns the width of the viewport, not the width of the html element.

Digressions aside, I think using document.documentElement.clientWidth instead of window.innerWidth will fix this layout glitch.