angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.4k stars 6.76k forks source link

feat(mat-form-field): Option to allow MatHint to persist alongside MatError #24319

Open RobertAKARobin opened 2 years ago

RobertAKARobin commented 2 years ago

Feature Description

On every Angular Material project I've worked on, our accessibility team has flagged that the mat-hint disappears when mat-error appears. To be accessible, it is required that the mat-hint continues to persist somehow. Currently, the mat-hint element is fully removed from the DOM: https://github.com/angular/components/blob/master/src/material/form-field/form-field.html#L83

This has been brought up on SO several times:

To compensate, on each project we:

  1. Write a wrapper component around MatLabel, MatHint, and MatError, as well as MatInput, MatSelect, MatCheckbox, etc, duplicating all of their inputs, or
  2. Write a wrapper component around MatLabel, MatHint, and MatError, using content projection for the form controls, which breaks the synchronization between all the elements.

Either way, we spend a lot of time on ugly hacks with which we are always ultimately unsatisfied.

I propose we hide mat-hint with CSS (e.g. display: none) instead of fully removing it from the DOM. This will allow users to easily continue to persist mat-hint as they see fit, simply by overriding the CSS.

(Alternatively, or in addition, we could add e.g. @Input('persistHint') to mat-form-field, but this would require more of a reworking of mat-form-field's logic, and would also require making some design decisions about how mat-hint should look when displayed alongside errors, for which the Material guidelines don't give instruction AFAIK.)

crisbeto commented 2 years ago

We're following the behavior specified in the Material Design specification:

  1. Error message When text input isn't accepted, an error message can display instructions on how to fix it. Error messages are displayed below the input line, replacing helper text until fixed.

I suspect that it was done this way, because it'll be cramped if there's both an error and hint text.

RobertAKARobin commented 2 years ago

@crisbeto Understood, but would it be possible to simply hide the hint text with CSS instead of removing it from the DOM altogether? That way it would still adhere to the Material spec, but our users with screen readers would be able to still access the hint text.

crisbeto commented 2 years ago

Wouldn't that be confusing for users because the input would have an aria-describedby pointing both to the error and the hint? E.g. if you had an email input, a screen reader might read out "Enter your email, This isn't a valid email".

RobertAKARobin commented 2 years ago

@crisbeto In most cases having the error first would fix that, I think.

crisbeto commented 2 years ago

I'm still not convinced that it won't be confusing, but I'll raise it during our team meeting to get some more opinions.

RobertAKARobin commented 2 years ago

I agree there are some implementation details to work out. But this always comes up in accessibility testing, and so think it would be prudent to address.

crisbeto commented 2 years ago

Sorry for the delay, I missed the meeting last week but we did discuss it this week and we decided to implement the proposal from this issue not as an opt-in feature, but as the default behavior.

RobertAKARobin commented 2 years ago

@crisbeto Thanks for letting me know! I actually have a PR almost ready to submit for this, just need to wrap up the test cases. Want me to give it a whirl before you guys start dedicating your time?

crisbeto commented 2 years ago

Sure, go for it.

RobertAKARobin commented 2 years ago

@crisbeto Here's the PR, although there's one e2e test that is failing but seems unrelated? Would love it if you could set me straight on that! https://github.com/angular/components/pull/24441

Oops, I wrote this as an opt-in, but see now that you said this would be default behavior. Would we care to do opt-in first so it's not a breaking change?