[WNMGDS-281] Move hint error outside legend

[bernardwang]

Changed

Moved hint and errors outside the legend and added a new example.
Addresses #529

How to test

View the app in IE with JAWS (I used localtunnel and a VM)
Change an example using FormLabel to have a hint with an <a> tag.
i.e.

    <ChoiceList
      choices={[
        { label: 'Choice 1', value: 'A' },
        { defaultChecked: true, label: 'Choice 2', value: 'B' }
      ]}
      className="ds-u-margin-top--0"
      label="Radio example"
      name="choices_field"
      hint={<a href="">click me</a>}
    />
    <TextField
      defaultValue="Example value"
      label="Single line"
      labelClassName="ds-u-margin-top--0"
      name="single_example"
      requirementLabel="Optional"
      hint={<a href="">click me</a>}
    />

[line47]

LGTM!

[bernardwang]

@davidakennedy @1Copenut I've tested this in IE JAWS myself and it seems fine, but I actually unclear on how to reproduce the issue. In JAWS even when a link is in the legend it seems to read fine when I tab to the link, or if I use the arrow keys to move through the page. It would be really helpful if either of you could take a look.

[davidakennedy]

@bernardwang I tested this, and it mostly works as expected:

• I'm able to hear the link announced by browsing normally.
• I can hear it announced when I navigate by fieldset group.
• The only place I can't access it is in JAWS with IE 11. Which appears to be a known issue. See this issue in Jaws, and some testing information.

I tested with JAWS and Edge, Chrome and IE 11; VoiceOver with Safari and Firefox and Chrome with NVDA.

Even with the issues, I'm okay with merging it. I'm not sure we can get around moving links entirely out of fieldsets. It's something to maybe note in the docs that placing links and headings in fieldsets/legends is problematic.

[1Copenut]

LGTM 👍

In the future we may opt to add an aria-describedby to the input(s). Right now our fieldset elements read out their respective label and "Date" in the examples, where they used to read label then "Date, for example 4 slash 28 slash 1986

We'd need to be able to pass a unique ID prop into the hint() method of FormLabel and an aria-describedby={ID_PROP} into TextField, but that would be easy to do. It hinges on what we want the user to hear every time they pause in an input. Do they just need to hear the <legend> or do we want them to hear <legend> + short pause + <hint> ?

I'm good going ahead as is, because it improves our station with screen readers, but let's keep an ear out for further improvement.

[bernardwang]

@davidakennedy Regarding this:
The only place I can't access it is in JAWS with IE 11. Which appears to be a known issue. See this issue in Jaws, and some testing information.,
Are you still having this issue on this PR? This PR is supposed to move the hint outside of the legend specifically to address that issue.

@1Copenut It seems like a pretty big change to no longer read the hint or error automatically... Are you sure we can leave this as is? I can try the aria-describedby solution too!

[davidakennedy]

Are you still having this issue on this PR? This PR is supposed to move the hint outside of the legend specifically to address that issue.

@bernardwang Sorry, I wasn't as clear as I could have been. I only encounter this when I browse via links only in JAWS. That's done by hitting the JAWS key + F7. So it has fixed the main issue as noted in the issue, not hearing it when hitting the fieldset group.

Do they just need to hear the <legend> or do we want them to hear <legend> + short pause + <hint>?

@1Copenut That's a good question. I'm inclined to say that we can add those aria values as props, but make them not required. In case the hint text is super long. Or I guess we could make them required, and have folks be mindful of the text length.

[1Copenut]

@davidakennedy This is a gray area item, I don't have a strong opinion which is the "right" approach. In the past I've gotten annoyed hearing long legends before I could just hear the individual labels read aloud, so I think we could have those be optional props and have the aria-describedby return null if no ID was passed in.

[davidakennedy]

I think we could have those be optional props and have the aria-describedby return null if no ID was passed in.

I agree with this approach, and think it strikes the right balance.

[bernardwang]

@davidakennedy @1Copenut Ok, I didn't realize certain people didnt like the hint text being read. I can make a new ticket to add the aria labels. The only other issue is the error text. I feel like that should be read by default. Thoughts?

And one other question, currently the hint and error are still inside the label components, so I assume they are also read with the label automatically too. Since that sounds inconsistent, should I move the hint and error outside the label as well?

[davidakennedy]

@bernardwang Yeah, please keep the errors inside the labels on the other component. That way, they're more easily read for screen readers.

[bernardwang]

@davidakennedy Just to confirm, these are different behaviors:
legend markup, hint + error is not automatically read

<fieldset>
  <legend> label </legend>
  hint
  error
</fieldset>

lablea markup, hint + error is automatically read

<div>
  <label> 
       label 
       hint
       error
  </label>
</div>

[davidakennedy]

Just to confirm, these are different behaviors: legend markup, hint + error is not automatically read

@bernardwang See: https://accessibility.blog.gov.uk/2016/07/22/using-the-fieldset-and-legend-elements/

Anything in legends will be read by screen readers in some cases. We're moving the hints out though because of the inconsistency with how links are read/not read in legends.

[bernardwang]

@davidakennedy @1Copenut
Just wanted to clarify my question:

Because of the bug around JAWS not reading links in legends, we decided to move hint+error outside of legend in <FormLabel>. My question is, should we also move hint+error outside of label in <FormLabel>? This is so the screen reader experience of <FormLabel> is consistent regardless of whether it's a label or legend.

[bernardwang]

Waiting on the decision for this comment before merging

[davidakennedy]

Hey @bernardwang and @line47 – @1Copenut and I met to chat about this issue today, and here's where we landed:

1. We see this as less of a problem in the design system, and more at the application level. Specifically, in App3, where the original ticket was filed.
2. We both think we should keep hints and errors in labels and legends, because the benefits that screen readers get from having them in there is too great, even if we fixed this bug.
3. This is a relatively isolated issue with one browser and screen reader combination, so it's better to approach where applicable, when links need to be inside/near legends, rather than in the system itself.

Sorry to do an about face here, but we think that's better in the long run. Thanks for asking such good questions, @bernardwang, and aiming for consistency.

I'm going to comment on the original issue, and advise that we move the help drawer component outside of the legend, and still connect it to the form fields via ARIA.

Let me know if you all have any other questions.

[bernardwang]

@davidakennedy No worries, thank you for getting back to me! To clarify, we decided to just leave the CMS Core component the way it is right? Because this issues is relatively limited to certain devices/browsers and App3?

[davidakennedy]

To clarify, we decided to just leave the CMS Core component the way it is right? Because this issues is relatively limited to certain devices/browsers and App3?

@bernardwang Yes, exactly. I do wonder if it's worth putting a note on the choice list component about issues that arise when you put links inside legends. Would that be worth it?

[eppixdev]

Thank you @bernardwang, @davidakennedy, and others.

Just a quick question! Would it be feasible/make sense to have an additional customHtml prop that would render after the legend in each of the design systems components that uses FormLabel?

[line47]

Hey @eppixdev can you say more and do you have a specific example in mind to help give clarity?

[eppixdev]

@line47, sure thing! Hopefully this makes sense.

Instead of moving the entire hint outside of the legend like originally discussed, would it be possible to add an additional prop that would render under the legend?

We pass a help drawer link into the hint prop in ChoiceList quite a bit. ChoiceList eventually renders some markup that looks like this:

<fieldset class="ds-c-fieldset">
    <legend class="ds-c-label">
    <span class="">
       <span>Are you an example?</span>
    </span> 
    <span class="ds-c-field__hint">
        <span>If a family member or friend is helping you, select “No.”</span>
            <span class="ds-u-display--block">
                <a href="im-a-link.com">Click here to learn about what an example is.</a>
            </span>
        </span>
    </span>
    </legend>
    <div>
        <input class="ds-c-choice" id="radio_example" name="example" type="radio" value="true">
        <label class="ds-c-label" for="radio_example"><span class="">Yes</span></label>
    </div>
    <div>
        <input class="ds-c-choice" id="radio_example" name="example" type="radio" value="false">
        <label class="ds-c-label" for="radio_example"><span class="">No</span></label>
    </div>
</fieldset>

With an additional rendered prop:

<fieldset class="ds-c-fieldset">
    <legend class="ds-c-label">
    <span class="">
       <span>Are you an example?</span>
    </span> 
    <span class="ds-c-field__hint">
        <span>I'm still here!</span>
    </span>
    </legend>
    // ** New "customHTML" prop would render here
    <span>
        <a href="link.com">Click here to learn more about the new rendered prop</a>
    </span>
    <div>
        <input class="ds-c-choice" id="radio_example" name="example" type="radio" value="true">
        <label class="ds-c-label" for="radio_example"><span class="">Yes</span></label>
    </div>
    <div>
        <input class="ds-c-choice" id="radio_example" name="example" type="radio" value="false">
        <label class="ds-c-label" for="radio_example"><span class="">No</span></label>
    </div>
</fieldset>

How would this look from an accessibility stand point and would it work with how your components are designed?

[davidakennedy]

I think that's a good idea, @eppixdev. From an accessibility perspective, that should solve our challenge. Always happy to test too once we have something.

We may want to advise in docs that in most cases, using the regular hint is the way to go. Using this custom HTML is only needed in certain cases, like this one.

[line47]

Offering a way to put some content into a fieldset is definitely something that would be great to get into the design system. I think this can be accomplished using the choice component as opposed to choicelist. See the examples here https://design.cms.gov/example/components.choice.react/

I feel like building a fieldset component that generated a legend as well as content inside the fieldset would be more generic and could serve other purposes.