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

Amend GOVUK.stickAtTopWhenScrolling to "unstick" for smaller screens #329

Merged
merged 3 commits into from
Oct 7, 2016

Conversation

gemmaleigh
Copy link
Contributor

@gemmaleigh gemmaleigh commented Sep 20, 2016

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:

screen shot 2016-09-20 at 17 57 03

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.
@NickColley
Copy link
Contributor

Is there somewhere I can test this in a browser?

@gemmaleigh
Copy link
Contributor Author

@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.
@gemmaleigh
Copy link
Contributor Author

gemmaleigh commented Sep 21, 2016

Hi @NickColley, there's a branch for service-manual-frontend that demos this.

@gemmaleigh
Copy link
Contributor Author

Stick at top when scrolling:

sticky

Release on resize (smaller than 768px):

resize

@36degrees
Copy link
Contributor

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 if(...){, if (...){, and if (...) { and there's a mix of function () {, function() { and function(){

@NickColley
Copy link
Contributor

Yeah we're trying to avoid specific conversations around syntax and rather focus on bringing project linting across the whole repo (#334)

@robinwhittleton
Copy link
Contributor

I’m happy with the code and approach. If someone else (@36degrees?) could OK the behaviour then I’m fine with it being merged.

@36degrees
Copy link
Contributor

Had a play with it and it seems to work as expected. Good stuff!

There are a few really picky edge cases:

  • If you scroll really quickly to e.g. the top of the document it doesn't quite keep up and you can end up with the wrong thing highlighted.
  • If you have a very short section at the end of your document it'll never become active as the previous heading doesn't leave the screen. This is especially odd if you click the heading to scroll down as the previous link becomes active instead.

However I think those really are edge cases and I'd be happy for this to go out as is.

@gemmaleigh
Copy link
Contributor Author

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.

@gemmaleigh
Copy link
Contributor Author

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.

@36degrees
Copy link
Contributor

Sorry, yes. I knew that wasn't part of these changes, yet still commented here. I blame my weary Friday brain.

@gemmaleigh
Copy link
Contributor Author

I'm going to go ahead and merge as @36degrees doesn't have permission to merge this.

@gemmaleigh gemmaleigh merged commit 035c4c1 into master Oct 7, 2016
@gemmaleigh gemmaleigh deleted the fix-stick-top-when-scrolling branch October 7, 2016 13:23
gemmaleigh added a commit that referenced this pull request Oct 7, 2016
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).
@gemmaleigh gemmaleigh mentioned this pull request Oct 7, 2016
gemmaleigh added a commit that referenced this pull request Oct 7, 2016
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).
@gemmaleigh gemmaleigh mentioned this pull request Oct 7, 2016
gemmaleigh added a commit to alphagov/service-manual-frontend that referenced this pull request Oct 7, 2016
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.
gemmaleigh added a commit to alphagov/service-manual-frontend that referenced this pull request Oct 8, 2016
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.
gemmaleigh added a commit to alphagov/service-manual-frontend that referenced this pull request Oct 8, 2016
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.
gemmaleigh added a commit to alphagov/govuk-prototype-kit that referenced this pull request Oct 13, 2016
# 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))
Guntrisoft added a commit to guidance-guarantee-programme/pension_guidance that referenced this pull request Dec 2, 2016
- 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
Guntrisoft added a commit to guidance-guarantee-programme/pension_guidance that referenced this pull request Dec 2, 2016
- 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
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