Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent horizontal jump as scrollbars appear #1230

Merged
merged 1 commit into from
Mar 4, 2019

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Mar 1, 2019

As discussed previously with @NickColley, we're seeing horizontal "jumps" in position as scroll bars appear and disappear in IE11—as page content grows.

Also spotted by @edwardhorsford here: #1204

We've fixed the issue manually (screen-only, not affecting print) by using:

@media only screen {
  .container {
    overflow-y: scroll;
  }
}

Also found in GOV.UK Template:
https://github.com/alphagov/govuk_template/blob/f2c10fae7ec7e371fd11c409cd71a399e3342e7f/source/assets/stylesheets/_basic.scss#L111

And inside GOV.UK Elements:
https://github.com/alphagov/govuk_elements/blob/84f485f688e7f796a4c44dd73b17177e8e39538c/assets/sass/elements/_govuk-template-base.scss#L50

Things to discuss:

1. Do we need to restrict to @media only screen
Does this prevent printable content from being hidden?

2. Do we need the old -ms-overflow-style: scrollbar style
To force the scrollbar to always display in IE10/11

Confirmed: Point 2) was previously required for "tablet mode" Internet Explorer that first came with Windows 8, but since scroll bars auto-hide by default it's not needed anymore.

Browser testing

  • Internet Explorer 8 (Windows)
  • Internet Explorer 9 (Windows)
  • Internet Explorer 10 (Windows)
  • Internet Explorer 11 (Windows)
  • Edge (latest 2 versions) (Windows)
  • Google Chrome (latest 2 versions) (Windows)
  • Mozilla Firefox (latest 2 versions) (Windows)
  • Safari 9+ (macOS)
  • Google Chrome (latest 2 versions) (macOS)
  • Mozilla Firefox (latest 2 versions) (macOS)
  • Safari 9.3+ (iOS)
  • Google Chrome (latest version) (iOS)
  • Google Chrome (latest version) (Android)
  • Samsung Internet (latest version) (Android)

Specific screenshots:

Edge 17 with fix

edge 17 with fix

Edge 17 without fix

edge 17 without fix

IE11 with fix

ie11 with fix

IE11 without fix

ie11 without fix

IE10 with fix

ie10 with fix

IE10 without fix

ie10 without fix

IE9 with fix

ie9 with fix

IE9 without fix

ie9 without fix

@edwardhorsford
Copy link
Contributor

Woop 🎉

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Mar 1, 2019

Further testing on Windows 8 (using the non-desktop "tablet mode" Internet Explorer) shows that scroll bars will hide by default without:

// Force IE10-11 scroll bar in "tablet" mode
-ms-overflow-style: scrollbar;

Added it back in.

That said, there is no visual jump here as tablet mode IE draws the scroll bars on top of the content before fading away automatically.

Could still remove this.

src/core/_template.scss Outdated Show resolved Hide resolved
@NickColley NickColley added the awaiting triage Needs triaging by team label Mar 4, 2019
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1230 March 4, 2019 10:27 Inactive
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Mar 4, 2019

I've done some more testing today of "tablet mode" on Windows 10.

In Microsoft Edge (and the touch-optimised Internet Explorer on Windows 8/8.1) scroll bars are designed to auto-hide without taking up any horizontal space. Just like iOS and macOS Safari.

Video demo showing auto-hiding scrollbars

YouTube example

I'd prefer to adhere to the device's defaults and retain this behaviour.

For example, on touch-first hardware running iOS, Android, Windows 10 in S mode or "tablet mode" we should prefer auto-hiding scroll bars (same with macOS with a trackpad or magic mouse).

So I've removed:

// Force IE10-11 scroll bar in "tablet" mode
-ms-overflow-style: scrollbar;

This pull now removes the horizontal jump in IE/Edge and when scroll bars are enabled on macOS, but without harming "tablet mode" IE/Edge and others with auto-hiding scroll bars.

@NickColley
Copy link
Contributor

NickColley commented Mar 4, 2019

Edge 18 with fix

image
image

Edge 18 without fix

image
image

@colinrotherham
Copy link
Contributor Author

Additional browser testing completed above #1230 (comment)

Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this in browsers too and can see nothing odd.

Given that this behaviour is also in the legacy GOV.UK Template project too, I think we should get this merged.

Will need a second review.

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work as ever. Thanks, @colinrotherham 👏

@36degrees 36degrees added this to the 2.8.0 milestone Mar 4, 2019
@dashouse dashouse self-requested a review March 4, 2019 13:51
Copy link

@dashouse dashouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks as always 👍

@NickColley NickColley merged commit ab71282 into alphagov:master Mar 4, 2019
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Mar 4, 2019

Since Firefox and Chrome on macOS will also obey the "Always" option when toggling "Show scroll bars", they also get the "visible but disabled" scroll bars—preventing the jump.

enable scrollbars

So we've prevented the visual jump in more than just IE:

Firefox 65 (with fix)

With scroll bars enabled, horizontal space is reserved

YouTube example

Firefox 65 (without fix)

With scroll bars enabled, horizontal space is not reserved

YouTube example

Browsers that auto-hide scroll bars are unaffected 👍

@colinrotherham
Copy link
Contributor Author

Thanks @NickColley @dashouse @36degrees 😊

@colinrotherham colinrotherham deleted the horizontal-scroll branch March 4, 2019 13:59
@36degrees 36degrees mentioned this pull request Mar 5, 2019
@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label Mar 20, 2019
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Mar 27, 2019
govuk-frontend v2.8.0 introduces an overflow-y: scroll for govuk-template that needs to be overwritten when the modal opens otherwise the content will scroll behind the modal. alphagov/govuk-frontend#1230
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Mar 28, 2019
govuk-frontend v2.8.0 introduces an overflow-y: scroll for govuk-template that needs to be overwritten when the modal opens otherwise the content will scroll behind the modal. alphagov/govuk-frontend#1230
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Mar 28, 2019
govuk-frontend v2.8.0 introduces an overflow-y: scroll for govuk-template that needs to be overwritten when the modal opens otherwise the content will scroll behind the modal. alphagov/govuk-frontend#1230
@alex-ju
Copy link
Contributor

alex-ju commented Mar 28, 2019

Quick question: does anyone know if it's possible to achieve the same effect by moving this logic to the <body> instead of the <html> element? Could try it myself, but I'm curious to know if there's a reason for doing it on the special <html> element.

@colinrotherham
Copy link
Contributor Author

@alex-ju I think the <body> tag would work just as well.

By using <html> we copied what GOV.UK and the old alphagov/govuk_elements kit did which we know is tried and testing in various services already.

@alex-ju
Copy link
Contributor

alex-ju commented Mar 28, 2019

@colinrotherham ok, thanks for clarifying this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants