-
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
Feat/add listing component #82
Conversation
|
||
const BaseCard = ({title, content, className}: IBaseCard): JSX.Element => { | ||
return ( | ||
<div className={className + containerClassName}> |
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.
Lets use the classnames library
const ColoredCircle = ({quantity, type}: IColoredCircle): JSX.Element => { | ||
const backgroundColor = type === ComponentType.Error ? "bg-lightRed" : "bg-yellow"; | ||
return( | ||
<div className={backgroundColor + " rounded-full text-black h-6 min-w-6 flex justify-center"}> |
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.
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.
maybe we should use https://ui.shadcn.com/docs/components/badge ?
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.
I don't think this is necessary, it is a simple div
{errors && errors > 0 ? ( | ||
<ColoredCircle quantity={errors} type={ComponentType.Error}/> | ||
) : <></>} | ||
{warnings && warnings > 0 ? ( | ||
<ColoredCircle quantity={warnings} type={ComponentType.Warning}/> | ||
) : <></>} |
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.
Nit: It will be better organized if we put this logic before the return
Error, | ||
} | ||
|
||
const ColoredCircle = ({quantity, type}: IColoredCircle): JSX.Element => { |
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.
Lets extract this component into its own directory/file since it is more generic and we will use it in several other places
b467789
to
431b610
Compare
@@ -0,0 +1,25 @@ | |||
import classNames from 'classnames'; | |||
|
|||
import { ComponentType } from '../Cards/ListingComponentCard/ListingComponentCard'; |
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.
nit: this backwards relative import make it a little hellish to refactor, you can add a shortcut for @/components
(I prefer @/components over @components to differentiate from npm libraries) in the .tsconfig
file
https://stackoverflow.com/questions/43281741/how-can-i-use-paths-in-tsconfig-json
431b610
to
5b30274
Compare
- add base card - add listing card component - add listing card component to storybook
5b30274
to
60d0ed5
Compare
Description
Related Issues
How to test it
cd dashboard pnpm run storybook
Visual reference