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

Add helper for hiding global bar #2364

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

injms
Copy link
Contributor

@injms injms commented Apr 21, 2021

What

Adds a helper that checks whether a page should hide the global bar; the view can then add a class to the body element that will hide the global bar.

These changes won't have any effect until Static has been updated as Static needs to contain the CSS for this to have any impact. Static needs to be updated to flip the behaviour from hide by default to show by default.

Why

Currently the global bar is initially hidden, then shown using JavaScript - this causes a shift in the layout of the page (aka jank aka Cumulative Layout Shift) and means that the global bar is not available for users without JavaScript.

(More about Cumulative Layout Shift on web.dev.)

CLS

To avoid this, a change will need to be made in Static to the make the global bar be initially shown by default. The bar can then be hidden on pages that don't need it - and doing this using CSS will avoid jank.

The screenshot below shows the layout shift that occurs when the global bar appears:

image

As the search form is present, it's frustrating for users when they select a link only for the layout shift to move the page and a different link to be selected.

This is also present on larger screens, though the effect is dwarfed by the CLS caused by the appearance of the cookie banner:

image

Worth bearing in mind that for users that have made a cookie choice, the CLS will only be due to the global bar.

No JavaScript

And for users without JavaScript turned on, the global bar does not appear at all:

image

But why does this change involve collections?

The global bar should not appear on certain pages - such as the Brexit or Coronavirus landing pages. Previously there was some JavaScript to prevent the global bar from appearing. This has been changed to an helper in the application that can be used to add a class to the body if the global should not appear.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

@bevanloon bevanloon temporarily deployed to govuk-collec-update-glo-2rejb9 April 21, 2021 11:03 Inactive
@injms injms force-pushed the update-global-bar-behaviour branch from b9f97b4 to 84eff48 Compare April 21, 2021 11:04
@bevanloon bevanloon temporarily deployed to govuk-collec-update-glo-2rejb9 April 21, 2021 11:04 Inactive
@injms injms force-pushed the update-global-bar-behaviour branch from 84eff48 to d45685f Compare April 21, 2021 14:37
@bevanloon bevanloon temporarily deployed to govuk-collec-update-glo-2rejb9 April 21, 2021 14:37 Inactive
Currently the global bar is initially hidden, then shown using JavaScript - this
causes a shift in the layout of the page (aka jank) and means that the global
bar is not available for users without JavaScript.

To avoid this, a change will need to be made in Static to the make the global
bar be initially shown by default. The bar can then be hidden on pages that
don't need it - and doing this using CSS will avoid jank.

This change adds a helper that checks whether a page should hide the global bar;
the view can then add a class to the body element that will hide the global bar.

These changes won't have any effect until Static has been updated.
@injms injms force-pushed the update-global-bar-behaviour branch from d45685f to fe9ccc3 Compare October 18, 2021 16:38
Comment on lines +72 to +76
"^/coronavirus$",
"^/coronavirus/.*$",
"^/transition(.cy)?$",
"^/transition-check/.*$",
"^/eubusiness(\\..*)?$",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self - check if these paths need updating.

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.

2 participants