components icon indicating copy to clipboard operation
components copied to clipboard

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

Open RobertAKARobin opened this issue 2 years ago • 10 comments

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:

  • https://stackoverflow.com/questions/60185384/keep-the-displaying-of-both-error-and-hint-if-error
  • https://stackoverflow.com/questions/47166694/angular-material-input-with-hint-text-and-error-text-at-the-same-time

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.)

RobertAKARobin avatar Jan 31 '22 03:01 RobertAKARobin

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.

crisbeto avatar Feb 08 '22 13:02 crisbeto

@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.

RobertAKARobin avatar Feb 08 '22 14:02 RobertAKARobin

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".

crisbeto avatar Feb 08 '22 14:02 crisbeto

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

RobertAKARobin avatar Feb 08 '22 14:02 RobertAKARobin

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.

crisbeto avatar Feb 08 '22 14:02 crisbeto

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.

RobertAKARobin avatar Feb 08 '22 15:02 RobertAKARobin

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.

crisbeto avatar Feb 17 '22 19:02 crisbeto

@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?

RobertAKARobin avatar Feb 17 '22 19:02 RobertAKARobin

Sure, go for it.

crisbeto avatar Feb 17 '22 19:02 crisbeto

@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?

RobertAKARobin avatar Feb 18 '22 16:02 RobertAKARobin