Fix "An input component must either have a 'aria-label' attribute..." error when a <label> is present

[tjosepo]

Issue: #1229

Summary

Removes the warning "An input component must either have a 'aria-label' attribute..." when a <label> is present.

What I did

I moved the validation logic inside a useEffect and add a check for isNilOrEmpty(input.labels).

I had to modify isNilOrEmpty to support arrays (and NodeList). Instead of checking value === "", it checks value.length === 0 (this works for strings, arrays and NodeLists).

How to test

Unit tests were added.

[changeset-bot]

🦋 Changeset detected

Latest commit: 871a9a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@orbit-ui/components	Patch
@sharegate/orbit-ui	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for sg-orbit ready!

Name	Link
🔨 Latest commit	871a9a3
🔍 Latest deploy log	https://app.netlify.com/sites/sg-orbit/deploys/64f895583e67b00008156181
😎 Deploy Preview	https://deploy-preview-1230--sg-orbit.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[netlify]

✅ Deploy Preview for sg-storybook ready!

Name	Link
🔨 Latest commit	871a9a3
🔍 Latest deploy log	https://app.netlify.com/sites/sg-storybook/deploys/64f89558d8cf0700088e1614
😎 Deploy Preview	https://deploy-preview-1230--sg-storybook.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[patricklafrance]

I believe this is textbook what React is now recommending NOT to do with useEffect: https://react.dev/reference/react/useEffect#caveats

[tjosepo]

Which part specifically are you concerned about?

Logging something in the console is the textbook definition of a side-effect in functional programming.

To check for the presence of a label, we need to access the ref to obtrain the HTMLInputElement. Using a useEffect or a callbackRef are the main ways to do it.

[patricklafrance]

I am not sure I understand the use case correctly.

Does the following code would print a warning?

    <Field required>
        <Label>Username</Label>
        <TextInput placeholder="[email protected]" />
    </Field>

When there's a Field and a Label, the label id should be retrieved from the FieldContext by the following code of the Input component:

const [fieldProps] = useFieldInputProps();

fieldProps would then add to the Input props the aria-describedby and aria-labelledby properties, which should prevent the warning from being displayed.

From what I can see by the following test, the issue might be that the Input and the Label are not wrapped within a Field component.

[tjosepo]

Does the following code would print a warning?

✅ I confirm that using <Field> worked without printing a warning.

I'll add it to the test scenarios.

---

I didn't know the <Field> component automatically added the accessibility properties.

To me, it still feel like a bug because using a label with htmlFor is accessible HTML, yet Orbit logs an error.

<>
    <Label htmlFor="test">Label</Label>
    <TextInput id="test" />
</>

I understand that labels are this is not as easily verifyable as using aria-label, aria-labelledby, and placeholder, since this forces us to move the validation logic after the component has been rendered on the page. This prevents us from checking accessibility during SSR. Altough, it is just as unit-testable.

I think it's a valid trade-off.

We can also argue against this change if we prefer people to use the <Field> component instead of manually passing in a label with htmlFor. In this case, we should update the warning message to mention using the <Field> component.

[patricklafrance]

Thank for the reply @tjosepo. Yes, the intent is to always use the <Field> component to group together a Label and an Input. Great idea to update the warning message!

[tjosepo]

Alright! I'll update my PR and go for it.