Bug: Wrong accessibility tags applied to wrapping components when using renderField

[janhesters]

renderField breaks custom layout's that use custom HTML / JSX tags (which wrap an input, but also if they don't) by applying the wrong props to the custom tags. (Tools like axe-core warn about this.)

Actual

Notice the superfluous (and accessibility breaking) id="errors-for-email" role="alert" in the wrapping <div /> with class name "mt-2".

<form method="post" action="/playground">
  <div>
    <label id="label-for-email" for="email">
      Email
    </label>
    <div class="mt-2" id="errors-for-email" role="alert">
      <input
        type="text"
        id="email"
        name="email"
        aria-labelledby="label-for-email"
        aria-invalid="false"
        aria-required="true"
        value=""
      />
    </div>
  </div>
  <button>Save</button>
</form>;

Expected

<form method="post" action="/playground">
  <div>
    <label id="label-for-email" for="email">
      Email
    </label>
    <div class="mt-2">
      <input
        type="text"
        id="email"
        name="email"
        aria-labelledby="label-for-email"
        aria-invalid="false"
        aria-required="true"
        value=""
      />
    </div>
  </div>
  <button>Save</button>
</form>;

Steps to reproduce

Configure your custom form like this:

const Form = <Schema extends SomeZodObject>(props: FormProps<Schema>) => (
  <RemixForm<Schema>
    renderField={({ Field, ...props }) => {
      const { name } = props;

      return (
        <Field key={name as string} {...props}>
          {({ Label, SmartInput, Errors }) => (
            <>
              <Label />

              <div className="mt-2">
                <SmartInput />
              </div>

              <Errors />
            </>
          )}
        </Field>
      );
    }}
    {...props}
  />
);

The props id="errors-for-email" and role="alert" should only be applied to the <Errors /> component, but are also applied to the div with class name "mt-2".

BTW, this also happens with any other custom tags, even if neither Label, nor <SmartInput /> or <Errors /> are children of those tags.

Implications for real world

In the example case, there might be a way around this, e.g. by including the wrapping <div /> into a custom <Input /> component.

However, consider the custom error input from Tailwind components for example:

import { ExclamationCircleIcon } from '@heroicons/react/20/solid'

export default function Example() {
  return (
    <div>
      <label htmlFor="email" className="block text-sm font-medium leading-6 text-gray-900">
        Email
      </label>
      <div className="relative mt-2 rounded-md shadow-sm">
        <input
          type="email"
          name="email"
          id="email"
          className="block w-full rounded-md border-0 py-1.5 pr-10 text-red-900 ring-1 ring-inset ring-red-300 placeholder:text-red-300 focus:ring-2 focus:ring-inset focus:ring-red-500 sm:text-sm sm:leading-6"
          placeholder="[email protected]"
          defaultValue="adamwathan"
          aria-invalid="true"
          aria-describedby="email-error"
        />

        <div className="pointer-events-none absolute inset-y-0 right-0 flex items-center pr-3">
          <ExclamationCircleIcon className="h-5 w-5 text-red-500" aria-hidden="true" />
        </div>
      </div>

      <p className="mt-2 text-sm text-red-600" id="email-error">
        Not a valid email address.
      </p>
    </div>
  )
}

If you implement this with Remix-Forms it looks something like this:

const Form = <Schema extends SomeZodObject>(props: FormProps<Schema>) => (
  <RemixForm<Schema>
    renderField={({ Field, ...props }) => {
      const { name, errors } = props;

      return (
        <Field key={name as string} {...props}>
          {({ Label, SmartInput, Errors }) => (
            <>
              <Label />

              <div className="mt-2">
                <div className="relative rounded-md shadow-sm">
                  <SmartInput
                    className={
                      errors &&
                      'text-red-900 ring-red-300 placeholder:text-red-300 focus:ring-red-500 dark:text-red-600/100 dark:ring-red-400 dark:placeholder:text-red-400/70 dark:focus:ring-red-500/80'
                    }
                  />

                  {errors && (
                    <div className="pointer-events-none absolute inset-y-0 right-0 flex items-center pr-3">
                      <ExclamationCircleIcon
                        aria-hidden="true"
                        className="h-5 w-5 text-red-500 dark:text-red-600"
                      />
                    </div>
                  )}
                </div>

                <Errors />
              </div>
            </>
          )}
        </Field>
      );
    }}
    {...props}
  />
);

Note: Extra <div /> tags are needed to properly align the icon, making it impossible to use with remix-forms because accessibility is broken.

[janhesters]

For now, one workaround is to create your own custom InputWrapper and prevent Remix Forms from passing the false props to the tags:

export const InputWrapper = ({
  className,
  children,
}: JSX.IntrinsicElements['div']) => <div className={className}>{children}</div>;

which can then be used like this in renderField:

<>
  <Label />

  <InputWrapper className="mt-2">
    <InputWrapper className="relative rounded-md shadow-sm">
      <SmartInput
        className={
          props.errors &&
          'text-red-900 ring-red-300 placeholder:text-red-300 focus:ring-red-500 dark:text-red-600/100 dark:ring-red-400 dark:placeholder:text-red-400/70 dark:focus:ring-red-500/80'
        }
      />

      {props.errors && (
        <InputWrapper className="pointer-events-none absolute inset-y-0 right-0 flex items-center pr-3">
          <ExclamationCircleIcon
            aria-hidden="true"
            className="h-5 w-5 text-red-500 dark:text-red-600"
          />
        </InputWrapper>
      )}
    </InputWrapper>

    <Errors />
  </InputWrapper>
</>

[danielweinmann]

Hey, @janhesters! Thank you for the perfect issue description 👌🏽

The behavior you described is part of what we're trying to figure out with #189. I'll keep that issue for the bigger-picture solution and use yours to fix your specific example.

• This happens because Remix Forms mistakenly thinks your div is an error component (which is what Add a way to avoid magically passing props to inputs #189 is mostly about).

• But even if it thinks it's an error component, it shouldn't pass the id and role props if the field is valid. It's probably a bug on the logic inside this block.

I think this will be a quick fix, and I'll work on it later this week. Unless, of course, you'd like to send a PR :) just let me know, and I can guide you and review it.

[janhesters]

@danielweinmann

First of all, big thank you and your team for Remix Forms.

We've been using Remix Forms with our agency full time now, and we came about quite a few edge cases now that we use it extensively. However, it seems to be the best library of its kind out there. Thanks a ton for creating it! 🙏

Secondly, I cloned the repo on the weekend and tried to fix it, but didn't understand the code well enough. If you have the time, please guide me through the issue, and I will try to create a PR in my evening.

Also, if you can, I'd love to "scratch my own itch" and also address this issue in a PR, if you're okay with it and can provide some high level guidance for that, too.

[danielweinmann]

Thank you, @janhesters!

I don't understand exactly what's happening, but I'm pretty sure it's something between these lines of code.

I would create a test example with a failing test (just like I suggested in #53) and TDD my way to fixing this. Feel free to implement #53 first if you think the path is clearer.