-
Notifications
You must be signed in to change notification settings - Fork 1
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
Additional refinements #387
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
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.
Would love it if we could just remove the accordion stuff in this PR (see comment)
@@ -35,7 +35,7 @@ AccordionTrigger.displayName = "AccordionTrigger"; | |||
const AccordionTriggerIcon = ({ className, ...props }: Omit<IconProps, "svg">) => ( | |||
<Icon | |||
{...props} | |||
svg={<CaretDown weight="fill" />} |
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 think this treatment is just too thin and disappears in this context (at least). These also need to be updated with the carets as the leftmost item. Also wondering if we should just allow the user to choose which icon weight for these depending on the context.
Could we just remove this part from this PR?
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.
yeah, the accordion component is getting a makeover soon and there's gonna be multiple variants
Still more documentation stragglers. Accordion carets as well.