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 GOVUK.ShowHideContent JavaScript #315

Merged
merged 12 commits into from
Sep 5, 2016

Conversation

gemmaleigh
Copy link
Contributor

Add GOVUK.ShowHideContent to toolkit, this is copied from #305.
This PR is rebased against master and has the conflicts from #305 resolved.

A link is added to the README to the documentation.
Standard JS has been used to lint the JS.

cc. @tombye, @NickColley.

this.$content = $(

// Radio buttons (yes/no)
'<form class="js-toggle-showhide">' +
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this js-toggle-showhide class needed? I can't see it used in the tests or the module code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot! Thanks @tombye, I've removed those.

This reduces the radios we are trying to hide the
content for to only be those which are controls.

This covers the following use cases:

1. If the control sent in is not a control

All content controlled by radios in this group
will be hidden.

2. If the control sent in is a control

All content controlled by radios in this group
will be hidden and the content for the control
will be shown.
The following errors came up:

- 80:54: Block must not be padded by blank lines.
- 105:12: Closing curly brace does not appear on
  the same line as the subsequent block.
@tombye tombye force-pushed the add-govuk-show-hide-content-js branch from 2d97aac to 9d9b3c6 Compare August 23, 2016 12:58
// All radios in this group
var selector = selectors.radio + '[name=' + escapeElementName($control.attr('name')) + ']'
// All radios in this group which control content
var selector = selectors.radio + '[name=' + escapeElementName($control.attr('name')) + '][aria-controls]'
Copy link
Contributor

@colinrotherham colinrotherham Aug 23, 2016

Choose a reason for hiding this comment

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

Hi @tombye, @gemmaleigh.

This has a side effect unfortunately… Now only controlling radios can be used to show/hide content.

Imagine a yes or no question and answer form, for example "Do you have a partner"?

Previously the "Yes" radio (controlling) could toggle the content open with further content, and the "No" radio in the same name group (not controlling) could toggle the content closed again.

With this latest change, how do you close it once it's open?

Copy link
Contributor

Choose a reason for hiding this comment

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

example

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure the method you show/hide content for radio groups works with the change in that scenario:

If you click 'yes', all content controlled by the group would be hidden by the loop on line 87. The condition on line 92 will return true and so show the content controlled by the event target.

If you click 'no', all content controlled by the group would still be hidden by the loop on line 87. The condition on line 92 will return false so no content is shown.

I ran the tests after making this change and they were green so figured it was ok. I made a jsbin to play around with it which seems to work as expected: https://jsbin.com/nademaxaxo/edit?html,js,output

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @tombye, that's great.

Forgot I wrote a test to cover that very scenario!
https://github.com/alphagov/govuk_frontend_toolkit/pull/315/files#diff-1cb888219674ec4080d3a7d2bdca5117R183

Still works as intended then 👍

They were rewritten to point at a local version of
jasmine-core but this is not listed in our
package.json so only exists in /node_modules as a
dependency of grunt-contrib-jasmine.
At the moment the ShowHideContent module binds
events to all radios and checkboxes in
block-labels. This mean a user clicking on any
block-label in the page will trigger code to
show/hide its related content, even if there is
none.

This commit limits the block-labels that trigger
the code to show/hide content related to it to
those that have that relationship.
The `result` variable will always be a string
(rather than an expression) so doesn't need
parenthesis.
@tombye
Copy link
Contributor

tombye commented Aug 26, 2016

This seems mostly there. I have one question for @gemmaleigh @robinwhittleton @NickColley @tvararu. I'd like to change data-target to data-controls so it's obvious that it's copied across into an aria-controls attribute.

Be interested in your thoughts on this.

@NickColley
Copy link
Contributor

@tombye I think if we want to keep the first release backwards compatible we should leave as is, otherwise there's a lot we could do to improve these and would be better thought about separately?

@@ -17,7 +17,7 @@
// Escape name attribute for use in DOM selector
function escapeElementName (str) {
var result = str.replace('[', '\\[').replace(']', '\\]')
return (result)
Copy link
Contributor

Choose a reason for hiding this comment

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

We could just return the above expression, result isn't used anywhere else so just keep it simple..

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed and done.

@tombye
Copy link
Contributor

tombye commented Sep 2, 2016

@gemmaleigh I'm done on my changes. It would be worth getting another pair of eyes on it before merging now we've both worked on it but it's 👍 by me.

@gemmaleigh
Copy link
Contributor Author

Hi @tombye, thanks for these.

I've updated the JavaScript in a test branch for GOV.UK elements - so if anyone else would like to preview this working - it can be seen here.

I'm keen that we don't change the data-target attribute for now, it appears in quite a few places.

Happy for this to be merged, then I'll bump the toolkit and update govuk_elements and the govuk_prototype kit.

@gemmaleigh gemmaleigh merged commit 157f5a3 into master Sep 5, 2016
gemmaleigh added a commit that referenced this pull request Sep 5, 2016
Add GOVUK.ShowHideContent JavaScript to support showing and hiding
content, toggled by radio buttons and checkboxes ([PR
#315](#315))
@gemmaleigh gemmaleigh mentioned this pull request Sep 5, 2016
gemmaleigh added a commit that referenced this pull request Sep 5, 2016
Add GOVUK.ShowHideContent JavaScript to support showing and hiding
content, toggled by radio buttons and checkboxes ([PR
#315](#315)).
gemmaleigh added a commit to alphagov/govuk-prototype-kit that referenced this pull request Sep 5, 2016
https://raw.githubusercontent.com/alphagov/govuk_frontend_toolkit/master
/CHANGELOG.md

# 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))
gemmaleigh added a commit to alphagov/govuk_accessibility_sandbox that referenced this pull request Sep 6, 2016
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG
.md

# 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.10.0

- Allow New Transport font stack to be overridden by apps using
`$toolkit-font-stack`
and `$toolkit-font-stack-tabular` (PR #230)

# 4.9.1

- Fix phase banner alignment (PR #266)

# 4.9.0

- Add websafe organisation colours
- Split colours into two files with backwards-compatible colours.scss
replacement

# 4.8.2

- Add GOV.UK lint to lint scss files (PR #260)
- Remove reference to old colour palette (PR #256)
- Fix link to GOV.UK elements - tabular data

# 4.8.1

- Update DEFRA brand colour to new green (PR #249)

# 4.8.0

- Pass cohort name to analytics when using multivariate test (PR #251)

# 4.7.0

- Add 'mailto' tracking to GOV.UK Analytics (PR #244)

# 4.6.1

- Use the Sass variable $light-blue for link active and hover colours
(PR #242)
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.
@robinwhittleton robinwhittleton deleted the add-govuk-show-hide-content-js branch October 25, 2016 13:30
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