-
Notifications
You must be signed in to change notification settings - Fork 104
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
Amend GOVUK.stickAtTopWhenScrolling to "unstick" for smaller screens #329
Conversation
Add three new functions and update the stick at top when scrolling js to use these new functions, so they can be mocked in tests.
0ec4065
to
76a5308
Compare
Is there somewhere I can test this in a browser? |
@NickColley I'm going to duplicate this over in service-manual-frontend to test it works as expected, I'll let you know when it's possible to preview. |
Add a checkResize function to check when the screen is resized and “unstick” a sticky element if the window width is smaller than “desktop” - less than or equal to 768px.
Re-order tests - when sticking and unsticking. Add mocks to test that sticking and unsticking works for smaller and larger screens, test that all three conditions in the checkScroll function stick/unstick.
9f8c557
to
76c62bb
Compare
Hi @NickColley, there's a branch for service-manual-frontend that demos this. |
A little pedantic, but stick-at-top-when-scrolling.js could do with some love in terms of consistent spacing around control structures and function definitions. For example, there's a mix of |
Yeah we're trying to avoid specific conversations around syntax and rather focus on bringing project linting across the whole repo (#334) |
I’m happy with the code and approach. If someone else (@36degrees?) could OK the behaviour then I’m fine with it being merged. |
Had a play with it and it seems to work as expected. Good stuff! There are a few really picky edge cases:
However I think those really are edge cases and I'd be happy for this to go out as is. |
Hi @36degrees, thanks! We might want to keep those comments with the HighlightActiveSection JS - over here. This PR is amending the "unsticking" (removing the position:fixed set for a sticky element) by checking on resize to see if the sticky element needs to be unstuck for smaller screens. This is to avoid the situation where the page content on the right overlaps the fixed position nav on the left, before the viewport is narrow enough to collapse to a single column layout. |
If you're happy to merge this - then I'll bump the version of the frontend toolkit in a separate PR, so we can pull these changes into service manual frontend. |
Sorry, yes. I knew that wasn't part of these changes, yet still commented here. I blame my weary Friday brain. |
I'm going to go ahead and merge as @36degrees doesn't have permission to merge this. |
Version 4.19.0 adds the following: [PR #329](#329) Amend GOVUK.StickAtTopWhenScrolling to check on resize if an element should be “unstuck”. For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim).
Version 4.18.3 adds the following: [PR #329](#329) Amend GOVUK.StickAtTopWhenScrolling to check on resize if an element should be “unstuck”. For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim).
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG .md # 4.18.3 - For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). ([PR #329](alphagov/govuk_frontend_toolkit#329)) # 4.18.2 - Remove unnecessary print font fallback that causes regression downstream ([PR #328](alphagov/govuk_frontend_toolkit#328)) - Update tooling to use npm script rather than grunt-shell ([PR #327](alphagov/govuk_frontend_toolkit#327)) # 4.18.1 - Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript ([PR #323](alphagov/govuk_frontend_toolkit#323)). # 4.18.0 - Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes ([PR #315](alphagov/govuk_frontend_toolkit#315)). # 4.17.0 - SelectionButtons will add a class to the label with the type of the child input ([PR #317](alphagov/govuk_frontend_toolkit#317)) # 4.16.1 - Fix anchor-buttons.js to trigger a native click event, also rename to shimLinksWithButtonRole.js to explain what it does - Add tests for shimLinksWithButtonRole ([PR #310](alphagov/govuk_frontend_toolkit#310)) # 4.16.0 - Add Department for International Trade organisation ([PR #308](alphagov/govuk_frontend_toolkit#308)) # 4.15.0 - Add support for Google Analytics fieldsObject ([PR #298](alphagov/govuk_frontend_toolkit#298)) - anchor-buttons.js: normalise keyboard behaviour between buttons and links with a button role ([PR #297](alphagov/govuk_frontend_toolkit#297)) # 4.14.1 - Fix tabular number sizing in Firefox ([PR #301](alphagov/govuk_frontend_toolkit#301)) # 4.14.0 - Allow use of multiple GA customDimensionIndex. See [this section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/ docs/javascript.md#using-google-custom-dimensions-with-your-own-statisti cal-model) of the documentation for more information. - Configurable duration (in days) for AB Test cookie. See [this section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/ docs/javascript.md#multivariate-test-framework) of the documentation for more information. - Allow base scripts to run within a module loader. See [this PR](alphagov/govuk_frontend_toolkit#290) for more information. # 4.13.0 - Make headings block-level by default (PR #200). If you are styling elements you want to be inline with heading includes, you’ll need to explicitly make them inline in your CSS. # 4.12.0 - Increase button padding to match padding from GOV.UK elements (PR #275). If you have UI which depends on the padding set by the button mixin in the frontend toolkit and this is not overridden by button padding set by GOV.UK elements, this change will affect it. # 4.11.0 - Remove the GDS-Logo font-face definition (PR #272) - Move the @Viewport statements to govuk_template (PR #272). If you upgrade to this version of govuk_frontend_toolkit and you’re also using govuk_template you’ll need to upgrade that to at least 0.17.2 to maintain compatibility with desktop IE10 in snap mode.
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG .md 4.18.3 - For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). ([PR 329](alphagov/govuk_frontend_toolkit#329)) 4.18.2 - Remove unnecessary print font fallback that causes regression downstream ([PR 328](alphagov/govuk_frontend_toolkit#328)) - Update tooling to use npm script rather than grunt-shell ([PR 327](alphagov/govuk_frontend_toolkit#327)) 4.18.1 - Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript ([PR 323](alphagov/govuk_frontend_toolkit#323)). 4.18.0 - Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes ([PR 315](alphagov/govuk_frontend_toolkit#315)). 4.17.0 - SelectionButtons will add a class to the label with the type of the child input ([PR 317](alphagov/govuk_frontend_toolkit#317)) 4.16.1 - Fix anchor-buttons.js to trigger a native click event, also rename to shimLinksWithButtonRole.js to explain what it does - Add tests for shimLinksWithButtonRole ([PR 310](alphagov/govuk_frontend_toolkit#310)) 4.16.0 - Add Department for International Trade organisation ([PR 308](alphagov/govuk_frontend_toolkit#308)) 4.15.0 - Add support for Google Analytics fieldsObject ([PR 298](alphagov/govuk_frontend_toolkit#298)) - anchor-buttons.js: normalise keyboard behaviour between buttons and links with a button role ([PR 297](alphagov/govuk_frontend_toolkit#297)) 4.14.1 - Fix tabular number sizing in Firefox ([PR 301](alphagov/govuk_frontend_toolkit#301)) 4.14.0 - Allow use of multiple GA customDimensionIndex. See [this section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/ docs/javascript.md#using-google-custom-dimensions-with-your-own-statisti cal-model) of the documentation for more information. - Configurable duration (in days) for AB Test cookie. See [this section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/ docs/javascript.md#multivariate-test-framework) of the documentation for more information. - Allow base scripts to run within a module loader. See [this PR](alphagov/govuk_frontend_toolkit#290) for more information. 4.13.0 - Make headings block-level by default (PR #200). If you are styling elements you want to be inline with heading includes, you’ll need to explicitly make them inline in your CSS. 4.12.0 - Increase button padding to match padding from GOV.UK elements (PR 275). If you have UI which depends on the padding set by the button mixin in the frontend toolkit and this is not overridden by button padding set by GOV.UK elements, this change will affect it. 4.11.0 - Remove the GDS-Logo font-face definition (PR #272) - Move the @Viewport statements to govuk_template (PR #272). If you upgrade to this version of govuk_frontend_toolkit and you’re also using govuk_template you’ll need to upgrade that to at least 0.17.2 to maintain compatibility with desktop IE10 in snap mode.
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG .md 4.18.3 - For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). ([PR 329](alphagov/govuk_frontend_toolkit#329)) 4.18.2 - Remove unnecessary print font fallback that causes regression downstream ([PR 328](alphagov/govuk_frontend_toolkit#328)) - Update tooling to use npm script rather than grunt-shell ([PR 327](alphagov/govuk_frontend_toolkit#327)) 4.18.1 - Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript ([PR 323](alphagov/govuk_frontend_toolkit#323)). 4.18.0 - Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes ([PR 315](alphagov/govuk_frontend_toolkit#315)). 4.17.0 - SelectionButtons will add a class to the label with the type of the child input ([PR 317](alphagov/govuk_frontend_toolkit#317)) 4.16.1 - Fix anchor-buttons.js to trigger a native click event, also rename to shimLinksWithButtonRole.js to explain what it does - Add tests for shimLinksWithButtonRole ([PR 310](alphagov/govuk_frontend_toolkit#310)) 4.16.0 - Add Department for International Trade organisation ([PR 308](alphagov/govuk_frontend_toolkit#308)) 4.15.0 - Add support for Google Analytics fieldsObject ([PR 298](alphagov/govuk_frontend_toolkit#298)) - anchor-buttons.js: normalise keyboard behaviour between buttons and links with a button role ([PR 297](alphagov/govuk_frontend_toolkit#297)) 4.14.1 - Fix tabular number sizing in Firefox ([PR 301](alphagov/govuk_frontend_toolkit#301)) 4.14.0 - Allow use of multiple GA customDimensionIndex. See [this section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/ docs/javascript.md#using-google-custom-dimensions-with-your-own-statisti cal-model) of the documentation for more information. - Configurable duration (in days) for AB Test cookie. See [this section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/ docs/javascript.md#multivariate-test-framework) of the documentation for more information. - Allow base scripts to run within a module loader. See [this PR](alphagov/govuk_frontend_toolkit#290) for more information. 4.13.0 - Make headings block-level by default (PR #200). If you are styling elements you want to be inline with heading includes, you’ll need to explicitly make them inline in your CSS. 4.12.0 - Increase button padding to match padding from GOV.UK elements (PR 275). If you have UI which depends on the padding set by the button mixin in the frontend toolkit and this is not overridden by button padding set by GOV.UK elements, this change will affect it. 4.11.0 - Remove the GDS-Logo font-face definition (PR #272) - Move the @Viewport statements to govuk_template (PR #272). If you upgrade to this version of govuk_frontend_toolkit and you’re also using govuk_template you’ll need to upgrade that to at least 0.17.2 to maintain compatibility with desktop IE10 in snap mode.
# 4.18.3 - For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). ([PR #329](alphagov/govuk_frontend_toolkit#329))
- Removed phase-banner scss file as no longer needed Full list of changes: SelectionButtons will add a class to the label with the type of the child input alphagov/govuk_frontend_toolkit#317 Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes alphagov/govuk_frontend_toolkit#315 Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript alphagov/govuk_frontend_toolkit#323 Remove unnecessary print font fallback that causes regression downstream alphagov/govuk_frontend_toolkit#328 Update tooling to use npm script rather than grunt-shell alphagov/govuk_frontend_toolkit#327 For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). alphagov/govuk_frontend_toolkit#329 Lint codebase using standard alphagov/govuk_frontend_toolkit#334 Add semicolons at the start of IIFE's alphagov/govuk_frontend_toolkit#339 Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase alphagov/govuk_frontend_toolkit#293 Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon alphagov/govuk_frontend_toolkit#345 Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set alphagov/govuk_frontend_toolkit#343 Allow custom options in GOVUK.analytics.trackPageview alphagov/govuk_frontend_toolkit#332 Fix role="button" click shim alphagov/govuk_frontend_toolkit#347 Make font variables lowercase alphagov/govuk_frontend_toolkit#348 Update selection button documentation alphagov/govuk_frontend_toolkit#350 Change colour used in phase tags to govuk-blue alphagov/govuk_frontend_toolkit#353
- Removed phase-banner scss file as no longer needed Full list of changes: SelectionButtons will add a class to the label with the type of the child input alphagov/govuk_frontend_toolkit#317 Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes alphagov/govuk_frontend_toolkit#315 Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript alphagov/govuk_frontend_toolkit#323 Remove unnecessary print font fallback that causes regression downstream alphagov/govuk_frontend_toolkit#328 Update tooling to use npm script rather than grunt-shell alphagov/govuk_frontend_toolkit#327 For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim). alphagov/govuk_frontend_toolkit#329 Lint codebase using standard alphagov/govuk_frontend_toolkit#334 Add semicolons at the start of IIFE's alphagov/govuk_frontend_toolkit#339 Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase alphagov/govuk_frontend_toolkit#293 Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon alphagov/govuk_frontend_toolkit#345 Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set alphagov/govuk_frontend_toolkit#343 Allow custom options in GOVUK.analytics.trackPageview alphagov/govuk_frontend_toolkit#332 Fix role="button" click shim alphagov/govuk_frontend_toolkit#347 Make font variables lowercase alphagov/govuk_frontend_toolkit#348 Update selection button documentation alphagov/govuk_frontend_toolkit#350 Change colour used in phase tags to govuk-blue alphagov/govuk_frontend_toolkit#353
For smaller screens (<768px) ensure that the element which was previously "stuck" is "unstuck", (removing both the class which sets fixed positioning and the shim).
This PR adds mocks for the window scroll position, offset of the sticky element and the window dimensions and updates stickAtTopWhenScrolling to use these.
It adds new tests to ensure that the three conditions of checkScroll are met, to "stick" or "unstick (release)" a sticky element.
Tests: