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

Icon instance vs class in Input component. #1063

Open
ImogenF opened this issue Jun 21, 2019 · 5 comments
Open

Icon instance vs class in Input component. #1063

ImogenF opened this issue Jun 21, 2019 · 5 comments

Comments

@ImogenF
Copy link
Contributor

ImogenF commented Jun 21, 2019

At the moment, there are 2 Input props concerning icons: statusIcon and placeholderIcon.

statusIcon is passed in as an icon instance, so that the user has more control over styling. placeholderIcon is currently the same, for consistency within the component, but it would make more sense for its size, colour etc. to be fixed.

The proposal is to remove statusIcon entirely from Input (Input already has too many options, and the user could just position an icon next to the input) and change placeholderIcon to a React component (type: IconComponentType).

@stereobooster
Copy link
Contributor

As well I would throw ID fields into the discussion. Do we really need them in library? Can't they be crafted in user land instead?

@micha-f
Copy link
Member

micha-f commented Jun 21, 2019

We're using it across the product in quite a few places. Might be nice to have it in the library then.

@ImogenF
Copy link
Contributor Author

ImogenF commented Jun 21, 2019

In that case maybe we could have ID fields as a separate component that extends Input, rather than having an isUniqueId prop on the Input component.

@TejasQ
Copy link
Contributor

TejasQ commented Jun 21, 2019

As well I would throw ID fields into the discussion. Do we really need them in library? Can't they be crafted in user land instead?

AFAIK, ID fields are optional props that are crafted in userland, but filled in for a11y if users omit them.

const uniqueId = useUniqueId(id)

Passing an argument to useUniqueId just uses whatever it's given (from userland) instead of generating one. I don't see your point, @stereobooster.

In that case maybe we could have ID fields as a separate component that extends Input, rather than having an isUniqueId prop on the Input component.

I like this. A new UniqueInput component or similar that's a thin wrapper around Input. 👌

@stereobooster
Copy link
Contributor

image

I mean those

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants