feat(charactercount): creates the character count form component

[duncan-truss]

This creates a new component CharacterCount as part of the forms/ group. The requirements are laid out in the issue so I won't recopy them all here.

https://designsystem.digital.gov/components/form-controls/#character-count
https://github.com/uswds/uswds/blob/develop/src/js/components/character-count.js

Please give me feedback on the use of state/hooks as I haven't done those before. Similarly if you feel strongly about how template strings are handled which we discussed a bit on Slack and @haworku's previously closed issue may be worth revisiting.

I also noticed that we (and uswds) aren't using the validity api anywhere else. Is this because there will usually be a form library used with their own validation functionality?

I had to add type definitions for an inputRef for TextInput and TextArea to make it easier to cast from the prop to the specific type needed for each component.

I did modify it to work with a defaultValue which seems important because we only listen for the onChange event to update the message.

fix #169

[eamahanna]

I'm not sure it makes sense to lump the Input and TextArea attributes. I think they are distinct and should be applied to the textArea or Input respectively.

[suzubara]

I don't think it's too bad... the thing is we'd need to figure out some conditionally-required props, ie. require textareaProps if it's a textarea and textInputProps if not. I'm not sure if this is possible with TS but I think treating them as the same is okay, and TS checking should catch issues if you tried to pass input-only props to a textarea and vice-versa. An alternative could be to create separate CharacterCountTextInput and CharacterCountTextarea components 🤷‍♀️

[ahobson]

I am but an 🥚 but since they are need mutually exclusive props (and are separate html tags), I would vote for separate components.

[duncan-truss]

In my opinion I don't think the implementation is different enough to separate the components. Textarea just happens to have some quirky attributes that textinput doesn't like rows, columns, wrap behavior, spellcheck ... USWDS seems to ignore rows and column attributes anyway at least within a default form group. Honestly there are more <input> types besides type="text" to probably consider supporting or that would diverge more than textarea does. Typescript will aid us here even though we're not doing an explicit cast/check.

[eamahanna]

You don't need to include data-maxLength in the rendered component because this isn't an actual HTML attribute.

[duncan-truss]

I can remove it, was just mirroring their implementation.

[eamahanna]

the maxLength attribute should be added to each of the input component types:
maxLength={maxLength}

[eamahanna]

See https://designsystem.digital.gov/components/form-controls/#character-count in the implementation section.

[duncan-truss]

If we do that then the field will not allow input over maxlength characters. Their javascript code reads that attribute, removes it then adds it as the data-maxlength to the character-count container in the uswds code.

[haworku]

I also noticed that we (and uswds) aren't using the validity api anywhere else. Is this because there will usually be a form library used with their own validation functionality?

Yes, I think validation (beyond what html5 does) should be happening outside our components, as you suggested, through form libraries or validation libraries. There's so much about validation that is specific to the use case. Even in our little example, there could be bugs if we hard code this too much - for example, thedefaultLength calculation probably would have to happen differently if the text included things like emojis.

I wonder if this component instead needs a prop validateLength. This would be a function called whenever the input changes and return a string with the validation message for the character count. It would take in parameters of the current input value, the max length, and (for our specific case/story) do the calculations in limitMessage. That would also remove the need to individually pass down the different lang strings to the component as props.

Def interested in other ideas here. The problem I see is that the USWDS js implementation seems opinionated and our version will probably have to be more flexible.

[haworku]

Can this be TextInputRef | TextareaRef ??

[duncan-truss]

🤦 Whoops that's why I made the interfaces then I didn't update the props

[haworku]

Another way to deal with this is just in order of props.

<TextInput
        id={id}
        data-testid="characterCountInput"
        name={name}
        className={classes}
        defaultValue={defaultValue}
        onChange={handleChange}
        onBlur={handleBlur}
        aria-describedby={messageId}
        inputRef={ref}
        {...inputProps}
        type="text"
      />

We could also write a test to make sure when you pass in a different type it doesn't apply (and document the expected behavior that way rather than through a comment).

[suzubara]

the value of type should be whatever the user passed in via inputProps.. why does this need to be explicitly passed in here?

[haworku]

I think we agreed to pass lang strings as props until we have a bigger discussion. One suggestion - the lang could be a fewer props if we ignore singular v. plural for now- emptyText, remainingText, overLimitText. We can write the english defaults as "character(s) over limit" for now.

[suzubara]

