-
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(#19): add table UI #32
Conversation
4ef873a
to
9a0a51d
Compare
824a81e
to
2515d29
Compare
9b680b3
to
f224830
Compare
"lightGray": '#F4F4F4', | ||
"mediumGray": '#EAEAEA', | ||
"darkGray": '#D6D6D6', | ||
"black": '#000000' |
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.
this color already exists, dark
color has the same value as black
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.
could you separate this change from this commit?
9a0a51d
to
bb0f90d
Compare
@@ -71,7 +71,7 @@ const LateralMenu = (): JSX.Element => { | |||
orientation="vertical" | |||
> | |||
<div className="w-full px-4"> | |||
<img src="./src/assets/kernelCI_logo.png"className="size-14 text-onSecondary" /> | |||
<img src="./src/assets/kernelCI_logo.png"className="size-14 text-white" /> |
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.
why does an img
have a text style?
BaseTable({ | ||
headers: treeTableColumns, | ||
body: <>{treeTableBody}</> | ||
}) |
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.
why are you using BaseTable as a simple function and not as a component?
@@ -20,7 +20,7 @@ const TableHeader = React.forwardRef< | |||
HTMLTableSectionElement, | |||
React.HTMLAttributes<HTMLTableSectionElement> | |||
>(({ className, ...props }, ref) => ( | |||
<thead ref={ref} className={cn("[&_tr]:border-b", className)} {...props} /> | |||
<thead ref={ref} className={cn("[&_tr]:border-b border-darkGray", className)} {...props} /> |
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.
Let's try not to modify the shadcn components, we can pass the style with the className
prop
f224830
to
cdd9a5b
Compare
"lightGray": '#F4F4F4', | ||
"mediumGray": '#EAEAEA', | ||
"darkGray": '#D6D6D6', | ||
"black": '#000000' |
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.
could you separate this change from this commit?
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.
nice
- add a basic table component - add a tree table component
cdd9a5b
to
a43c28b
Compare
Description
Related Issues