fix(material/core): disable strong focus indicators in high contrast mode

[crisbeto]

Most components have special handling for high contrast mode which can cause double focus indication when strong focus indication is disabled.

These changes remove the strong focus indicators if such a case is detected.

Fixes #24097.

cc @zelliott

[crisbeto]

I went with this approach, rather than disabling the HC styles, because it’s easier to maintain since we only have one place that needs to be disabled. Our HC focus indicators are pretty prominent so this shouldn’t be a problem for the user.

[zelliott]

I thumbed up, and then thought a bit more about it. Suppose we remove all of the HCM focus styles and instead just rely upon strong focus indicators? I understand we'll be introducing a HCM regression for apps that aren't opted into strong focus indicators, but if they care about HCM focus indication, they should care about all focus indication. It would allow us to simply remove a bunch of HCM styles across the components. WDYT?

My selfish motivation for wanting this is that in my app, we have a bunch of custom components that today use the strong focus indicators for both HCM and non-HCM focus indication. Once this PR lands, we'll need to instead add HCM styles to all of these components to ensure they have proper focus indication. This seems a little silly... as these components already had good focus indication before this PR.

[crisbeto]

I don't think that we'd actually remove that many HCM styles if we went down this route. Most of the current HCM styles are "structural" (e.g. menus without outlines) and there are a handful of cases where we need the extra focus indication. If you're concerned about your existing components, I could add an opt-out.

Alternatively, I can spot fix the few places where the strong focus and high contrast indicators conflict, but that leaves more room for bugs.

[zelliott]

From my audit, most if not all components with focusable elements have HCM styles that add outlines. For example, just going down the component list, I think we could remove some styles from:

• Button
• Checkbox
• List
• Stepper
• Button toggle
• Chips
• Datepicker
• Slide toggle
• Tabs

And note that this isn't a complete list, I probably only looked at half of the components and stopped midway through because the ones with focusable elements had HCM focus indication could remove. I think this is a non-trivial amount of styles we'd be able to remove.

To me, it's choosing between (1) removing some HCM styles (not all) from a dozen+ components, or (2) adding a few more lines of styles here to the strong focus indicator mixin, but potentially complicating things for consumers. The only downside for option 1 is that - again - people who aren't opted into the strong focus indicator mixin won't have any HCM focus styles. I'm happy to take a stab at approach 1 so we can see what it looks like - it would be more work.

[crisbeto]

Here's a very rough count of how many lines approximately we would be able to remove across the entire library (not including the MDC components):

• button - 4 lines
• button toggle - 20 lines
• checkbox - 5 lines
• chips - 3 lines
• core - 5 lines
• datepicker - 6 lines
• expansion - 15 lines
• form-field - 10 lines
• list - 3 lines
• menu - 6 lines
• radio - 8 lines
• slide-toggle - 4 lines
• slider - 5 lines
• stepper - 3 lines
• tabs - 5 lines
  Total: 102 lines

I counted the mixin itself in the lines (@include high-contrast), but I excluded comments. This is offset somewhat by the ~32 lines that would be added by the strong focus indicators. I'm not convinced that this is enough of reduction for us to end up in a situation where consumers would have to include an extra mixin just to get focus indicators in HCM. There's also the issue that we wouldn't be able to cover users that want focus indication in HCM, but not the strong indication for non-HCM users. I'm open to exploring other approaches, but I don't think that replacing the HCM focus indication with the strong focus indicators is the right solution either.

[zelliott]

Thanks for the detailed analysis, I'll defer to your judgment, although maybe it makes sense for one of Jeremy/Miles/Andrew to also chime in.

... consumers would have to include an extra mixin just to get focus indicators in HCM. There's also the issue that we wouldn't be able to cover users that want focus indication in HCM, but not the strong indication for non-HCM users.

My understanding is that MD is heading in the direction of permanent strong focus indication (happy to discuss this over chat). For that reason, I don't think these will be issues long-term. That being said, maybe that means that this issue should be revisited then.

[mmalerba]

I don't feel too strongly either way, but this seems like a reasonable approach so as long as nobody objects I'm going to LGTM

[jelbourn]

My inclination would be to have the strong focus indicators supercede the default HCM styles, but I don't want to drop HCM styles by default. I don't feel super strongly, though, so long as we're hitting requirements.

One idea: what would it look like if we refactored existing components to use a common class like .mat-hcm-focus for all default HCM styles, and then turned off those styles in the strong focus indicators mixin?

[crisbeto]

Having another selector for HCM focus styles will be a larger refactor, because in a lot of cases the existing HCM selectors reuse the same focus selectors that we have for non-HCM users. We'd have to add extra elements to all the templates and pull out all of the HCM focus styles into new selectors, similarly to what we did for strong focus indicators. It isn't complicated, but it's a lot of churn so we might as well reuse the strong focus elements.

Another option that is similar to what @zelliott was proposing above would be to move all the strong focus styles into the individual components, use them for focus in HCM by default and have the opt-in if consumers want them to be visible for non-HCM users as well. Something like:

.mat-checkbox:focus .focus-indicator {
  border: solid 2px transparent; // transparent is visible in HCM
}

@mixin strong-focus-indicators-color {
  .mat-checkbox .focus-indicator {
    border-color: $primary;
  }
}

This is basically what we have now, except that the strong-focus-indicators mixin will become obsolete.

[mmalerba]

LGTM

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}