Add checkbox mousedown event handler

[owenatgov]

Fixes #1906

Handles the visual bug mentioned in the issue using a mousedown event handler that targets checkbox labels. More details can be found in the commit message.

Thanks to @edwardhorsford for his investigation into this issue and original PR #1907

[querkmachine]

This looks all cushty to me.

The outcome is a little different in Safari. Previously Safari wouldn't show the yellow outline at all, as the focus state wasn't triggered by click events. With this change, it now shows the yellow outline during the mousedown/active state.

That's not a blocker as far as I'm concerned, but just in case someone raises that checkboxes are 'flashing' yellow on Safari, going forwards...

[querkmachine]

Relatedly: Technically this is also an issue with radio buttons if you click the same one multiple times. That's not how radio buttons are intended to be used, however, so also not a big deal, IMO.

[owenatgov]

@querkmachine Good spots. The safari note gives me pause as we're essentially recreating the issue but for safari only. We could consider it a safari bug but it's known and they've said they wont fix it. I'm gonna investigate if there's an alternative that could cover safari, including @edwardhorsford's original solution, although IMO we've reduced the scope of the problem and this feels more like a safari problem than a problem with the component now.

[edwardhorsford]

@owenatgov PR works well for me I think. It would be nice if the same fix could be applied to radios too to make them consistent.

[36degrees]

Although it's somewhat an edge case, because the active and focused elements can be different, you can end up with two elements that look focused.

(If you click a checkbox label, keep the mouse button held, and tab, you'll end up with both the checkbox you're clicking and the checkbox that are focused in the 'focus state')

Generally, I'm a bit nervous about applying the focus state to something that isn't explicitly focused.

[edwardhorsford]

@36degrees I think my original solution didn't have the same issue - as it made the parent thing get focus, and then styled based on that.

Otherwise I still think I'd go with this - as it's a visual glitch you get on all mouse-downs of labels (albeit not very noticeably) vs a much rarer behaviour of mouse-down and tab. But then looking more wrong.

[owenatgov]

An update on where we are with this.

• Both the :active label solution and the original solution involved giving focus styles to things in instances where an element wasn't explicitly focused. Whilst this might not have immediate user consequences, it is a risky technical choice that may come back to us in unforeseen ways.
• I've put together a new solution via js. In summary, it adds a mousedown event listener, checks if the part of the checkbox being clicked is the label and then prevents default behaviour. This seems to work well from testing. My concern is that I've been unable to work out a reliable way to write tests for it and am hoping my colleagues and beyond can help me on this. Based on that, I'm currently not comfortable with this being merged.
• From a chat on slack with @36degrees there's a general concern that we might be doing a bit too much for an issue that has little to no impact on the user. Depending on the outcome of the testing quandary above, I may or may not recommend we formally reject this issue. That is probably going to be frustrating for those involved but we have to be tight about user impact vs risk of solutions.

Based on the above, @querkmachine I'm going to reject your review for now (thanks all the same!)

See previous comment

[edwardhorsford]

One thing I'll mention is where I first discovered this was because of the more obvious visual glitch you get if the checkboxes are contained in a focusable container.

[owenatgov]

@edwardhorsford Correct me if I'm wrong but it looks like that instance you mentioned, specifically the wider issue around the focusable filter element mentioned in ministryofjustice/moj-design-system#39, is intended to give keyboard users a way to skip to the filters from another part of the page. In that case, we'd recommend using js to give that containing element focus programatically ($module.setAttribute('tabindex', '-1') and then add a blur event listener to remove it so that it's only focusable when it's required, as dictated by the js and as an enhancement to the keyboard experience. We've done something like this in the error summary component.

[owenatgov]

Closing this PR along with the linked issue. Reasons detailed in comment on the issue #1906 (comment)