-
Notifications
You must be signed in to change notification settings - Fork 0
Feature 7 - Input Component [Dev] #78
base: develop
Are you sure you want to change the base?
Changes from 5 commits
85c37f7
4d9cb26
6e9f7ab
63decaf
3ea6619
4a46de2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import { HTMLProps } from 'react'; | ||
|
||
export interface InputProps extends HTMLProps<HTMLInputElement> {} | ||
|
||
export default function Input(props: InputProps): JSX.Element; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import { useState, forwardRef } from 'react'; | ||
import { createStyleSheet } from '@platzily-ui/styling'; | ||
import PropTypes from 'prop-types'; | ||
|
||
const useStyleSheet = createStyleSheet( | ||
(theme, { required, type }) => ({ | ||
inputDefault: { | ||
width: 199, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This, and the height are givin issues with the checkbox type. They make it looks huge. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could provide these styles conditionally. |
||
height: 40, | ||
borderRadius: 5, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we have this in the theme? |
||
display: 'flex', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I not gettin this one, I removed those styles and the input behave the same. What are you trying to achieve here? |
||
alignContent: 'center', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also applies to the point above. |
||
alignItems: 'center', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this. |
||
justifyContent: 'center', | ||
backgroundColor: theme.palette.neutral.light, | ||
borderWidth: 1, | ||
borderStyle: 'solid', | ||
borderColor: theme.palette.neutral.secondary, | ||
required, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not needed 😁 |
||
type, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. neither this. Those are HTML attributes. |
||
'&:focus': { | ||
borderColor: theme.palette.tertiary.main, | ||
backgroundColor: theme.palette.neutral.light, | ||
borderWidth: 1, | ||
borderStyle: 'solid', | ||
outline: 'none', | ||
}, | ||
'&:disabled': { | ||
borderColor: theme.palette.neutral.tertiary, | ||
cursor: 'not-allowed', | ||
}, | ||
'&:invalid': { | ||
borderColor: theme.palette.error.main, | ||
}, | ||
'&:hover': { | ||
borderColor: theme.palette.tertiary.main, | ||
}, | ||
}, | ||
inputRequired: { | ||
textAlign: 'left', | ||
'&::placeholder': { | ||
color: theme.palette.error.main, | ||
paddingLeft: '95%' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets remove this styles. I see what you're trying to achieve here. But this will make the user struggle if another placeholder is needed. |
||
} | ||
}, | ||
}), | ||
{ key: 'InputComponent' }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets change this with |
||
); | ||
|
||
const Input = forwardRef(function Input(props, ref) { | ||
const { required, className, ...otherProps } = props; | ||
const { classes, cx } = useStyleSheet(props); | ||
|
||
// useState Hook | ||
const [state, setState] = useState({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets remove this. This state should be handled by the user. |
||
name: '', | ||
}); | ||
const handleInputChange = (event) => setState({ name: event.target.value }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this, this function needs to be received by props. Not as a private function. |
||
|
||
// Required Validation because of the styles | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These validations wont be necesary. |
||
let requiredStyles; | ||
let placeHolder; | ||
if(required) { | ||
requiredStyles = classes.inputRequired; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be achieved with the cx function using the object approach, you can look how is done in the Paper component. |
||
placeHolder = '*'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should remove this, lets let the users handle that asterisk theirselves. |
||
} else { | ||
requiredStyles = null; | ||
placeHolder = null; | ||
} | ||
|
||
return ( | ||
<input | ||
ref={ref} | ||
className={cx(classes.inputDefault, requiredStyles , className)} | ||
value={state.name} | ||
onChange={handleInputChange} | ||
placeholder={placeHolder} | ||
{...otherProps} | ||
/> | ||
); | ||
}); | ||
|
||
Input.propTypes = { | ||
className: PropTypes.string, | ||
required: PropTypes.boolean, | ||
}; | ||
|
||
Input.defaultProps = { | ||
required: false, | ||
}; | ||
|
||
export default Input; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import { getByTestId, render } from '@testing-library/react'; | ||
import { ThemeProvider, createTheme } from '@platzily-ui/styling'; | ||
import Input from './Input'; | ||
|
||
describe('@Components/Input', () => { | ||
it('Given the Input Component, when the props provide an object within the styles attribute then the component will take those styles ', () => { | ||
// arrange | ||
const theme = createTheme(); | ||
const { getByRole } = render( | ||
<ThemeProvider theme={theme}> | ||
<Input role="banner" style={{ color: theme.palette.secondary.main }} /> | ||
</ThemeProvider>, | ||
); | ||
|
||
// act | ||
const InputTestedText = getByRole('banner'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be done using another role, like textbox, the inputs have that role by default. And the banner role won't be needed. |
||
// assert | ||
expect(InputTestedText).toBeDefined(); | ||
expect(InputTestedText).toHaveStyle(`color:#97c343`); | ||
}); | ||
|
||
it('Given the Input Component, when the props provide type or required attributes then the component will take those props', () => { | ||
// arrange | ||
const { getByRole } = render( | ||
<ThemeProvider theme={createTheme()}> | ||
<Input role="form" type={'date'} required /> | ||
</ThemeProvider>, | ||
); | ||
|
||
// act | ||
const InputTestedText = getByRole('form'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
|
||
// assert | ||
expect(InputTestedText).toBeDefined(); | ||
expect(InputTestedText).toHaveProperty('type'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 what are we validating here? What about if we assert that the type property is date? |
||
expect(InputTestedText).toHaveProperty('required'); | ||
}); | ||
|
||
it('Given the Input Component, when include the component inside form tag, then form tag gets properties and his values. ', () => { | ||
// arrange | ||
const { getByTestId } = render( | ||
<ThemeProvider theme={createTheme()}> | ||
<form data-testid="login-form"> | ||
<Input type="text" name="username" value="platzily-user" /> | ||
<Input type="password" name="password" value="12345678" /> | ||
<Input type="checkbox" name="rememberMe" checked /> | ||
<button type="submit">Sign in</button> | ||
</form> | ||
</ThemeProvider>, | ||
); | ||
|
||
// act | ||
const InputTestedText = getByTestId('login-form'); | ||
|
||
// assert | ||
expect(InputTestedText).toHaveFormValues({ | ||
omarefg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
username: 'platzily-user', | ||
rememberMe: true, | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './Input'; | ||
export { default } from './Input'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from './Input'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { Input } from '@platzily-ui/components'; | ||
import { createStyleSheet } from '@platzily-ui/styling'; | ||
|
||
const useStyleSheet = createStyleSheet((theme) => ({ | ||
inputStyle: { | ||
padding: theme.spacing(2), | ||
}, | ||
}), { key: 'padding' }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whe could use here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mean of the key is to give a classname prefix. |
||
|
||
const InputComponent = (props) => { | ||
const { classes } = useStyleSheet(); | ||
const { required, type, placeholder, disabled, otherProps } = props; | ||
|
||
return <Input | ||
className={classes.inputStyle} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good use of the useStyleSheet hook 😁. We could change the classname to |
||
type={type} | ||
required={required} | ||
placeholder={placeholder} | ||
disabled={disabled} | ||
{...otherProps} | ||
/>; | ||
}; | ||
|
||
export default InputComponent; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,8 +41,9 @@ | |
"start": "next dev" | ||
}, | ||
"dependencies": { | ||
"@platzily-ui/icons": "0.1.0", | ||
"@lerna/bootstrap": "^4.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this package? |
||
"@platzily-ui/components": "0.1.0", | ||
"@platzily-ui/icons": "0.1.0", | ||
"@platzily-ui/styling": "0.1.4", | ||
"next": "12.0.2", | ||
"nextra": "1.1.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import { Input } from '../../components/styling/index'; | ||
|
||
# Inputs | ||
|
||
Input fields let get the user different type of data. | ||
|
||
|
||
<div style={{marginTop: 50, display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px', backgroundColor:'#F4F8FB', borderRadius:'5px'}}> | ||
<Input placeholder="Hello Input" /> | ||
</div> | ||
|
||
------ | ||
|
||
The component has type='text' by default but the user can change it using the attribute type and his prefer value. | ||
|
||
<div style={{ display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px'}}> | ||
<Input placeholder="Dafault type" /> | ||
|
||
</div> | ||
|
||
```jsx | ||
import { Input } from '@platzily-ui/components'; | ||
|
||
const InputComponent = () => { | ||
|
||
return <Input />; | ||
}; | ||
export default InputComponent; | ||
``` | ||
|
||
The user also can provide principally the disabled and required attribute . | ||
|
||
<div style={{ display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px'}}> | ||
<Input placeholder="disabled" disabled /> | ||
<Input placeholder="*" required /> | ||
</div> | ||
|
||
```jsx | ||
import { Button } from '@platzily-ui/components'; | ||
import { Fragment } from 'react'; | ||
|
||
const InputComponent = () => { | ||
return ( | ||
<Fragment> | ||
<Input placeholder="disabled" disabled /> | ||
<Input placeholder="required" required /> | ||
</Fragment> | ||
); | ||
}; | ||
export default InputComponent; | ||
``` | ||
|
||
|
||
------ | ||
|
||
## Type attribute | ||
|
||
Users can use all his different values in the type attribute like: date, checkbox, password, | ||
|
||
<div style={{ display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px'}}> | ||
<Input type="date" /> | ||
</div> | ||
|
||
```jsx | ||
import { Input } from '@platzily-ui/components'; | ||
|
||
const InputComponent = () => { | ||
|
||
return <Input type="date" />; | ||
}; | ||
export default InputComponent; | ||
``` | ||
|
||
<div style={{ display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px'}}> | ||
<Input type="checkbox" /> | ||
</div> | ||
|
||
```jsx | ||
import { Input } from '@platzily-ui/components'; | ||
|
||
const InputComponent = () => { | ||
|
||
return <Input type="checkbox" />; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the one I'm talking that the weight and height are giving troubles. Making it looks huge. |
||
}; | ||
|
||
export default InputComponent; | ||
``` | ||
|
||
<div style={{ display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px'}}> | ||
<Input placeholder="password" type="password" /> | ||
</div> | ||
|
||
```jsx | ||
import { Input } from '@platzily-ui/components'; | ||
|
||
const InputComponent = () => { | ||
|
||
return <Input type="password" />; | ||
}; | ||
|
||
export default InputComponent; | ||
``` | ||
|
||
## Styling | ||
|
||
The user can provide styles to the component using the createStyleSheet method. | ||
|
||
<div style={{ display: 'flex', alignItems:'center', flexDirection:'column', gap:'15px', justifyContent: 'center', padding: '15px'}}> | ||
<Input placeholder="Styled Input" /> | ||
</div> | ||
|
||
```jsx | ||
import { Input } from '@platzily-ui/components'; | ||
import { createStyleSheet } from '@platzily-ui/styling'; | ||
|
||
const useStyleSheet = createStyleSheet((theme) => ({ | ||
inputStyle: { | ||
backgroundColor: theme.palette.secondary.main, | ||
padding: theme.spacing(2), | ||
}, | ||
}), { key: 'classNameStyle' }); | ||
|
||
const InputComponent = (props) => { | ||
const { classes } = useStyleSheet(); | ||
|
||
return <Input | ||
className={classes.inputStyle} | ||
/>; | ||
}; | ||
|
||
export default InputComponent; | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1446,7 +1446,7 @@ | |
pacote "^11.2.6" | ||
semver "^7.3.4" | ||
|
||
"@lerna/bootstrap@4.0.0": | ||
"@lerna/bootstrap@4.0.0", "@lerna/bootstrap@^4.0.0": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 |
||
version "4.0.0" | ||
resolved "https://registry.yarnpkg.com/@lerna/bootstrap/-/bootstrap-4.0.0.tgz#5f5c5e2c6cfc8fcec50cb2fbe569a8c607101891" | ||
integrity sha512-RkS7UbeM2vu+kJnHzxNRCLvoOP9yGNgkzRdy4UV2hNalD7EP41bLvRVOwRYQ7fhc2QcbhnKNdOBihYRL0LcKtw== | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this package?