Input Label Accessibility Updates #514

danparnella · 2021-11-19T21:57:38Z

Resolves aria-labelled by for LabeledField #436
~~Plan is also to add aria props to any input based component~~
Would also like to make sure the fieldset/legend setup is correct for checkbox and radio groups

Author Checklist

Add unit test(s)
Update version in package.json (see the versioning guidelines)
Update documentation (if necessary)
Add story to storybook (if necessary)
Assign dev reviewer

src/forms/inputs/input.js

chawes13 · 2021-11-24T17:48:23Z

@danparnella Can you do an analysis on how this will impact styling (both in storybook and in the client template)?

# Conflicts: # .storybook/styles/application.scss # .storybook/styles/base/_typography.scss # .storybook/styles/components/_activity.scss # .storybook/styles/components/_buttons.scss # .storybook/styles/components/_due-dates.scss # .storybook/styles/components/_expander.scss # .storybook/styles/components/_forms.scss # .storybook/styles/components/_grid.scss # .storybook/styles/components/_invites.scss # .storybook/styles/components/_navigation.scss # .storybook/styles/components/_profile-card.scss # .storybook/styles/components/_profile-section.scss # .storybook/styles/components/_purchased-profile-blade.scss # .storybook/styles/components/_recruiter-search.scss # .storybook/styles/components/_reminders.scss # .storybook/styles/components/_search-results-and-filtering.scss # .storybook/styles/components/_sidebar-content.scss # .storybook/styles/components/_sign-up-blocks.scss # .storybook/styles/components/_spinner.scss # .storybook/styles/components/_summit-list.scss # .storybook/styles/components/_tabs.scss # .storybook/styles/pages/_main.scss # .storybook/styles/pages/_styleguide.scss # .storybook/styles/settings/mixins/_layout.scss # .storybook/styles/settings/variables/_colors.scss # package.json # src/controls/color-picker.js # src/controls/nav-link.js # src/controls/tab-bar/tab-bar.js # src/forms/buttons/button.js # src/forms/helpers/blur-dirty.js # src/forms/helpers/button-classes.js # src/forms/helpers/dropdown-select.js # src/forms/helpers/field-prop-types.js # src/forms/helpers/replace-empty-string-value.js # src/forms/inputs/checkbox-group.js # src/forms/inputs/checkbox.js # src/forms/inputs/cloudinary-file-input.js # src/forms/inputs/color-input.js # src/forms/inputs/date-input.js # src/forms/inputs/dropdown-checkbox-group.js # src/forms/inputs/file-input/file-input.js # src/forms/inputs/file-input/file-preview.js # src/forms/inputs/input.js # src/forms/inputs/masked-input.js # src/forms/inputs/radio-group.js # src/forms/inputs/range-input.js # src/forms/inputs/setter-link.js # src/forms/inputs/switch.js # src/forms/labels/input-error.js # src/forms/labels/input-label.js # src/forms/labels/labeled-field.js # src/indicators/spinner.js # src/routes/authorized-route.js # src/routes/index.js # src/routes/unauthorized-route.js # src/tables/components/table-row.js # src/tables/sortable-table.js # stories/controls/nav-link.story.js # stories/controls/tab-bar.story.js # stories/dynamic-input.js # stories/forms/inputs/checkbox-group.story.js # stories/forms/inputs/file-input.story.js # stories/forms/inputs/input.story.js # stories/forms/inputs/radio-group.story.js # stories/forms/inputs/range-input.story.js # stories/forms/inputs/setter-link.story.js # stories/forms/labels/labeled-field.story.js # stories/tables/sortable-table.story.js # stories/tables/table.story.js

…ents, add stories, add tests

danparnella · 2023-03-10T23:33:39Z

.storybook/styles/application.scss

@@ -48,10 +48,8 @@ $output-bourbon-deprecation-warnings: false;
 @import 'components/progress-circle';
 @import 'components/login';
 @import 'components/pagination';
-@import 'components/sidebar-content';


I noticed a couple style files were no longer being used. Not sure if they should still be there for reference?

I'm fine with deleting them!

danparnella · 2023-03-10T23:35:27Z

.storybook/styles/components/_forms.scss

