Skip to content
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

feat(material/form-field): add MatLabel input property to change use of html <label> #27241

Closed
jpduckwo opened this issue Jun 6, 2023 · 12 comments · Fixed by #28948
Closed
Assignees
Labels
Accessibility This issue is related to accessibility (a11y) area: material/form-field area: material/select P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@jpduckwo
Copy link

jpduckwo commented Jun 6, 2023

Feature Description

Add an input property to MatLabel that changes the use of <label> into a basic div element without linking it to a control. Perhaps it could be implemented as:

<mat-form-field>
  <mat-label [displayOnly]="true">The label</mat-label>
  <non-labelable-form-control/>
</mat-form-field>

Use Case

When using form fields with standard 'labelable' form elements such as <input> and <select> and using a <mat-label> to label the field, the HTML specification for the <label for="x"> works very nicely. However, there are times when non-standard form elements are desirable to use, such as <mat-select> with html formatted options, or other custom form fields that you might build yourself. In these cases when you use the <mat-label> element, the resulting <label for="x"> generates a page error with the violation:

Incorrect use of <label for=FORM_ELEMENT>

image

You can leave out <mat-label> to work around this, however, it is a better user experience to have the floating label and to also have these form fields match the appearance and behaviour of the other fields in a form.

@jpduckwo jpduckwo added feature This issue represents a new feature or feature request rather than a bug or bug fix needs triage This issue needs to be triaged by the team labels Jun 6, 2023
@jpduckwo jpduckwo changed the title feat(MatFormField): Add MatLabel input property to change use of html <label> for non-labelable form elements and prevent page errors feat(material/form-field): Add MatLabel input property to change use of html <label> for non-labelable form elements and prevent page errors Jun 6, 2023
@jpduckwo jpduckwo changed the title feat(material/form-field): Add MatLabel input property to change use of html <label> for non-labelable form elements and prevent page errors feat(material/form-field): add MatLabel input property to change use of html <label> Jun 6, 2023
@mmalerba mmalerba added Accessibility This issue is related to accessibility (a11y) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: material/form-field and removed feature This issue represents a new feature or feature request rather than a bug or bug fix needs triage This issue needs to be triaged by the team labels Jun 12, 2023
@mmalerba
Copy link
Contributor

In the screenshot you show it looks like the label is associated with an invalid id, which seems like a separate bug. Angular Material allows custom form controls to specify their id and components like mat-select do this. However, it is a valid point that the for attribute is supposed to link to a "labelable element", and mat-select doesn't qualify as labelable according the html spec. We should look into an alternate way to make the association in these cases, or figure out if we can make the mat-select and other custom elements labelable (the spec mentions that form-associated custom elements are labelable)

@jpduckwo
Copy link
Author

Thanks @mmalerba for the input. Interestingly, you can see the same error as per my screenshot in the error generated on the mat-select example page. However, I'm not able to replicate it on a stackblitz. here is my stackblitz.

Is there something else at play here?

@Sergiobop
Copy link

This bug is present everywhere, not only in your case. It's even on the stackblitz examples of the select component guide:
https://material.angular.io/components/select/overview

When you open the first example in stackblitz, the console shows:

Incorrect use of <label for=FORM_ELEMENT>
The label's for attribute doesn't match any element id. This might prevent the browser from correctly autofilling the form and accessibility tools from working correctly.
To fix this issue, make sure the label's for attribute references the correct id of a form field.

@jpduckwo
Copy link
Author

jpduckwo commented Jun 15, 2023

I'm thinking actually this is an issue with Chrome detecting this problem. @mmalerba is right that the error message is stating that there is no element with the id in the for="" element. This isn't right, as the mat-select has the correct matching id attribute. I believe Chrome may be picking up this error due to the way the element is created - maybe it picks up the error in the middle of the process of initialisation of the field - or it's a bug with how Chrome detects the error. I don't believe it would be classified as an error if the element was matching but non-labelable. I'm going to leave this feature open for the moment, but leaning towards this request not being the way to solve this.

@LeroyGerrits
Copy link

LeroyGerrits commented Sep 2, 2023

This issue appears for every mat-select. The reason being that the generated <label for=""> points to a non-existing element, and there isn't a <select> being generated by using a mat-select in the first place. I hope this gets looked into soon because I now have to go through all kinds of hoops to generate a flat <span> or <div> that looks and behaves the same as a mat-label.

@todorpavlovic
Copy link

Any updates on this ?

@anderer455
Copy link

I encountered the same issue with mat-label and mat-date-range-input on Brave browser

image

@jeanfliu
Copy link

jeanfliu commented Feb 2, 2024

By adding a hidden <select> element, this error can be avoided.
Temporary solutions, informal. Hope this issue can be properly resolved.

