Skip to content
This repository has been archived by the owner on Jan 23, 2025. It is now read-only.

ui-components: Input #52

Merged
merged 8 commits into from
Jun 3, 2020
Merged

Conversation

koorosh
Copy link
Contributor

@koorosh koorosh commented May 7, 2020

ui-components // Input

Resolves: #23
Base implementation of Input component
Added Storybook as a playground to validate styles.
Supports only text type yet.

  • <TextInput />
  • <NumberInput />
  • <BaseInput />

Checklist

  • I have written or updated test for the changes I made
  • I have updated the README of the package I'm working in to reflect my changes
  • I have added or updated Storybook if appropriate for my changes

@koorosh koorosh force-pushed the input-component branch from 111f8e6 to ef1f1a2 Compare May 8, 2020 10:43
@koorosh koorosh marked this pull request as draft May 12, 2020 14:39
@koorosh koorosh force-pushed the input-component branch from b0def8e to a3bd24a Compare May 13, 2020 19:10
@koorosh koorosh changed the title WIP. ui-components: Input ui-components: Input May 13, 2020
@koorosh koorosh marked this pull request as ready for review May 13, 2020 19:18
@koorosh koorosh force-pushed the input-component branch 2 times, most recently from f5f2785 to 1d41131 Compare May 14, 2020 15:46
@nathanstilwell nathanstilwell added the ✨ candidate-component A component to either reduce duplication or provide planned functionality across applications. label May 18, 2020
@koorosh koorosh force-pushed the input-component branch 2 times, most recently from 8fb592a to 299f2a3 Compare May 22, 2020 14:18
koorosh added 7 commits May 29, 2020 12:36
Base implementation of Input component
- Added Storybook as a playground to validate styles.
- Supports only `text` type yet.
- Added tests to validate default state and user interaction with component
- TextInput component
- NumberInput component
- BaseInput - can be used to compose own
type of Input component.
It has to be possible to prefix Icon component at the beginning of Input element.
Current changes introduce some code refactoring where Input element is always
wrapped with outer container. And `prefix` prop is added to consume Icon component
with correct positioning and coloring.

Helpers module is supposed to contain small components like InputWrapper.
Firefox handled differently <input/> width so spinner
group was moved out of container's borders.
Applying 'overflow': auto attribute fixes this problem

@firefox-only mixin allows to apply styles for Firefox
browser only.
- InputProps are extended with HTMLInputElementAttributes to
allow provide all allowed props down to <input> element
and do not limit usage of component.
- `prefix` prop is renamed to avoid naming conflicts with
native `prefix` prop.
@koorosh koorosh force-pushed the input-component branch from 8face4e to 767a09a Compare May 29, 2020 09:38
@koorosh koorosh requested a review from nathanstilwell May 29, 2020 09:41
@nathanstilwell nathanstilwell requested a review from Annebirzin June 2, 2020 15:12
const { prefixIcon, disabled, invalid } = props;
return (
<InputWrapper disabled={disabled} invalid={invalid}>
<InputPrefix>{prefixIcon}</InputPrefix>
Copy link
Contributor

@nathanstilwell nathanstilwell Jun 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to hold up this PR any longer, but wanted to leave a note to come back to. Should we conditionally render this prefixIcon only if an icon is provided? Will React be smart enough to not render empty DOM elements?

Copy link
Contributor

@nathanstilwell nathanstilwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nathanstilwell nathanstilwell merged commit d104a83 into cockroachdb:master Jun 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✨ candidate-component A component to either reduce duplication or provide planned functionality across applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Input Component
3 participants