thinking long term for components like this that render complex strings (of which I doubt there will be many in this lib), this is where I think it would be more useful to expose a function prop that gets called with the current len value and returns the whole message string, so the user can have complete control over the logic and the copy. and the default value of this prop can be the existing limitMessage fn with the default English strings encapsulated in the fn.

[haworku]

@suzubara did you see this comment? #217 (review) seems like we are on the same page. Can we just do that now? I think that's more useful.

[suzubara]

ohh got it. So, yes I think we agree, my only suggestion is that there be two props, one that returns the message copy (ie, getCharacterCountMessage), and one that indicates validity (ie, isValid or hasError) (see my note about disable native validation). It is very likely that a user who is doing a custom implementation for copy will use the same code on their end to feed both props, but I think the component API will be easier to work with if they are decoupled. To me, a prop called validateLength sounds like an imperative function that might trigger validation, when really it should just return the message string.

[haworku]

A few nitpicks, opinions here:

• Is there a need to track the characterLength in state?
• Suggestion - rename invalidMessage to make it clearer, maybeisOverLimit
• Suggestion - spread state first to prevent bugs when making all of state a single object

setState((state) => ({
      ...state,
      limitMessage: limitMessage(len),
      invalidMessage: len > maxLength,
    }))

[duncan-truss]

Agreed, good points! Yep I did read about setState not being a merge operation in this case.

[suzubara]

What if we use a render prop to render the input, so the user doesn't need to pass in a label/hint/etc.? I'd like to avoid re-rendering a <Label> (and other possible related components) in CharacterCount. More info at https://reactjs.org/docs/render-props.html but I'm imagining something like:

<CharacterCount
  render={(input) => <FormGroup>My custom thing {input}</FormGroup>}
  />

[suzubara]

the value of type should be whatever the user passed in via inputProps.. why does this need to be explicitly passed in here?

[suzubara]

I don't think it's too bad... the thing is we'd need to figure out some conditionally-required props, ie. require textareaProps if it's a textarea and textInputProps if not. I'm not sure if this is possible with TS but I think treating them as the same is okay, and TS checking should catch issues if you tried to pass input-only props to a textarea and vice-versa. An alternative could be to create separate CharacterCountTextInput and CharacterCountTextarea components 🤷‍♀️

[suzubara]

thinking long term for components like this that render complex strings (of which I doubt there will be many in this lib), this is where I think it would be more useful to expose a function prop that gets called with the current len value and returns the whole message string, so the user can have complete control over the logic and the copy. and the default value of this prop can be the existing limitMessage fn with the default English strings encapsulated in the fn.

[suzubara]

both handleChange and handleBlur must implement onChange, onBlur props that may be passed in as part of inputProps in order to preserve any custom event handlers the user may pass in.

[suzubara]

according to https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-describedby_attribute aria-describedby is a space-separated list of element IDs. so IMO let's treat this like className, where we append the message ID to the user-defined aria-describedby which can be passed in as part of inputProps

[suzubara]

this is not a blocker, but also just thinking long-term to support whatever validation method the user has, we'll probably want to expose a flag prop that enables/disables native validation and lets them pass in their own invalid state somehow

[duncan-truss]

What if we use a render prop to render the input, so the user doesn't need to pass in a label/hint/etc.? I'd like to avoid re-rendering a <Label> (and other possible related components) in CharacterCount. More info at https://reactjs.org/docs/render-props.html but I'm imagining something like:

<CharacterCount
  render={(input) => <FormGroup>My custom thing {input}</FormGroup>}
  />

I think I'm understanding your suggestion.

I also noticed that we (and uswds) aren't using the validity api anywhere else. Is this because there will usually be a form library used with their own validation functionality?

Yes, I think validation (beyond what html5 does) should be happening outside our components, as you suggested, through form libraries or validation libraries. There's so much about validation that is specific to the use case. Even in our little example, there could be bugs if we hard code this too much - for example, thedefaultLength calculation probably would have to happen differently if the text included things like emojis.

I wonder if this component instead needs a prop validateLength. This would be a function called whenever the input changes and return a string with the validation message for the character count. It would take in parameters of the current input value, the max length, and (for our specific case/story) do the calculations in limitMessage. That would also remove the need to individually pass down the different lang strings to the component as props.

Def interested in other ideas here. The problem I see is that the USWDS js implementation seems opinionated and our version will probably have to be more flexible.

Ahhhh so true about unicode 💩 and maybe ligatures too 😩

[haworku]

Closing this for now it can be picked up when someone takes on this issue again.