<mat-form-field>
  <mat-label>Favorite Food</mat-label>
  <select id="food-select" style="display: none;"></select>  <!-- add this line -->
  <mat-select id="food-select" [(value)]="selectedFood">  <!-- add the same `id` attribute as above -->
    <mat-option value="apple">apple</mat-option>
    <mat-option value="banana">banana</mat-option>
  </mat-select>
</mat-form-field>

@EliseoPlunker
Copy link

EliseoPlunker commented Feb 2, 2024

By adding a hidden <select> element, this error can be avoided. Temporary solutions, informal. Hope this issue can be properly resolved.

<mat-form-field>
  <mat-label>Favorite Food</mat-label>
  <select id="food-select" style="display: none;"></select>  <!-- add this line -->
  <mat-select id="food-select" [(value)]="selectedFood">  <!-- add the same `id` attribute as above -->
    <mat-option value="apple">apple</mat-option>
    <mat-option value="banana">banana</mat-option>
  </mat-select>
</mat-form-field>

Personally I feel it's a bad idea. Your making a label for to any not visible, why not the solution proposed in the SO?

@Directive({
  selector: 'mat-select',
})
export class RemoveFor implements AfterViewInit {
  constructor(private el:ElementRef){}
  ngAfterViewInit()
  {
    setTimeout(()=>{
      const label=this.el.nativeElement.parentNode.getElementsByTagName('label')
      if (label.length && label[0].getAttribute('for'))
          label[0].removeAttribute('for')
    })
  }
}

When the problem is solved, simply we'll remove the directive from all the imports places

@anderer455
Copy link

By adding a hidden <select> element, this error can be avoided. Temporary solutions, informal. Hope this issue can be properly resolved.

<mat-form-field>
  <mat-label>Favorite Food</mat-label>
  <select id="food-select" style="display: none;"></select>  <!-- add this line -->
  <mat-select id="food-select" [(value)]="selectedFood">  <!-- add the same `id` attribute as above -->
    <mat-option value="apple">apple</mat-option>
    <mat-option value="banana">banana</mat-option>
  </mat-select>
</mat-form-field>

Personally I feel it's a bad idea. Your making a label for to any not visible, why not the solution proposed in the SO?

@Directive({
  selector: 'mat-select',
})
export class RemoveFor implements AfterViewInit {
  constructor(private el:ElementRef){}
  ngAfterViewInit()
  {
    setTimeout(()=>{
      const label=this.el.nativeElement.parentNode.getElementsByTagName('label')
      if (label.length && label[0].getAttribute('for'))
          label[0].removeAttribute('for')
    })
  }
}

When the problem is solved, simply we'll remove the directive from all the imports places

I mean yeah this is the solution that works but it works only now. It wont work once it gets fixed. The thing is I don't want to remember that I will have to delete this someday actually I'm sure I won't remember. I will forget it 100%.
And to be even able to remove the directive in future I have to keep an eye here to actually know when it gets fixed. Hate to see the error but only logical thing for me is to leave it as is. Once it gets fixed it should just go away. Right? Right?

@zarend
Copy link
Contributor

zarend commented Feb 2, 2024

Hello folks,

Thank you for reporting this. We prioritize accessibility issues that are a problem for users. If you have an explaination why this does not meet an a11y criteria (such as WCAG), we can consider that.

Angular components doesn't have a hard requirement to cleanly pass automated accessibility checks.

The reported issue in Chrome may be a false positive. Are you having accessibility or autocomplete issues in your application?

Best Regards,

Zach

@crisbeto crisbeto self-assigned this Apr 24, 2024
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 24, 2024
Adds an option to remove the `for` attribute from the `<label>` inside the form field from elements that can't be labeled. Also applies the new option to `mat-select` and `mat-date-range-picker`. This isn't an accessibility regression, because those elements were already being labeled using `aria-labelledby`.

Fixes angular#27241.
crisbeto added a commit that referenced this issue Apr 24, 2024
)

Adds an option to remove the `for` attribute from the `<label>` inside the form field from elements that can't be labeled. Also applies the new option to `mat-select` and `mat-date-range-picker`. This isn't an accessibility regression, because those elements were already being labeled using `aria-labelledby`.

Fixes #27241.
crisbeto added a commit that referenced this issue Apr 24, 2024
)

Adds an option to remove the `for` attribute from the `<label>` inside the form field from elements that can't be labeled. Also applies the new option to `mat-select` and `mat-date-range-picker`. This isn't an accessibility regression, because those elements were already being labeled using `aria-labelledby`.

Fixes #27241.

(cherry picked from commit 94a0834)
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) area: material/form-field area: material/select P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants