feat: Added form field component

[nicolechung]

🚀 Description

This PR introduces a Field component. It will enable us to build other form controls by providing styled form-elements.

---

🔬 How to Test

CI
CI (Actions) should all be green 🟢 . CI tests linting and tests!

Docs App

View the docs at https://87acbda6.ember-toucan-core.pages.dev/

- OR -

Pull down this repo and run the following:

pnpm i
cd docs-app
pnpm start

View the Field documentation

---

📸 Images/Videos of Functionality

[changeset-bot]

⚠️ No Changeset found

Latest commit: 38e906e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[github-actions]

Preview URLs

GH Env: preview
docs: https://5d3abc97.ember-toucan-core.pages.dev
api docs: https://5d3abc97.ember-toucan-core.pages.dev/api/modules.html

[ynotdraw]

One of the things @nicolechung and I talked about, but I'm not personally quite sure provides a ton of value is yielding back named blocks here versus only doing the contextual components.

So if you do:

<Field>
  <:label>Label<:/label>
</Field>

You'll still get the hint + error divs rendered like #10 (comment) mentions, so you'll get extra stuff in the DOM that you don't (potentially) care about. And in the case of the Error, you'll have a lingering error icon.

I feel like the contextual component path is a better API for consumers, as folks can add only what they need like:

// Taken from https://github.com/CrowdStrike/ember-toucan-core/compare/10-field-prototype-w-nicole#diff-6b74531eede9b2af7db5be79e6cc466ce08e9f1c7c604348a819b3a0825f8d76
<template>
  <Field as |field|>
    <field.Label>{{@label}}</field.Label>

    {{#if @helperText}}
      <field.Hint>{{@helperText}}</field.Hint>
    {{/if}}

    <field.Control>
      <input class="border-critical bg-blue" ...attributes />
    </field.Control>

    {{#if @error}}
      <field.Error>{{@error}}</field.Error>
    {{/if}}
  </Field>
</template>

and then usage of above:

<InputField @label="Input" />
<InputField @label="Input" @helperText="help!" />
<InputField @label="Input" @helperText="help!" @error="error" />

---

I also don't have strong opinions, I think it's just more my brain trying to figure out a use case for the named blocks if they can't conditionally render blocks they don't care about. Any thoughts around this or guidance?

[simonihmig]

For conditionally rendering named blocks, wouldn't something like this work?

{{#if (has-block "label")}}
  {{yield to="label"}}
{{/if}}

But this is not necessarily better than yielded components...

The way I would think about this is who is in charge of the overall markup and layout?

If we have strong opinions/guidelines/rules for where to put which element, or having markup that wraps everything, that shouldn't be user-controllable, then named blocks are the better fit. They are more restrictive, or let you be in control, which I think is good for design systems, as that's what they do: they impose restrictions on the what and how.

On the other side, if you want to be less opinionated, and give the user freedom to compose things in arbitrary ways, then yielded components are good. headless-form is a good example of this latter case, it's by its own definition un-opinionated.

So not sure how the field components fits in here. Some questions, which also might help to understand how opinionated this component needs to be:

• what's the deal with the "hint"? Is it really the case that the user can put them either before or after the control, or doesn't the design system dictate where the should be positioned, like ? (unfortunately, I wasn't able to take part in the latest sessions. Wasn't there cases where the hint was above the control, aligned to the right?)
• don't we need to wrap the whole field in some markup, like tailwind flex classes or something, to position things properly according to our Figma designs? Or is this not a concern of this component?

[ynotdraw]

TL;DR: Maybe it'd be good for us to chat how our pieces of toucan-core + headless-form will come together?

---

Long Version

what's the deal with the "hint"? Is it really the case that the user can put them either before or after the control, or doesn't the design system dictate where the should be positioned, like ? (unfortunately, I wasn't able to take part in the latest sessions. Wasn't there cases where the hint was above the control, aligned to the right?)

Thank you for bringing this up. I paired a bit with @NullVoxPopuli today and I think we landed on the following:

Field can be a contextual component rather than contextual + named blocks as we do need some flexibility to cover use cases we have. Reason being is for cases like checkboxes/radios where we want the following (where [ ] denotes a control (e.g., <input)).

[  ]  <LABEL>
      <HELP>
      <ERROR

This kind of breaks out of our typical vertical flow of:

<LABEL>
<HELP>
[  ]  
<ERROR

We talked about going the contextual-only route instead as it allows a bit more flexibility as you described above. It is potentially risky, as folks could put things in whatever order they'd like as you mention; however, they'd still be using our design approved building blocks for that (label, error, hint). It essentially makes sure we don't paint ourselves into a corner in the short-term. I mention below, but we'll provide complete, wired up components to users for common use cases like <input + <textarea, etc. that folks can use without having to do anything (e.g., <form.Input, <form.Textarea>). Any thoughts on this?

---

don't we need to wrap the whole field in some markup, like tailwind flex classes or something, to position things properly according to our Figma designs? Or is this not a concern of this component?

Yeah! So I think the latter, as it depends on what type of form component we are dealing with. Inputs are more vertical while checkboxes and radios are a bit different. So similar to above, I think we can be a bit loose here so that we are flexible to build all of our current needs.

Now that I'm typing this, I think Field's concern is: it provides styled label + hint + error components for you, but is not opinionated on the control you render with it. It's essentially an element shell with slight opinions of how things look, but pushes the functionality pieces onto consumers a bit.

The thinking for PRs after this one is to build exact use-case form components. We'd have something like an InputField that'd look roughly like this (I don't have my notes with me on how we'd do this integrating with ember-headless-form, but this should give an idea):

// InputField.hbs
<Field as |field|>
    <field.Label for={{field.id}}>
      {{@label}}
      {{yield to="label"}}
    </field.Label>

    {{#if @helperText}}
      <field.Hint id={{field.hintId}}>
        {{@helperText}}
        {{yield to="hint"}}
      </field.Hint>
    {{/if}}

    <field.Control>
      <input class="border-critical bg-blue" id={{field.id}} aria-describedby='{{field.hintId}} {{field.errorId}}' ...attributes />
    </field.Control>

    {{#if @error}}
      <field.Error id={{field.errorId}}>
        {{@error}}
        {{yield to="error"}}
      </field.Error>
    {{/if}}
  </Field>

Then consumers of Toucan Forms will do:

<form.Input @label="First Name" @helperText="help" />

I apologize for the wall of text, but what do you think? I'm curious if you think there's too much overlap between this and what ember-headless-form provides - I'm second guessing myself now!

I think as long as we can provide a Form API that renders accessible, styled form components to users + allows users to render raw elements alone, outside of the Form component (like a single checkbox with no wrapping form element (lots of cases of this in the wild)) then I think we're good!

Also totally open to chat through this so we can land on something we like!

[simonihmig]

Sounds great! Thanks for the thorough explanation, especially related to the "what is the concern of this" question!

I'm curious if you think there's too much overlap between this and what ember-headless-form provides

Yeah, I am seeing that a bit at least. But I guess we cannot prevent this, when Field should also be usable standalone, without a full-fledged form. I also think this is not terrible.

[simonihmig]

In headless-form, I yielded components using a lowercase name, for no particular reason. Should we try to establish a convention (within CS at least)?

I don't have any strong feelings for one or the other, has anyone of you?

[ynotdraw]

Excellent callout. I think there is a lint we are using somewhere that suggests using uppercase for these, but I also don't have strong feelings. What do you think @nicolechung @NullVoxPopuli?

[NullVoxPopuli]

I always go with PascalCase for components 🤷

[nicolechung]

But for the yielded part...here it is lowercase like field:

<Form::Field as |field|>

[simonihmig]

headless-form automatically applies for to labels, and a matching id to the control element, at least when using the provided components. This is the most basic a11y rule we need to follow for forms. So I wonder if/how this component should be concerned about...

The user brings their own control component, so we cannot set an id for them. So we could either generate an id (e.g. using unique-id helper), automatically set it as for on the label, and yield it, so users cann apply it to their controls, like this:

<Form::Field as |field|>
  <field.Label>First name</field.Label>
  <field.Control>
    <input id={{field.id}} ...attributes />
  </field.Control>
  <field.Hint>Hint text below the input</field.Hint>
</Form::Field>

Or we choose to not make this our concern, but then we should at least call this out in the docs!

For the more opinionated toucan-forms, I think we will wire this all up correctly for the user, right?

[ynotdraw]

Definitely! Thank you for bringing this up as a whole. Yeah, I think we'll need to figure this out. @nicolechung and I were discussing this last week as we were pairing. We'll need to do the id + for, as well as "link" the hint + error blocks as well via something like aria-describedby or similar. We're still discussing if we do this work as part of this PR or a follow up one. Our goal at the moment is pairing to get this far enough along that we can work on separate tasks without stepping on each other's toes. We're getting close!

The user brings their own control component, so we cannot set an id for them. So we could either generate an id (e.g. using unique-id helper), automatically set it as for on the label, and yield it, so users cann apply it to their controls, like this

Yeah! I like this idea a lot 😄. Commit coming soon!

Or we choose to not make this our concern, but then we should at least call this out in the docs!

Absolutely! If we decide it's not our concern here, we can call it out in the docs. I'd definitely prefer if we can do as much as we can though! (but maybe separate PR for wiring things up properly?)

For the more opinionated toucan-forms, I think we will wire this all up correctly for the user, right?

Oh yeah, definitely!

[ynotdraw]

Okay, yeah, after playing around a bit tonight it's a bit tricky as we don't have access directly to the control the user renders. Due to that, I think your example above is the best we can do.

Here's what we can provide to make things fully accessible, so that the hint + error blocks get read to screenreaders:

{{#let (unique-id) as |uniqueId|}}
  {{#let (concat "hint-" uniqueId) as |hintId|}}
    {{#let (concat "error-" uniqueId) as |errorId|}}
      {{yield
        (hash
          Label=this.Label
          Hint=this.Hint
          Control=this.Control
          Error=this.Error
          id=uniqueId
          hintId=hintId
          errorId=errorId
        )
      }}
    {{/let}}
  {{/let}}
{{/let}}

And then usage:

<Form::Field as |field|>
  <field.Label for={{field.id}}>First name</field.Label>
  <field.Hint id={{field.hintId}}>What should we call you?</field.Hint>
  <field.Control>
    <input
      class='border bg-basement text-titles-and-attributes'
      id={{field.id}}
      aria-describedby='{{field.hintId}} {{field.errorId}}'
      ...attributes
    />
  </field.Control>
  <field.Error id={{field.errorId}}>error</field.Error>
</Form::Field>

Since we can't access the control element directly, as that's provided by the consumer, we can provide IDs for consumers to wire things up as they please. Then they can put the for + id + aria-describedby in the correct places. What's your feeling about something like this?

[simonihmig]

Yeah, seems good overall.

Some (minor) notes:

• we could wire up the for attribute on the label already, as we do have control over the <label> element. headless-form does this. But not sure if that's really preferable here. We have two sides of the same coin here, for on the label, and id on the control. But auto-applying for would obfuscate the one, will leaving the other for the user to do. So maybe letting the user do both is less confusing? I'm 50/50 on this one, so your call! 😉
• FWIW, I used aria-errormessage for linking from the control to the element holding the errors., instead of aria-describedby with multiple IDs. Not sure which is better, or if there is any significant difference. But "errormessage" sounded more specific, so I chose that one...
• you don't have to use those nested {{#let}}, you can also introduce multiple new local vars in a single one, like here

[ynotdraw]

Appreciate all of the notes! This is very helpful. I like the idea of using aria-errormessage + aria-invalid="true". That seems a bit more explicit for error messages and then we can use aria-describedby for helper text only!

[ynotdraw]

Alright, I gave aria-errormessage a shot but ran into something similar to nvaccess/nvda#8318 with voice over as well. The screenreader wouldn't announce the error message on focus, but had no problems with the hint text. I added two examples of wiring this up in the docs - one using aria-describedby and the other using aria-errormessage. If we can get aria-errormesssage to announce properly on focus, I say we roll with that in our Form fields!

One thing from the docs:

Often times, you will want the element with the error message to be an ARIA live region, such as when an error message is displayed to users after they have provided an invalid value.

So adding aria-live="polite" announces the error once visible, but on focusing the element it does not.

[simonihmig]

Hm, that's bad. Thanks for testing this, I should do the same for headless btw...

Probably have to change this to describedby as well in headless-form, tracking here: CrowdStrike/ember-headless-form#47

Reminds me of the old days of browser incompatibilities, where you have nice new features in the spec, but you cannot use them due to IE support, instead going for crazy workarounds... 😩

[simonihmig]

For conditionally rendering named blocks, wouldn't something like this work?

{{#if (has-block "label")}}
  {{yield to="label"}}
{{/if}}

But this is not necessarily better than yielded components...

The way I would think about this is who is in charge of the overall markup and layout?

If we have strong opinions/guidelines/rules for where to put which element, or having markup that wraps everything, that shouldn't be user-controllable, then named blocks are the better fit. They are more restrictive, or let you be in control, which I think is good for design systems, as that's what they do: they impose restrictions on the what and how.

On the other side, if you want to be less opinionated, and give the user freedom to compose things in arbitrary ways, then yielded components are good. headless-form is a good example of this latter case, it's by its own definition un-opinionated.

So not sure how the field components fits in here. Some questions, which also might help to understand how opinionated this component needs to be:

• what's the deal with the "hint"? Is it really the case that the user can put them either before or after the control, or doesn't the design system dictate where the should be positioned, like ? (unfortunately, I wasn't able to take part in the latest sessions. Wasn't there cases where the hint was above the control, aligned to the right?)
• don't we need to wrap the whole field in some markup, like tailwind flex classes or something, to position things properly according to our Figma designs? Or is this not a concern of this component?

[danwenzel]

beta versions?

[ynotdraw]

Yeah! Pairing with @NullVoxPopuli we hit a few issues with the current version that this version seemed to solve. By the time we release, maybe @glint/core will be proper 1.0.0?

[NullVoxPopuli]

yeah, the beta.3 is very close to full 1.0.0

[danwenzel]

temporary as in this will be updated in this PR?

[ynotdraw]

Excellent question! Still very temporary/WIP, but not worth blocking the PR on so we can continue to drive ahead from a development point of view. We should have a much better understanding from a design perspective of exactly what we want in about 2-4 weeks if I had to guess, but for the time being this is essentially a placeholder.

We know we want an icon of sorts here, but not quite sure what it should be yet or exactly how it should look. We think something like this works, but we definitely want to validate. And it could be removed completely if we find it doesn't work! Around the time mentioned above as well, we'll probably have quite a few PRs going up around styling tweaks to match what our design system expects; however, since we're re-thinking Form elements completely, things are still up in the air a bit. We should land safely soon™ though 😄 . We're definitely taking advantage of being pre-release 😂 !

[simonihmig]

Approving, with just one minor comment to consider.

[simonihmig]

This is now making also the yielded components available for direct usage, as in <Form::Label>. Do we want this? We could also try to hide them as much as possible from users, and only make them available yielded by <Form::Field>. This is what headless is doing.

To do this, we would need to make tighten the glob(s) for appReexports in the rollup config.

[ynotdraw]

Perfect, thank you for the suggestion! I agree - forcing folks to use <Form::Field> instead of the other building blocks feels much better. Went ahead and updated via 38e906e - let me know if I misconfigured something! 🙇 I verified I get an error on build when trying to import the Label component from the test-app with these changes.

I also stole your '**/*.js', public entry point line from headless-form as I think it makes sense for us too?

[simonihmig]

Perfect! 🎉