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

Use CSS sticky on HTML publications instead of custom script #2493

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Jul 20, 2022

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

What

Stop using sticky-element-container.js on HTML publication pages and instead use native CSS sticky.

The page layout has a contents list in the left hand column and the main content in the right. The script was used to keep a jump link to the contents (and a print this page button) visible at all times when scrolling, so users can jump back to the top at any time.

This change switches from using the script to the native CSS position: sticky attribute. It has been applied to the contents list, so now the entire contents list is visible in the left column when the page is scrolled. It also stops in the right position automatically when reaching the bottom.

Internet Explorer doesn't support the CSS sticky property, so this doesn't work in that browser. It fails gracefully though, in that nothing bad happens.

Behaviour on mobile remains the same - contents stack at top, link at bottom jumps back to top, no sticky.

Why

sticky-element-container.js isn't perfect - it has a bug under certain conditions that makes the sticky element overlap the footer. It's also very complicated, and something we'd like to retire, but it's used on another page still so it's still needed.

I've kept the 'Contents' jump link at the foot of the page but renamed it to 'Back to top', as that is more of an accurate description. However now that the whole contents menu scrolls with the page it's possible that we don't need this link at all (except possibly for Internet Explorer users).

Visual changes

Before After
Initial state at the top of the page (unchanged) Initial state at the top of the page (unchanged)
Screenshot 2022-07-20 at 11 50 05 Screenshot 2022-07-20 at 12 43 53
When scrolling down - just contents link and print button visible When scrolling down - whole contents menu and print button visible
Screenshot 2022-07-20 at 11 50 17 Screenshot 2022-07-20 at 12 44 02
At the bottom - print button and contents link stop At the bottom - contents stop, back to top link replaces contents link (but does the same thing)
Screenshot 2022-07-20 at 11 50 28 Screenshot 2022-07-20 at 12 44 15

- we've got a custom script on publications like /government/publications/heatwave-plan-for-england/beat-the-heat-staying-safe-in-hot-weather that keeps the contents list in the sidebar on the right as the page is scrolled
- this script has a bug that lets the contents overlap the content at the bottom of the page in some situations
- and we don't need the complexity of the JS now that CSS can do this natively
- this isn't supported in Internet Explorer, but it fails gracefully
- separate out the sidebar-with-body class from the grid row elements, so that the contents link at the bottom of the page is inside it and the margin bottom of sidebar-with-body comes beneath the contents link, not above it
- was 'contents' because it linked to the contents
- however now the contents are sticky, can have this link and the contents on the same page, so 'Back to top' seems more appropriate
- would remove altogether execept sticky isn't supported in IE so probably still useful to IE users
@andysellick andysellick requested a review from injms July 20, 2022 15:46
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2493 August 2, 2022 09:02 Inactive
- if the height of the contents list exceeds the height of the viewport users can't see the whole contents unless they scroll to the bottom of the page, where the sticky element hits the bottom and stops
- to fix this, we apply a max height of the viewport and an overflow y scroll, so that the element never exceeds the height of the viewport and can be scrolled
- also adding a bottom margin to prevent collision with the back to top link at the bottom
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2493 August 2, 2022 10:18 Inactive
Copy link
Contributor

@injms injms left a comment

Choose a reason for hiding this comment

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

Nice update - thank you!

@DilwoarH
Copy link
Contributor

@andysellick - does this fix #2056 as well?

@andysellick
Copy link
Contributor Author

@andysellick - does this fix #2056 as well?

@DilwoarH yes, I believe so.

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.

4 participants