Bug or potential issue - show-hide-content.js

[paulmsmith]

Hi,

Following up from a discussion with @dsingleton on XGOV Slack. I noticed a slight issue with the show-hide-content.js code.

If you examine line 83 you will see there is an assumption that the 'Conditionally revealing content' pattern will existing within a form element.

As I was quickly hacking together a prototype within the the govuk prototyping kit, I hadn't wrapped the markup in a form element and it broke silently (no errors or console warnings). It's an edge case but as there is now a firm movement toward modular builds of components this could bite people.

Simple short term suggestion, check for existence of a form element, else console log a developer friendly message.

Longer term, assume this will be replaced by what's delivered in GOVUK Front-end?

[robinwhittleton]

@gemmaleigh bumped into this when adding radio buttons to GOV.UK Frontend – the selection button JS assumes a form as well. In that case we decided that we can drop the JS entirely given a change in markup, but here we probably just need a check for the existence of a form ancestor. Something like this might work?

var $form = $control.closest('form')
var $radios = $form.length ? $form.find(selector) : $(selector)

[paulmsmith]

@robinwhittleton That looks fine to me. What's the reason for using closest? To restrict the affected radios to just those within the form that they are situated?

[robinwhittleton]

Yes. Imagine a situation where you have two sets of radios with the same name attributes. Outside of a form or within the same form they’ll share the selected state toggling behaviour. In two seperate forms they’ll maintain two seperate toggle states.

[paulmsmith]

I thought so but my brain was failing me. :) 👍

[paulmsmith]

Nice. +1