-
Notifications
You must be signed in to change notification settings - Fork 107
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
refactor(DropdownItem): make wrapping of children with anchor tag optional #386
Conversation
👷 Deploy Preview for react-daisyui processing.
|
1 similar comment
👷 Deploy Preview for react-daisyui processing.
|
✅ Deploy Preview for react-daisyui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
@kodjunkie I like the idea, thanks for the PR! My one concern is that then the props don't match (the props are currently typed as React.AnchorHTMLAttributes<HTMLAnchorElement>
) and cannot be passed thru to any effect. If you can solve this issue, I'm happy to approve!
Yes, that's the case for users who want to use the default wrapped anchor tag. If you're passing a full react node as |
Right but the component's props are still typed as anchor attributes and it will still accept props as if that were the underlying element. Because of this a developer could pass in props but nothing will happen, which would be a misleading developer experience. You could use something like a Discriminated Union which will only type it as |
You're right, I just pushed a fix. |
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.
@kodjunkie thanks for making the change, looks good!
I believe this solves #144 and also, allows the usage of the new updates introduced to daisyUI v3 related to Menu (eg: Menu with active item). The default behavior is the case where the
children
is wrapped in an anchor tag so it doesn't cause a breaking change.