Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Component: InputGroup #1692

Closed
brandonlenz opened this issue Oct 13, 2021 · 2 comments · Fixed by #2383
Closed

New Component: InputGroup #1692

brandonlenz opened this issue Oct 13, 2021 · 2 comments · Fixed by #2383
Assignees
Labels
good first issue Good for newcomers type: feature New feature or request

Comments

@brandonlenz
Copy link
Contributor

brandonlenz commented Oct 13, 2021

Input Group

USWDS Version

Not sure when it was introduced, but some other components (e.g. Input Prefix and Suffix) make use of this convenience component, and it seems like we should include it as well. It is very similar to FormGroup.

Description

A component that USWDS uses to group components (linked above), but not mentioned as its own exclusive component. Providing this convenience component may be nice

Requirements (proposed)

Should be able to toggle the error state with a prop

Prop Interface

interface InputGroupProps {
  children: React.ReactNode
  className?: string
  error?: boolean
}

Example Implementation (proposed)

export const InputGroup = ({
  children,
  className,
  error,
}: InputGroupProps): React.ReactElement => {
  const classes = classnames(
    'usa-input-group',
    { 'usa-input-group--error': error },
    className
  )

  return (
    <div data-testid="inputGroup" className={classes}>
      {children}
    </div>
  )
}
@jenbutongit
Copy link

I think InputGroup would be a good candidate for the API I suggested in here #1687, although it is generally quite restrictive, I'm not sure if that's how you'd like the lib to be consumed.

children would only be of type (Label | Input | InputPrefix | InputSuffix)[] Then you would be able to control the order in which they appear using React.Children.toArray. I think this article touches on it, but you get the idea (hopefully?) https://www.smashingmagazine.com/2021/08/react-children-iteration-methods/. You could have extra controls for prefix/suffix so that you don't have a prefix AND suffix, or throw an error when there's an input without a label etc.

@brandonlenz
Copy link
Contributor Author

@jenbutongit thanks for that detailed article! That's super helpful. From what I've seen from some internal conversations, we like the API you suggested! I think we just need to have a conversation about how to roll out a change like that.

Right now, we generally lean towards as flexible an API as possible, so that consumers can mix and match components as they choose (for better or for worse 😅). I think that is due in large part so that if they're using their own components or components from another library, they play nicely together. (Plus if they need a component new to USWDS that has not been implemented in React USWDS yet, they'd be able to do that. So that would likely be the main reason to not be highly restrictive when typing children.

When it comes to introducing the API pattern you suggested, I suppose we need to decide if that should be done one component at a time (i.e. new components are built using the "dot" syntax), or if a single breaking release should be targeted to convert all the relevant components to use this syntax.

I have some minor reservations about putting the library in a state where a fraction of the Components use a different API syntax than the rest, but will happily invest some time in ideating with some of the other maintainers about how we'd like to pursue this change 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants