-
-
Notifications
You must be signed in to change notification settings - Fork 24
docs: add props-table into button docs #446
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
Conversation
|
0257ed0 to
dc76fd8
Compare
dc76fd8 to
5c9853f
Compare
| export function Table() { | ||
| return <></> | ||
| } |
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.
Remove empty function
| return <Box as="th" padding="0.5rem 1rem" textAlign="left" {...props} /> | ||
| } | ||
|
|
||
| export { TableBody, TableCell, TableHead, TableHeaderCell, TableRoot, TableRow } |
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.
Add export with function declaration
| return ( | ||
| <Box | ||
| as="thead" | ||
| {...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.
Devup UI Works with static analysis.
So props for stying can not be applied...
|
@owjs3901 I've committed the changes to address the current issues. However, I'd like to raise a concern about the The existing lint rule enforces that each file must export a component with the same name as the file. While I understand this rule was likely designed to prevent namespace component patterns (which devup-ui doesn't support), it's inadvertently blocking the compound component pattern, which devup-ui does support.
This structure makes it difficult to use the compound component pattern. I suggest that modify the |
|
I agree with your point to some extent. That said, since the design allows exporting other functions from the same file as long as the main exported function matches the file name, I believe the pattern you mentioned can still be fully implemented. |
| return ( | ||
| <Box | ||
| as="code" | ||
| bg="var(--containerBackground)" |
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.
using $containerBackground
| import { Box } from '@devup-ui/react' | ||
| import { ComponentPropsWithoutRef } from 'react' | ||
|
|
||
| export const TableRoot = ({ ...props }: ComponentPropsWithoutRef<'table'>) => { |
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’m just curious why did you exclude the ref props?
Why aren’t you using ComponentProps?
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.
Thanks for pointing that out!
If your question about ref of React 19, I actually missed that — I was assuming a pre-React-19 environment when implementing this. I’ll update it accordingly.
Otherwise, the reason is that ref isn’t treated as a normal prop in React.
To pass it down to a child component, forwardRef must be used.
Since this component doesn’t need to handle a ref, I didn’t include it.
| @@ -0,0 +1,10 @@ | |||
| import { Box } from '@devup-ui/react' | |||
| import { ComponentPropsWithoutRef } from 'react' | |||
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.
Add type keyword plrease
…and update PropsTable accordingly
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
docs: add props-table into button docs
Summary
PropsTableMDX component that composes the shared table primitives and renders prop metadata with markdown-aware code blocks@devup-ui/react