+fieldset,
+.field-wrapper {


My hope is that this might ease some of the style changes (just swapping fieldset for .field-wrapper in stylesheets). Hopefully most projects can just find and replace in their stylesheets without too much auditing.

Can you add this documentation to a migration guide?

danparnella · 2023-03-10T23:36:22Z

.storybook/styles/components/_forms.scss

+  .input-wrapper {
+    position: relative;
+
+    i {
+      @include position(absolute, 3px null null 10px);
+      background-repeat: no-repeat;
+      background-size: 15px;
+      display: inline-block;
+      height: 20px;
+      width: 20px;
+    }
+  }


This was included further down in a duplicate "Input Icon" section.

danparnella · 2023-03-10T23:37:23Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "@launchpadlab/lp-components",
-  "version": "8.0.0",
+  "version": "9.0.0",


I figured I'd hold off on writing a migration guide until the code changes are approved.

src/forms/inputs/checkbox-group.js

danparnella · 2023-03-10T23:50:18Z

src/forms/inputs/dropdown-checkbox-group.js

+    <div>
+      <FieldsetLegend {...{ label, name }} id={inputId} />
      <DropdownSelect selectedValues={value} className="checkboxes">
-        <CheckboxGroup {...props} label={false} />
+        <CheckboxGroup {...props} label={false} ariaLabelledby={inputId} />
      </DropdownSelect>
-    </fieldset>
+    </div>


The checkbox group is actually the fieldset so div should be correct here and since a label/legend isn't provided, I'm using the id on the "outer" legend to provide the aria labelledby.

If we aren't using a fieldset, is legend the semantically appropriate tag? Or would p/span suffice?

Looking at the specs, the only Permitted parent is a fieldset https://developer.mozilla.org/en-US/docs/Web/HTML/Element/legend#technical_summary

danparnella · 2023-03-10T23:52:32Z

src/forms/labels/fieldset-legend.js

+ * This component is used within {@link LabeledFieldset}, and therefore is incorporated into most fieldset grouped input components ({@link RadioGroup}, {@link CheckboxGroup}) by default.
+ *
+ * The text of the legend is set using the following rules:
+ * - If the `label` prop is set to `false`, the legend is hidden completely


I kept debating whether to change "label" fully to "legend", but I was thinking for backwards compatibility (potentially in code, but also in terms of consistency mentally), sticking with label as the prop might be better. It's also still a "label", just a different kind of one. BUT, that's just my two cents.

danparnella · 2023-03-10T23:55:41Z

@jhp0621 Tagging you for visibility!

jhp0621 · 2023-03-23T22:20:13Z

src/forms/inputs/radio-group.js

-      {...props}
-    >
-      {optionObjects.map((option, i) => {
+    <LabeledFieldset className={className} {...props}>


@danparnella what happens if label is false? Pretty certain <fieldset> must have a unique <legend>; if a radio group has label set to false, I believe we would still be rendering a <fieldset> but without a <legend>?

Yep, that's correct. If the design requires label={false}, then the developer should use aria-labelledby to specify which element to use as the "legend". It might be good for me to add that as an example in the story though.

It sounds like Ji ran into some issues where aria-labelledby on a fieldset still fails accessibility checks because a legend is required when using a fieldset (reference). An alternative might be to have <div role="group" aria-labelledby=""> in this instance.

The reference above is a technique which isn't required to meet WCAG, but is rather a guideline. We might need to dig more into the actual Success Criterion to see if this is a strict requirement or rather a strict interpretation on the accessibility tool. Maybe we could do testing with SiteImprove and Axe's accessibility testing extensions?

@jhp0621 Am I remembering correctly? Do you have more information/resources that you can share?

I just did some testing here, and the output is the exact same on VoiceOver. @jhp0621 maybe something to dig into a little deeper with the EA report. Does it list which success criterion are failing?

chawes13 · 2023-03-31T14:07:27Z

.storybook/styles/application.scss

@@ -48,10 +48,8 @@ $output-bourbon-deprecation-warnings: false;
 @import 'components/progress-circle';
 @import 'components/login';
 @import 'components/pagination';
-@import 'components/sidebar-content';


I'm fine with deleting them!

chawes13 · 2023-03-31T14:07:57Z

.storybook/styles/components/_forms.scss

+fieldset,
+.field-wrapper {


Can you add this documentation to a migration guide?

chawes13 · 2023-03-31T14:08:31Z

.storybook/styles/components/_forms.scss

@@ -145,8 +159,7 @@ Radio
 Checkbox
 -----------------------*/
 .checkbox {
-  @include rem(margin-top, 15px);
-  width: auto;
+  @include rem(margin-top, 3px);


Can you explain this change?

chawes13 · 2023-03-31T14:09:51Z

.storybook/styles/components/_forms.scss

@@ -567,7 +557,7 @@ Custom Dropdown Select
    }
  }

-  fieldset.checkbox,
+  .field-wrapper.checkbox,


I don't think this is a one-for-one change. If we have rules for fieldset.checkbox and fieldset.CheckboxGroup, then I believe we would additionally need field-wrapper.checkbox and field-wrapper.CheckboxGroup, eh?

chawes13 · 2023-03-31T14:12:00Z

src/forms/inputs/checkbox-group.js

        return (
-          <Checkbox // eslint-disable-line react/jsx-key
+          <Checkbox
+            key={option.value}


chawes13 · 2023-03-31T14:33:09Z

stories/forms/labels/labeled-fieldset.story.js

+  .add('with custom label', () => {
+    // eslint-disable-next-line
+    const CustomLabel = ({ onClickLabel }) => (
+      <legend onClick={onClickLabel} htmlFor="inputName">


Can we pass a different custom prop (or use it in a different way)? I don't want to encourage inappropriate onClick handler usage

chawes13 · 2023-03-31T14:36:55Z

stories/forms/labels/labeled-fieldset.story.js

+  })
+  .add('with custom error', () => {
+    const CustomError = (props) => (
+      <span id="inputError">


Can we change this to inputNameError or use the generate input error id util here? I don't want to encourage the wrong usage of a custom error component (the aria-describedby that's added to the input won't match this id)

chawes13 · 2023-03-31T14:40:36Z

test/forms/labels/fieldset-legend.test.js

+  const onClick = jest.fn()
+  const wrapper = mount(
+    <FieldsetLegend name={name}>
+      Are you <span onClick={onClick}>sure</span>?


Can we use the semantically appropriate <button> here instead?

Or remove it? Doesn't seem relevant to the test 🤔

chawes13 · 2023-03-31T14:41:51Z

test/forms/labels/fieldset-legend.test.js

+  const wrapper = mount(
+    <FieldsetLegend name={name}>
+      Are you{' '}
+      <span id="click" onClick={onClick}>


Please use a semantically appropriate element (i.e., button)

chawes13 · 2023-03-31T14:43:45Z

test/forms/labels/labeled-fieldset.test.js

+    </LabeledFieldset>
+  )
+  expect(wrapper.find('fieldset').exists()).toEqual(true)
+  expect(wrapper.find('fieldset').hasClass('error')).toEqual(false)


This doesn't align with the test description 🤔 . Can we create a separate test for this?

danparnella · 2023-09-27T18:56:31Z

Closing in favor of #595

danparnella added 13 commits November 19, 2021 13:16

add prettier

8ce4ea4

add prettier scripts

c7fd44a

run format (only src files)

01e065e

run format-storybook (mostly story styles)

6c61365

add format scripts to pre-commit

0de56e0

linting

f37d3f9

v5.4.3

b653e66

add formatting to stories as well

d9ddebd

run format-stories

686036d

swap fieldset for div

25f60b5

give every labeled field story input an id

573062b

add aria props to input, update story with examples

5dc32fb

update labeled field description to remove references to fieldset

8ba5a8e

danparnella linked an issue Nov 19, 2021 that may be closed by this pull request

aria-labelled by for LabeledField #436

Open

danparnella commented Nov 19, 2021

View reviewed changes

src/forms/inputs/input.js Outdated Show resolved Hide resolved

danparnella changed the base branch from add-prettier to main January 10, 2023 00:03

danparnella added 8 commits January 9, 2023 18:06

re-remove files

4c48452

revert additional merge changes

944e125

run prettier

dbd3cd1

create fieldset related components and use them in input group compon…

8c268b3

…ents, add stories, add tests

update styles

5d17b18

remove legend style

b82a3a6

v9.0.0

f89d97f

revert modifying input label, switch to fieldset legend

e492596

danparnella commented Mar 10, 2023

View reviewed changes

danparnella marked this pull request as ready for review March 10, 2023 23:55

danparnella requested a review from chawes13 March 10, 2023 23:55

jhp0621 reviewed Mar 23, 2023

View reviewed changes

chawes13 reviewed Mar 31, 2023

View reviewed changes

danparnella closed this Sep 27, 2023

danparnella deleted the inputs-aria-props branch September 27, 2023 18:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Input Label Accessibility Updates #514

Input Label Accessibility Updates #514

danparnella commented Nov 19, 2021 •

edited

Loading

chawes13 commented Nov 24, 2021

danparnella Mar 10, 2023

chawes13 Mar 31, 2023

danparnella Mar 10, 2023

chawes13 Mar 31, 2023

danparnella Mar 10, 2023

danparnella Mar 10, 2023

danparnella Mar 10, 2023

chawes13 Mar 31, 2023

danparnella Mar 10, 2023

danparnella commented Mar 10, 2023

jhp0621 Mar 23, 2023

danparnella Mar 30, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

chawes13 Mar 31, 2023

danparnella commented Sep 27, 2023

		fieldset,
		.field-wrapper {

		fieldset,
		.field-wrapper {

Input Label Accessibility Updates #514

Input Label Accessibility Updates #514

Conversation

danparnella commented Nov 19, 2021 • edited Loading

Author Checklist

chawes13 commented Nov 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danparnella commented Mar 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danparnella commented Sep 27, 2023

danparnella commented Nov 19, 2021 •

edited

Loading