-
Notifications
You must be signed in to change notification settings - Fork 0
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
Likhith/76940/ts migrate toggle switch toast tick progress #37
Changes from 4 commits
84d93fe
0547239
21bd5d4
73b978f
73ff21f
9495f69
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 |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import TickProgress from './tick-progress.jsx'; | ||
import TickProgress from './tick-progress'; | ||
import './tick-progress.scss'; | ||
|
||
export default TickProgress; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,19 @@ | ||
import classNames from 'classnames'; | ||
import PropTypes from 'prop-types'; | ||
import React from 'react'; | ||
|
||
const Tick = ({ is_on }) => { | ||
type TTick = { | ||
is_on?: boolean; | ||
}; | ||
|
||
type TTickProgress = { | ||
className: string; | ||
columns: number; | ||
value: number; | ||
rows: number; | ||
size: number; | ||
}; | ||
|
||
const Tick = ({ is_on }: TTick) => { | ||
return ( | ||
<div | ||
className={classNames('dc-tick-progress__tick', { | ||
|
@@ -12,7 +23,7 @@ const Tick = ({ is_on }) => { | |
); | ||
}; | ||
|
||
const TickProgress = ({ className, rows, columns, size, value }) => { | ||
const TickProgress = ({ className, rows = 2, columns = 5, size = 10, value = 0 }: Partial<TTickProgress>) => { | ||
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. @likhith-deriv Same 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. I thought of using the interfaces that React provides. I don't think Partial makes the code less readable |
||
return ( | ||
<div | ||
className={classNames('dc-tick-progress', className)} | ||
|
@@ -21,26 +32,11 @@ const TickProgress = ({ className, rows, columns, size, value }) => { | |
gridTemplateColumns: `repeat(${columns}, 1fr)`, | ||
}} | ||
> | ||
{new Array(size).fill(null).map((item, index) => { | ||
{new Array(size).fill(null).map((_, index) => { | ||
return <Tick is_on={index < value} key={index} />; | ||
})} | ||
</div> | ||
); | ||
}; | ||
|
||
TickProgress.defaultProps = { | ||
columns: 5, | ||
rows: 2, | ||
size: 10, | ||
value: 0, | ||
}; | ||
|
||
export default TickProgress; | ||
|
||
TickProgress.propTypes = { | ||
className: PropTypes.string, | ||
columns: PropTypes.number, | ||
value: PropTypes.number, | ||
rows: PropTypes.number, | ||
size: PropTypes.number, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import Toast from './toast.jsx'; | ||
import Toast from './toast'; | ||
import './toast.scss'; | ||
|
||
export default Toast; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,7 +3,24 @@ import PropTypes from 'prop-types'; | |||||
import React from 'react'; | ||||||
import { CSSTransition } from 'react-transition-group'; | ||||||
|
||||||
const Toast = ({ children, className, is_open = true, onClose, onClick, type = 'info', timeout = 0 }) => { | ||||||
type TToast = { | ||||||
className: string; | ||||||
is_open: boolean; | ||||||
onClick: (event: React.MouseEvent<HTMLElement>) => void; | ||||||
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.
Suggested change
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. 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. @likhith-deriv It seems like you have given onClick: React.MouseEventHandler; Or to be more specific: onClick: React.MouseEventHandler<HTMLElement>; |
||||||
onClose: () => void; | ||||||
type: 'error' | 'info'; | ||||||
timeout: number; | ||||||
}; | ||||||
|
||||||
const Toast = ({ | ||||||
children, | ||||||
className, | ||||||
is_open = true, | ||||||
onClose, | ||||||
onClick, | ||||||
type = 'info', | ||||||
timeout = 0, | ||||||
}: React.PropsWithChildren<Partial<TToast>>) => { | ||||||
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. @likhith-deriv Why use 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 thought of using the interfaces that React provides. I don't think Partial makes the code less readable 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. @likhith-deriv The props type of a component defines the signature of that component, As a dev, I will be looking at the type definition to understand what this component needs to work, So if I check the type I will see everything as required and I can easily miss the part It's nothing important tho 😅 Let's move forward 🙇🏻 |
||||||
const [is_visible, setVisible] = React.useState(false); | ||||||
|
||||||
React.useEffect(() => { | ||||||
|
@@ -56,14 +73,4 @@ const Toast = ({ children, className, is_open = true, onClose, onClick, type = ' | |||||
); | ||||||
}; | ||||||
|
||||||
Toast.propTypes = { | ||||||
className: PropTypes.string, | ||||||
is_open: PropTypes.bool, | ||||||
onClick: PropTypes.func, | ||||||
onClose: PropTypes.func, | ||||||
type: PropTypes.oneOf(['error', 'info']), | ||||||
timeout: PropTypes.number, | ||||||
children: PropTypes.oneOfType([PropTypes.node, PropTypes.array]), | ||||||
}; | ||||||
|
||||||
export default Toast; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import ToggleSwitch from './toggle-switch.jsx'; | ||
import ToggleSwitch from './toggle-switch'; | ||
import './toggle-switch.scss'; | ||
|
||
export default ToggleSwitch; |
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.
if it only has one argument, why not typing it here instead of having
TTick
? 🤔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.
It can be changed as u suggested.