-
Notifications
You must be signed in to change notification settings - Fork 334
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
Fix hover style on small checkboxes and radio buttons in High Contrast Mode #3777
Conversation
34d5b84
to
109fe7c
Compare
109fe7c
to
3ce761f
Compare
Thanks for picking this up! I do have some mild reservations about using Highlight as a colour for hover states. Similar to how we try to reserve yellow only for focus states, I think it would make sense to do the same for Highlight. I'm wondering if we could explore a few alternative options, possibly using a thinner dotted or dashed outline could work? Given the limited palette and reduced feature set of forced colours mode, I'd also consider simplifying things by not trying to cover the focused + hovered state, and instead just letting the focus state 'win'. I think it would be worth having this reviewed by @davidc-gds and a designer too to make sure they're happy with the changes. |
In looking at the prior work (aka, Windows itself) in High Contrast Mode, it seems to be standard to use the Highlight colour for hovering. This is at least the case when hovering over checkboxes, radios, buttons, menu items, selects and toggles too. Interestingly, this means our usage of the colours is inverted compared to the Windows default, which uses white for focus states and Highlight for hover. |
Heya! Chiming in to give some thoughts that might help us make a decision, while not rethinking our entire high-contrast-mode focus and hover implementation. In my view, a visible hover when using contrast themes (like Windows High Contrast Mode) in any format is better than no visible hover. I am also thinking that some sort of visual change (dotted, dashed, thickness change) might satisfy that view though, as Ollie suggested. |
Yep! Not planning to reverse everything we have currently, just an interesting thing I noticed whilst experimenting 😅 |
Updated the initial comment with a dashed line approach instead. Note that I'm not applying any specific styles for the 'hovered and focused' states, this is just how they're turning out with the cascade the way it currently is, and documented for completion's sake. |
3ce761f
to
edb3e20
Compare
@CharlotteDowns @christopherthomasdesign @Ciandelle Sorry for another ask, but any particular views on the dashed border look in the top comment? |
Hey @querkmachine I think I'm ok with this in theory, it's quite a neat solution to the problem and it maintains a feeling that moving from hover to focus adds more visual weight to the thing. Are there any other components with hover states that should use this convention on high contrast mode too? |
@christopherthomasdesign I'm not sure, myself. It was flagged in this specific instance because we provide a hover style normally to compensate for the radios and checkboxes having a small visual target, but a large actual target area. This affordance didn't exist in High Contrast Mode. There are a few other elements that have a hover style normally but don't in High Contrast Mode, but they also don't have an invisible target area, so it's not precisely the same circumstances as here. |
Based on this thread, it seems like there's agreement that this is a good solution that doesn't interfere with anything else. So I'd recommend we go for it! 👍 On @christopherthomasdesign's question about whether we have other hover-able interactive components where the hover isn't carrying over to the contrast-themed version (for example, when using Windows High Contrast Mode):
1. Button (Start)Taken from this page: https://design-system.service.gov.uk/components/button/ 2. Accordion ('Show all' button)Taken from this page: https://design-system.service.gov.uk/components/accordion/ Accordion 'Show all sections' hovered: 3. CheckboxesWe already know this one (it's what this issues is about) 4. File upload(This doesn't have a hover stat when contrast themed, but it's also a native browser control at the moment) 5. RadiosWe already know this one (it's what this issues is about) 6. SelectTaken from this page: https://design-system.service.gov.uk/components/select/ Select un-hovered in high-contrast mode: Select hovered in high-contrast mode: 7. Text inputWe know these don't have hover states, in both contrast theme and regular theme. 8. Text areaWe know these don't have hover states, in both contrast theme and regular theme. |
@querkmachine sorry to add another thought right after that previous one. For the states where there's multiple things happening at once, would it make sense to combine the focus/active and hover state properties? So make the dotted 'hover' outline use the 'highlight' blue colour when:
|
@davidc-gds That seems a sensible change to me! Don't think it's too controversial either (I hope!) |
Forced colour modes (like Windows High Contrast Mode) ignore the box-shadow styling we apply to small checkboxes and radio buttons on hover. This change adds an outline on hover too, so that a visual change still takes place upon hovering in High Contrast Mode.
edb3e20
to
2e0ac10
Compare
Yeah all looks good to me 👍 thanks for the suggestions @davidc-gds 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we've switched to the dotted outline concept for the hover enhancements, this feels like an overall positive improvement for accessibility when using contrast themes. ✅
I can't speak fully to the code portion of this and would recommend getting a developer to review as well, but from a glance it looks to change what we want to change and doesn't do anything extra.
@@ -282,6 +286,10 @@ | |||
// We use two box shadows, one that restores the original focus state [1] | |||
// and another that then applies the hover state [2]. | |||
.govuk-checkboxes__item:hover .govuk-checkboxes__input:focus + .govuk-checkboxes__label::before { | |||
// Set different HCM colour when we have both hover/focus applied at once | |||
@media screen and (forced-colors: active), (-ms-high-contrast: active) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double checked to see if Autoprefixer does it now (just in case) but no.
Thanks for adding prefixes still. Adding another mention for:
@@ -282,6 +286,10 @@ | |||
// We use two box shadows, one that restores the original focus state [1] | |||
// and another that then applies the hover state [2]. | |||
.govuk-checkboxes__item:hover .govuk-checkboxes__input:focus + .govuk-checkboxes__label::before { | |||
// Set different HCM colour when we have both hover/focus applied at once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this boilerplate comment from other places Hightlight
is used?
// When in an explicit forced-color mode, we can use the Highlight system
// color for the outline to better match focus states of native controls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No blockers just some minor comments, looks brill 🙌
Hover styles are currently not visible on 'small' checkbox and radio buttons in Windows High Contrast Mode, because the
box-shadow
property is ignored in High Contrast Mode.This PR aims to fix that by also applying an
outline
attribute at the same time: transparent normally, but coloured in High Contrast Mode, so that a visual change still takes place on hover.As
outline
is also used by the focus state, there is some possible overlap here, but given we additionally thicken the input border on focus, a visual distinction between hovered, focused, and simultaneously hovered and focused is retained.Raised as a low vision usability issue by DAC in their recent audit of Frontend. Resolves #3695.