[ButtonUnstyled] Update to render as link when href or to is provided #34337
Conversation
… without component or components.Root props
EduardoSCosta
requested review from
michaldudak and
mnajdova
as code owners
|
michaldudak
added
component: button
| const rootProps: WithOptionalOwnerState<ButtonUnstyledRootSlotProps> = useSlotProps({ | ||
| elementType: Root, | ||
| getSlotProps: getRootProps, | ||
| externalForwardedProps: other, | ||
| externalForwardedProps: { ...other, href: other.href ?? other.to }, |
There was a problem hiding this comment.
Can you expand on why this is necessary?
There was a problem hiding this comment.
Can you expand on why this is necessary?
Without this, the href attribute doesn't appear in the html when passing only the to prop.
There was a problem hiding this comment.
The to prop exists to make it easier to work with router links. It's not meant to be used as a replacement for the href. I'd pass through all the props as they are.
Side note:
This got me thinking if the Button component is doing too much. We're basically encapsulating the behavior for links and buttons under the button component. Perhaps having a LinkButtonUnstyled (or LinkUnstyled) component would allow the implementation to be cleaner? Any thoughts, @mnajdova?
There was a problem hiding this comment.
This got me thinking if the Button component is doing too much. We're basically encapsulating the behavior for links and buttons under the button component. Perhaps having a LinkButtonUnstyled (or LinkUnstyled) component would allow the implementation to be cleaner? Any thoughts, @mnajdova?
Button as link and Link component in my opinion are different components. On many web pages there are buttons that act as links, so supporting it in the Button component makes sense. Of course I am saying this from UI perspective, as those button looks like buttons, not links.
The Link component in my opinion is an underlined text that contains a link, which is a different UI component. So I would say 👍 for adding link component, but 👎 for removing this logic from the Button :)
github-actions
bot
added
PR: out-of-date
|
I resolved the conflicts caused by the recent prop renaming. |
|
Could you please merge in the latest master? Codecov shows something weird. |
…mui#34337) Co-authored-by: Michał Dudak <michal@dudak.me>
…mui#34337) Co-authored-by: Michał Dudak <michal@dudak.me>
This PR resolve #32870
With this change, the
ButtonUnstyledcomponent renders as anaelement whenhrefortoprops are present, but neithercomponentnorcomponents.Rootare provided.