MuckRock / documentcloud-frontend

DocumentCloud's front end source code - Please report bugs, issues and feature requests to info@documentcloud.org
https://www.documentcloud.org
GNU Affero General Public License v3.0
15 stars 5 forks source link

Global input styles #586

Open eyeseast opened 1 month ago

eyeseast commented 1 month ago

Putting this here to remind myself to think about it. Our form input components are mostly for styling. For example: Text.svelte.

I'm not sure this needs to be a component. It might be better as a global style for input[type=text], maybe with a class to preserve the option of different styles.

One place this has caused problems is adding labels:

<!-- svelte-ignore a11y-label-has-associated-control -->
<label>
  <span class="sr-only">{$_("sections.title")}</span>
  <Text name="title" bind:value={section.title} required />
</label>

Svelte warns about labels being associated with inputs, because it can't see into the component that wraps the input.

allanlasser commented 1 month ago

This is a good critique. You're right that only styling is really being applied, and proxying all the input props is not very elegant.

Counterpoint to consider:

Using the Field component solves for adding labels without raising a11y warnings (we can extend Field to support "sr-only" labels). By using the Field, we have a consistent, composed way of laying out inputs:

<Field title={$_("sections.page")} required hideLabel>
  <Number
    name="page_number"
    value={section.page_number + 1}
    min={1}
    max={document.page_count}
    required
    on:input={(e) => (section.page_number = fixValue(e))}
  />
</Field>
allanlasser commented 1 month ago

After thinking about this more, I believe you are correct that using global styles with standard elements is the right approach 👍