-
Notifications
You must be signed in to change notification settings - Fork 601
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: label for attribute in FASTTextField template should reference unique input id #6552
Comments
So as discussed in the support channel this might not be real bug but I'd still love to hear how to solve this problem better! |
Thanks for your patience @usrrname - following up here from our quick convo in Discord with some more robust feedback to questions. The quick TLDR is that I think that there are some ARIA issues around implementation which exist due to the lack of support for Cross Root ARIA. Essentially, with Web Components the shadow DOM is an isolated DOM instance...it's unique and elements within the light DOM cannot reference elements within a components shadow DOM. An example of the impact here is likely that if you needed to associate a light DOM element ID for a live region (as an example) it won't work because these are separate DOM instances. There were a few things called out which I don't believe are issues, which I'll enumerate below.
Per the above note, I think this is somewhat of a misunderstanding...the reason we can hardcode the id element is because the shadow DOM is essentially it's own encapsulate DOM. So, you can absolutely have "Foo" as an ID in the light DOM and we can use "Foo" as an ID within the component markup itself. Are you getting an actual error on this or is this just an assumption based on the ARIA guidance?
This is by design and while I think we have an extensibility story coming which may help this via the dynamic composition story we've outlined in #5901, I'm not sure we'll bring some of this into the default base implementation. One of the difficult things when creating something like foundation is that we have to strike a balance between what is the most common, necessary implementation for something and how much you shove into that implementation. While I agree it is a common use case and scenario to add helper text, error messages inline, etc - I don't know that enumerating all of that would be considered "foundational". There's nuance here, but ultimately anything we add ends up as "first party" supported, which then begs the question of how much we wire up in logic...and that leads to the question of, "What if my system doesn't need this...it's just bloat". In FAST specifically, we value composition - I think the issue here is that Cross Root ARIA gaps mean that extending with the existing template isn't readily possible, which I get. In this case, I'd definitely suggest forking the template to ensure that all of this can be encapsulated. We are looking at Form Associated controls for vNext now given these issues...
I'm curious about this feedback because the entire purpose of this is unrelated to the rest of the feedback from what I can tell. Passing a string as you've noted serves as the label. The observable for defaultSlottedNodes lets us know whether that's been passed or not so folks can hide the label when empty from assistive tech using CSS, thus the class. The presence here is an implementation detail...I think the larger issue is that everything goes into the default slot. With that said, I'd again suggest forking the template for now as an option. Hopefully this helps. We are tracking the Cross Root ARIA issues and will be making a decision on that in short order as well as a path forward. For now, I'm going to close the issue simply since the primary concern here is unique ID's and I'm not sure that there is actually any AT impact here since the id is in a unique DOM instance. If you have an error and repro you can share where this is causing a11y issues, more than happy to take a second look! |
Thanks for your thoughtful response @chrisdholt !
Yes, this is really difficult when it comes to trying to create composite form elements that need to meet stringent WCAG 2.1+ standards... and the extending around each form element I'm doing is making me doubt the cleaniness of my component architecture. I wonder, is there a better, cleaner way I could be architecting a component that should contain labels and instructional text next to form input, to associate all adjacent element id to the input element aria-attributes without doing all the funny business above? You know how there are well trodden patterns in Angular and React such as container-presentational component pattern, proxy pattern, etc. I'm not really sure what to do here because downstream, we'll also have users that expect to use these components in Angular, React and AEM while hooking in their expected state management solutions... When you say you the Fast team "encourage(s) composition"... would that mean that your team would recommend, in the interest of keeping components and templates atomized as much as possible, I probably should have created a factory function or higher-order class to render view templates adjacent to this text field...? |
So, in terms of meeting WCAG, the issue comes when you have a hard-line approach of using the base foundation template. IF that is a hard rule for your implementation, then "YES", I think you end up with WCAG issues...but I don't think that has to be the case. In this case I would fork the template to accomplish your goals...at least how it stands now. Again, we are looking at resolving this, but in the immediate term forking solves the issue. Without forking, yes you have WCAG issues because of cross root ARIA :(. Let me say this - there's nothing wrong with creating your own template to accomplish what you need for your P0 requirements and components. Cross Root ARIA is blocking us enabling accessible composition for these scenarios so we are looking at it for vNext prior to going stable. There's a lot of conversations happing to try and figure out the right path forward here and we'll known likely early in the new year :).
So, to repeat the above, "fork and support". Fork the template in a way that enables your scenario for now. You could extend the class to have the label be an attribute but ultimately I think that you could extend the template to provide either slots or attributes for the helper text, etc and enable your scenario. The primary issue right now is that the text field is in the shadow root and therefore to bind it to helper text id's, etc you will need those to exist in the shadow root right now.
We definitely have considered these things...I think cross root ARIA complicates the relationship right now so we need to figure out our path forward there.
Kind of - A simple example would be to look at Select, Combobox, Tree View, etc...the way that those come together are "composed". Again, with that said, the big blocker here is that composition with the existing template is prevented due to cross root ARIA issues. So, while our hope is to enable that, this prevents it. The two choices we have are to hoist your requirements to all implementations of text field, OR to have you fork the template. Again, we're looking at how to solve the Cross Root ARIA issue for the time being with our approach, but I think "composition" in your case means that if every text field requires potential error or help text, that forking the template to support that is likely the best approach given the current state of things. Hopefully this helps clarify! |
I think I might have found the thing that's "causing" my issue in the
|
@break-stuff Perhaps of interest? |
🐛 Bug Report
For WCAG 2.1 purposes, a label element should have a for attribute that references the ID of the associated form control input element.
See:
WCAG 2.2 Success Criterion 3.3.2 Labels or Instructions (Level A): Labels or instructions are provided when content requires user input.
H44: Using label elements to associate text labels with form controls
💻 Repro or Code Sample
https://stackblitz.com/edit/fast-textfield-repro or https://github.com/usrrname/fast-textfield
Problem in your codebase: https://github.com/microsoft/fast/blob/master/packages/web-components/fast-foundation/src/text-field/text-field.template.ts#L16
🤔 Expected Behavior
The label
for
attribute should reference the input id value for WCAG. In order for fields to be accessibly meaningful and compliant, the id property on input elements should be unique and be populated in the label element's for attribute.😯 Current Behavior
Currently the label
for
attribute in FASTTextField is hardcoded to thecontrol
input element which shares the same class and id value.A secondary "grievance" is that the first string provided to FASTTextField becomes the de facto label. If designers require there to be helper text or text that describes whether a field is "optional" or "required", this must be added in as children markup per text element so there is no dynamic way to create a relationship between the ids of the label or help text, and the input control element with aria-labelledby and aria-describedby attributes
🤔 Expected Behavior
defaultSlottedNodes
should not be present; perhaps a label attribute could be used instead for assigning the text value to a label element, or perhaps default slots could be more clearly documented as a feature of form fields on the fast.design docsany included label elements that are rendered should be able to reference the id of its corresponding input that it is labelling.*
💁 Possible Solution
*Because my company has beyond-WCAG-2.1 standards (can't complain, that's pretty cool but it definitely took some imagination), in order to respond to the a11y-apathy of most devs using such components, I used string template literals to interpolate unique IDs on label, helper texts and error message component so that the aria attributes could be properly referenced on the input field.
Here is an example of a way I created a composite component with a label, optional text, and helper text that reference each other while only requiring a developer to provide 1 input id and using interpolation of potential label, helper text IDs to populate aria attributes for
aria-labelledby
,aria-describedby
attributes.https://stackblitz.com/edit/fast-textfield-repro?file=README.md,src%2Fcomponents%2Fcustom-text-field%2Fcustom-text-field.template.ts
Example (top): extended FASTTextField with no customization
Example(second from top): custom-text-field with
label
,helptertext
,optionality
attribute.The input ID is applied to the input element.
The helper text element contains an id that interpolates
helper-<input-id>
. The label template has a for attribute that references the input ID (x => x.id
).The label element has an id the is created in the syntax
label-<input-id>
.The label element id is assigned to the
aria-labelledby
attribute on the input element.The helper text element id is assigned to
aria-describedby
attribute on the input element.🔦 Context
My team is building a design system/component library and they are as much as possible, extending Fast components, using the existing templates, and also importing the existing Playwright tests but modifying them to fit the business requirements/design needs. For any customizations or extensions on the component we are adding attributes or writing custom templates, or creating custom components with
FastElement
.In the last 2 weeks I was given the task of adding a label and 2 pieces of helper text above any form field, in addition to showing a custom error component (consisting of icon + message). The existence of a default slot for a label, made it seem as if any text supplied within the
<fast-text-field></fast-text-field>
tags would become the label... which is totally fine if that's all that needs to be on the component, but not if you're hoping to show more text above/below it.Just so you know... the requirements at my work are such that they want to show both helper text and inline helper text about whether a field may be "Optional" or "Required". (I.e. below https://cfpb.github.io/design-system/components/helper-text )... hence my need to create a solution that creates ids for the text elements to populate
aria-labelledby
andaria-describedby
on the input element.🌍 Your Environment
Chrome 108
Macbook Pro 2019 Mac OS Monterey 12.6
The text was updated successfully, but these errors were encountered: