Radios and checkboxes without JS #406

robinwhittleton · 2017-02-16T16:24:33Z

This PR removes the JavaScript requirement for radio buttons and checkboxes. To do this the markup had been changed, but there are no visual changes for users (beyond IE < 9 having a focus box around just the input rather than the whole label).

What problem does the pull request solve?

The new radios and checkboxes still required the selection buttons JS. This is a potential point of failure that’s a hangover from the old grey-box implementation, and isn’t really needed. We originally went with it because removing it would require markup changes and would mean that it they weren’t a drop-in solution. I think that was probably a mistake though, so this corrects it.

While making the markup changes I took the opportunity to rename the component from the old block-label to the more descriptive multiple-choice. This has better developer semantics, but also has the benefit that it’ll obviously require markup changes.

This fixes #375 and #367.

How has this been tested?

Tested in all macOS native browsers, and in Browserstack for IE and Edge on Windows (back to 8) and a selection of iOS and Android versions. The only initial bug found was a problem in IE9/10 that was fixed. No assistive tech testing has been done yet, and I’d want to do at least some to make sure that no new issues have been introduced. The assistive tech bugs fixed during the development of the new radios and checkboxes shouldn’t have regressed as their implementation hasn’t changed.

What type of change is it?

Breaking change (fix or feature that would cause existing functionality to change)

Has the documentation been updated?

I have updated the documentation accordingly.

NickColley · 2017-02-16T16:34:01Z

This is great, thanks Robin 👍

Would this fix this issue? #354

gemmaleigh · 2017-02-16T17:01:08Z

public/sass/_govuk-elements.scss

@@ -46,7 +46,7 @@
 @import "elements/details";                       // Details summary
 @import "elements/panels";                        // Panels with a left grey border
 @import "elements/forms";                         // Form - wrappers, inputs, labels
-@import "elements/forms/form-block-labels";       // Chunky labels for radios and checkboxes
+@import "elements/forms/form-multiple-choice";    // Chunky labels for radios and checkboxes


The comment needs updating here. "Custom" radio buttons and checkboxes?

robinwhittleton · 2017-02-16T17:10:22Z

No, that seems to be a generic browser rendering issue at different zoom levels (although it seems more pronounced in Chrome than other browsers). This doesn’t change the CSS rules that render the inputs (apart from fixing that IE bug).

NickColley · 2017-02-17T12:52:27Z

@robinwhittleton it seems as though, if we change the markup to add specific elements instead of pseudo elements we'd be able to fix that issue?

robinwhittleton · 2017-02-17T12:54:39Z

Honestly I haven’t tried it, but I’m not sure it would make any difference. I’ll give it a shot though.

robinwhittleton · 2017-02-20T09:03:31Z

I’ve tried out a few different ways of fixing #354 as part of this issue, but nothing is looking particularly promising. I’m loathe to complicate the markup here, especially as adding empty spans can cause issues for JAWS-on-IE and requires extra fragile workarounds. In the componentised future this wouldn’t be a problem, but where we’re relying on people cutting and pasting HTML I’m worried about the potential for accessibility problems in a small visual fix for a non-standard setup. It’s worth pointing out that the visual problem only manifests itself when the page is zoomed out, an unusual thing to do compared to zooming in.

tl;dr it’s worth leaving the issue open, but I don’t think we need to address it in this PR.

robinwhittleton · 2017-02-20T09:34:42Z

Blocked by #394 - that’ll need to be merged and this PR updated first.

NickColley · 2017-02-20T10:37:27Z

@robinwhittleton do you have a link to the the empty spans problem?

robinwhittleton · 2017-02-20T11:11:01Z

It was found and fixed on an internal repo, but the gist is that with the markup:

<label>
  <input type="radio" />
  <span><span></span></span>
  Example
</label>

The label text wouldn’t be read by JAWS-on-IE (JAWS-on-Firefox was fine). Some text needed to be added to the middle of the elements (we settled on <span><span> </span></span>) to get it read out. The old Verify custom radio builder shows this: https://github.com/alphagov/verify-frontend/blob/4f5b1592bc3e861dfd3f2294ddf9f816ceca01d3/lib/verify_form_builder.rb

NickColley · 2017-02-20T11:17:26Z

Very interesting, I wonder if that could be solved with aria-hidden?

robinwhittleton · 2017-02-20T18:16:32Z

Potentially? In other news, I’ve rebased this and updated on top of Gemma’s work to convert the joint text box + checkbox over to the new style so this is now unblocked for merging.

robinwhittleton · 2017-02-20T18:20:57Z

…assuming the accessibility team are happy. @cfq / @accessiblewebuk / @selfthinker ?

selfthinker · 2017-03-03T20:31:32Z

Should we give it another full testing round? Or should we only have a quick look? Or do you want us to test anything specific?

robinwhittleton · 2017-03-06T12:41:14Z

Well, the whole markup has changed, but the CSS remains mostly the same. I guess we probably want to retest it in everything based on that?

selfthinker · 2017-03-06T22:40:25Z

This is now at the top of our backlog, Richard will probably test these soon.
Do you have a URL for him to test?

robinwhittleton · 2017-03-07T09:40:54Z

I’ve pushed the branch to http://govuk-elements-wip.herokuapp.com/form-elements/#form-radio-buttons . That’s a temporary site, but it’s only really @gemmaleigh and I who use it so we’ll let you know if we need to repurpose it.

accessiblewebuk · 2017-03-07T17:32:25Z

@robinwhittleton I have completed a full round of accessibility testing (including WCAG 2.0 A and AA, JAWS14, JAWS17, NVDA 2017, Dragon 12.5 and 13, Read and Write Gold and Zoomtext) with the exception of VoiceOver on Mac which I can't do. No issues found.

The current implementation of radios and checkboxes relies on JS. This removes that reliance by changing the markup to allow the use of input pseudoclasses to infer state. We didn’t go down this route in the first place due to a perceived requirement for a drop-in solution. In practice we probably should have bitten the bullet and made a breaking change, but hindsight is 20:20. IE8 doesn’t support the required input pseudoclasses, but it also doesn’t support the pseudoelements needed for the new inputs so that’s OK. It does take a slight usability hit in that it no longer gets the focus ring for the whole block, but that feels like a reasonable compromise given its low usage and the reduction in JS use. None of this has been retested for accessibility yet, and will need a thorough test schedule before it goes live. It’s entirely possible that it will never make it into elements in favour of simply adding it to the new govuk_frontend, but it’s worth recording an implementation here.

`block-label` made sense when the labels were presented as buttons in an effort to get people to click on them. If we’re updating the markup in order to not require JS then we can take the opportunity to rename them to better match the functionality they expose. govuk_frontend_toolkit is (annoyingly) coupled to `.block-label` (https://github.com/alphagov/govuk_frontend_toolkit/blob/e03e7f56e1b70194310ee970a3ccf00bd9376b36/javascripts/govuk/show-hide-content.js#L13) and would need to be updated as well if this lands.

IE9/10 will happily change state when a click event is triggered against the label text, the space for the bullet, or outside the radio border but within the overall area. They will not change state if a user clicks in the gap inside the border and outside the bullet area (although focus state changes). Giving the transparent input an explicit z-index causes IE9/10 to recognise it as a target and assign the selection correctly.

The comment was still talking about the old ‘block label’ style inputs.

This now matches the new markup style for checkboxes.

robinwhittleton · 2017-03-14T12:23:14Z

I integrated this into a branch on my copy of Verify’s codebase (with a custom built gem). There wasn’t too much work in the end, it was mostly down to updating the markup in the tests.

robinwhittleton · 2017-03-16T09:21:19Z

When this is merged and released we’ll want to deprecate selection-buttons.js in govuk_frontend_toolkit. See alphagov/govuk_frontend_toolkit#389

gemmaleigh · 2017-03-16T12:28:38Z

I'm happy to merge, this has been reviewed by the accessibility team - thanks @accessiblewebuk and @robinwhittleton!

- Breaking changes to form validation (#PR 405) This adds two new classes: `.form-group-error` and `.form-control-error`, and removes the old `.error` class. This lets specific controls within a group be accurately marked as having errors. For example, this allows parts of a date entry to be marked rather than the whole section. To upgrade, you’ll need to change any occurences of `.error` that were applied to form groups to be `.form-group-error`, and then add `.form-control-error` to the failing input inside. - Radio buttons and checkboxes have new markup that doesn’t need JavaScript (PR #406) Previously the radio buttons and checkboxes relied on selection-buttons.js (from govuk_frontend_toolkit) to work. By reworking the markup we’ve been able remove this requirement. To upgrade, you’ll need to change your markup to match the new pattern. Once that has been done you’ll be able to stop including selection-buttons.js and remove the `GOVUK.SelectionButtons` constructor from your JavaScript. The look and feel remains the same barring some tweaks to the focus weight. Accessibility and device compatibility remains the same as the previous version. - Add examples of phase tags used outside of phase banners (PR #366) - Fix invisible input content when changing colours (PR #397) - Recommend the use of `aria-current="page"` for the last breadcrumb (PR #418)

The visual appearance of radio and checkbox form inputs changed in GOV.UK Elements here: alphagov/govuk_elements#296 This was subsequently reimplemented with different markup and no Javascript here: alphagov/govuk_elements#406 This has meant making the following changes to our app: - changing the markup in our radio/checkbox macros to match the example markup given by GOV.UK Elements - removing the previous Javascript file because it’s no longer needed to make the radios appear visual selected - making the buttons on the scheduled job picker look like links, because the grey button style looked weird with the new radio buttons

gemmaleigh reviewed Feb 16, 2017

View reviewed changes

robinwhittleton force-pushed the radios-without-js branch from 9df9956 to d1358dc Compare February 20, 2017 18:07

robinwhittleton force-pushed the radios-without-js branch from d1358dc to 5c8cebe Compare March 7, 2017 09:38

Robin Whittleton added 5 commits March 7, 2017 18:07

Refer to the new radios/checkboxes in govuk-elements

2ce69d6

The comment was still talking about the old ‘block label’ style inputs.

Update text input & checkbox example to new style

fcff249

This now matches the new markup style for checkboxes.

gemmaleigh force-pushed the radios-without-js branch from 5c8cebe to fcff249 Compare March 7, 2017 18:07

gemmaleigh temporarily deployed to govuk-elements-review-pr-406 March 7, 2017 18:07 Inactive

robinwhittleton added this to the 3.0.0 milestone Mar 16, 2017

gemmaleigh merged commit a8d9f7e into master Mar 16, 2017

robinwhittleton mentioned this pull request Mar 16, 2017

v3.0.0 #420

Merged

gemmaleigh deleted the radios-without-js branch March 16, 2017 12:40

andymantell mentioned this pull request Mar 27, 2017

Update dependencies LandRegistry/land-registry-elements#108

Closed

gemmaleigh mentioned this pull request Apr 7, 2017

Bump govuk_frontend_toolkit to 5.2.0 #445

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Radios and checkboxes without JS #406

Radios and checkboxes without JS #406

robinwhittleton commented Feb 16, 2017

NickColley commented Feb 16, 2017 •

edited

Loading

gemmaleigh Feb 16, 2017

robinwhittleton commented Feb 16, 2017

NickColley commented Feb 17, 2017

robinwhittleton commented Feb 17, 2017

robinwhittleton commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017 •

edited

Loading

NickColley commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017

NickColley commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017

selfthinker commented Mar 3, 2017

robinwhittleton commented Mar 6, 2017

selfthinker commented Mar 6, 2017

robinwhittleton commented Mar 7, 2017

accessiblewebuk commented Mar 7, 2017 •

edited

Loading

robinwhittleton commented Mar 14, 2017

robinwhittleton commented Mar 16, 2017

gemmaleigh commented Mar 16, 2017

Radios and checkboxes without JS #406

Radios and checkboxes without JS #406

Conversation

robinwhittleton commented Feb 16, 2017

What problem does the pull request solve?

How has this been tested?

What type of change is it?

Has the documentation been updated?

NickColley commented Feb 16, 2017 • edited Loading

gemmaleigh Feb 16, 2017

Choose a reason for hiding this comment

robinwhittleton commented Feb 16, 2017

NickColley commented Feb 17, 2017

robinwhittleton commented Feb 17, 2017

robinwhittleton commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017 • edited Loading

NickColley commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017

NickColley commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017

robinwhittleton commented Feb 20, 2017

selfthinker commented Mar 3, 2017

robinwhittleton commented Mar 6, 2017

selfthinker commented Mar 6, 2017

robinwhittleton commented Mar 7, 2017

accessiblewebuk commented Mar 7, 2017 • edited Loading

robinwhittleton commented Mar 14, 2017

robinwhittleton commented Mar 16, 2017

gemmaleigh commented Mar 16, 2017

NickColley commented Feb 16, 2017 •

edited

Loading

robinwhittleton commented Feb 20, 2017 •

edited

Loading

accessiblewebuk commented Mar 7, 2017 •

edited

Loading