-
Notifications
You must be signed in to change notification settings - Fork 188
Convert Breadcrumbs v2 to Typescript #1058
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Size Change: -737 B (0%) Total Size: 490 kB
ℹ️ View Unchanged
|
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.
Minor tweaks, but otherwise great work ⭐
} | ||
|
||
export const CrumbSimpleMenu = ({ children, ...props }: CrumbSimpleMenuProps) => ( | ||
<SimpleMenu icon={<CarretDown />} as={StyledButton} size="S" {...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.
Where did the noBorder
prop go? I'm not sure, but I think to remember it is needed, because it behaves, but doesn't look like a simple menu.
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 the type definitions are wrong? I'm not sure. In any case: the menu looks broken: https://design-system-git-chore-breadcrumbs-ts-strapijs.vercel.app/?path=/story/design-system-components-v2-breadcrumbs--base

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.
@simotae14 did you solve this puzzle?
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've also created an issue, because this is already broken on main: #1090
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.
ok then I can take a look at the issue. Maybe in the meantime you can review this PR which is unrelated to the bug
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.
Compared this version to the current version on main ✅
What does it do?
Converts
Breadcrumbs
to TypescriptRelated issue(s)/PR(s)
#983