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

#342 Support type prop to set the type of input, adjusted styles to fit with designs, added Icon functionality #369

Merged
merged 15 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions packages/react-components/src/components/Input/Input.module.scss
Original file line number Diff line number Diff line change
@@ -1,12 +1,43 @@
.input {
align-items: center;
background: var(--surface-basic-default);
border: 1px solid var(--border-default);
border-radius: 4px;
box-sizing: border-box;
color: var(--content-default);
display: flex;
font-size: 15px;
line-height: 22px;
outline: none;
padding: 5px 12px;

&:hover {
border-color: var(--border-hover);
}

&--focused,
&--focused:hover {
border-color: var(--color-action-default);
}

&--disabled,
&--disabled:hover {
background-color: var(--surface-basic-disabled);
border-color: var(--border-disabled);
}

input {
background: none;
border: 0;
color: var(--content-default);
outline: none;
padding: 0;
width: 100%;
}

&::placeholder {
color: var(--content-disabled);
}

&::placeholder {
color: var(--content-disabled);
Expand All @@ -22,24 +53,28 @@
color: var(--content-disabled);
}

&--error {
&--error,
&--error:hover {
border-color: var(--color-negative-default);
}

&--xsmall {
height: 24px;
padding: 1px 4px;
}
&--small {
&--compact {
height: 32px;
padding: 4px 8px;
}

&--medium {
height: 36px;
padding: 6px 8px;
}

&--large {
height: 40px;
padding: 8px;
}

&__icon {
margin-right: 9px;

&--disabled {
color: var(--content-disabled);
}
}
}
53 changes: 42 additions & 11 deletions packages/react-components/src/components/Input/Input.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { render } from 'test-utils';
import { fireEvent, render } from 'test-utils';
import { Input } from './Input';

import styles from './Input.module.scss';
Expand All @@ -15,28 +15,59 @@ describe('<Input> component', () => {
expect(container.firstChild).toHaveClass(styles['input--medium']);
});

it('should allow for xsmall size', () => {
const { container } = render(<Input size="xsmall" />);
expect(container.firstChild).toHaveClass(styles['input--xsmall']);
});

it('should allow for small size', () => {
const { container } = render(<Input size="small" />);
expect(container.firstChild).toHaveClass(styles['input--small']);
it('should allow for compact size', () => {
const { container } = render(<Input kind="compact" />);
expect(container.firstChild).toHaveClass(styles['input--compact']);
});

it('should allow for medium size', () => {
const { container } = render(<Input size="medium" />);
const { container } = render(<Input kind="medium" />);
expect(container.firstChild).toHaveClass(styles['input--medium']);
});

it('should allow for large size', () => {
const { container } = render(<Input size="large" />);
const { container } = render(<Input kind="large" />);
expect(container.firstChild).toHaveClass(styles['input--large']);
});

it('should have error class on error', () => {
const { container } = render(<Input error />);
expect(container.firstChild).toHaveClass(styles['input--error']);
});

it('should have disabled class and input should be disabled if "disabled" prop is set', () => {
const { container, getByTestId } = render(<Input disabled />);
expect(container.firstChild).toHaveClass(styles['input--disabled']);
sgraczyk marked this conversation as resolved.
Show resolved Hide resolved
expect(getByTestId('input')).toHaveAttribute('disabled');
});

it('should have custom placeholder text if it is set', () => {
const { getByTestId } = render(<Input placeholder="Custom placeholder" />);
expect(getByTestId('input')).toHaveAttribute(
'placeholder',
'Custom placeholder'
);
});

it('should have text type input as default', () => {
const { getByTestId } = render(<Input />);
expect(getByTestId('input')).toHaveAttribute('type', 'text');
});

it('should have password type input if kind "password" is set', () => {
const { getByTestId } = render(<Input type="password" />);
expect(getByTestId('input')).toHaveAttribute('type', 'password');
});

it('should change the input type if show password icon is clicked', () => {
const { getByTestId, getByRole } = render(<Input type="password" />);
const input = getByTestId('input');
const button = getByRole('button');

expect(input).toHaveAttribute('type', 'password');
fireEvent.click(button);
expect(input).toHaveAttribute('type', 'text');
fireEvent.click(button);
expect(input).toHaveAttribute('type', 'password');
});
});
39 changes: 30 additions & 9 deletions packages/react-components/src/components/Input/Input.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import * as React from 'react';
import { ComponentMeta, Story } from '@storybook/react';
import { AddCircle as AddCircleIcon } from '@livechat/design-system-icons/react/material';

import { StoryDescriptor } from '../../stories/components/StoryDescriptor';

import { Icon } from '../Icon';
import { Input, InputProps } from './Input';

const placeholderText = 'Placeholder text';

export default {
title: 'Forms/Input',
component: Input,
argTypes: {
onChange: { action: 'changed' },
},
} as ComponentMeta<typeof Input>;

export const Default: Story<InputProps> = (args: InputProps) => (
Expand All @@ -18,23 +22,20 @@ export const Default: Story<InputProps> = (args: InputProps) => (

Default.storyName = 'Input';
Default.args = {
size: 'medium',
kind: 'medium',
placeholder: 'Placeholder text',
};

export const Sizes = (): JSX.Element => (
<>
<StoryDescriptor title="XSmall">
<Input size="xsmall" placeholder={placeholderText} />
</StoryDescriptor>
<StoryDescriptor title="Small">
<Input size="small" placeholder={placeholderText} />
<StoryDescriptor title="Compact">
<Input kind="compact" placeholder={placeholderText} />
</StoryDescriptor>
<StoryDescriptor title="Medium">
<Input size="medium" placeholder={placeholderText} />
<Input kind="medium" placeholder={placeholderText} />
</StoryDescriptor>
<StoryDescriptor title="Large">
<Input size="large" placeholder={placeholderText} />
<Input kind="large" placeholder={placeholderText} />
</StoryDescriptor>
</>
);
Expand All @@ -49,3 +50,23 @@ export const States = (): JSX.Element => (
</StoryDescriptor>
</>
);

export const Types = (): JSX.Element => (
<>
<StoryDescriptor title="Text">
<Input type="text" placeholder={placeholderText} />
</StoryDescriptor>
<StoryDescriptor title="Password">
<Input type="password" placeholder={placeholderText} />
</StoryDescriptor>
</>
);

export const WithIcon = (): JSX.Element => (
<>

Choose a reason for hiding this comment

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

React.Fragment is not necessary here

Choose a reason for hiding this comment

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

Bump ☝🏼

<Input
icon={<Icon source={AddCircleIcon} />}
placeholder={placeholderText}
/>
</>
);
88 changes: 72 additions & 16 deletions packages/react-components/src/components/Input/Input.tsx
Original file line number Diff line number Diff line change
@@ -1,34 +1,90 @@
import * as React from 'react';
import cx from 'clsx';

import {
VisibilityOn as VisibilityOnIcon,
VisibilityOff as VisibilityOffIcon,
} from '@livechat/design-system-icons/react/material';

import styles from './Input.module.scss';
import { Button } from '../Button';
import { Icon } from '../Icon';

type InputSize = 'xsmall' | 'small' | 'medium' | 'large';
type InputSize = 'compact' | 'medium' | 'large';
Copy link
Contributor

Choose a reason for hiding this comment

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

For me it could be a generic size type lifted up in folders structure as we are slowly starting to repeat ourselves

Screenshot 2022-11-15 at 08 42 54


export interface InputProps extends React.HTMLAttributes<HTMLInputElement> {
size?: InputSize | undefined;
export interface InputProps
extends React.InputHTMLAttributes<HTMLInputElement> {
kind?: InputSize | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to | undefined in this and the next three lines

error?: boolean | undefined;
disabled?: boolean | undefined;
icon?: React.ReactElement;
}

const baseClass = 'input';

export const Input = React.forwardRef<HTMLInputElement, InputProps>(
({ size = 'medium', error = false, className, ...inputProps }, ref) => {
(
{
kind = 'medium',
error = false,
disabled,
icon = null,
className,
...inputProps
},
ref
) => {
const [isFocused, setIsFocused] = React.useState(false);
const [isPasswordVisible, setIsPasswordVisible] = React.useState(false);
const { type } = inputProps;
const mergedClassNames = cx(
className,
styles[baseClass],
styles[`${baseClass}--${kind}`],
{
[styles[`${baseClass}--disabled`]]: disabled,
[styles[`${baseClass}--focused`]]: isFocused,
[styles[`${baseClass}--error`]]: error,
}
);

return (
<input
className={cx(
className,
styles[baseClass],
styles[`${baseClass}--${size}`],
{
[styles[`${baseClass}--error`]]: error,
}
<div className={mergedClassNames}>
{icon &&
React.cloneElement(icon, {
className: cx(styles[`${baseClass}__icon`], {
[styles[`${baseClass}__icon--disabled`]]: disabled,
}),
})}
<input
{...inputProps}
data-testid="input"
ref={ref}
onFocus={() => setIsFocused(true)}
onBlur={() => setIsFocused(false)}
disabled={disabled}
type={type && !isPasswordVisible ? type : 'text'}
/>
{type === 'password' && (
<Button
disabled={disabled}
kind="plain"
icon={
<Icon
customColor={

Choose a reason for hiding this comment

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

I would move these conditions to constants

!disabled
? 'var(--content-default)'
: 'var(--content-disabled)'
}
source={
!isPasswordVisible ? VisibilityOnIcon : VisibilityOffIcon

Choose a reason for hiding this comment

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

As above, the target is to make JSX as simple as possible

}
/>
}
onClick={() => setIsPasswordVisible((v) => !v)}
/>
)}
type="text"
ref={ref}
{...inputProps}
/>
</div>
);
}
);