Some components (like Buttons) might end up rendering a tags. That's ok, but in usually you cannot use "links" in SPA - they have to be Links, playing nicely with the router of your choice.
It's expected that SPA is using some router, and only one router, as well as expected to use one, and only one Link.
That "right link" should be configurable via Theme.
Long story short - for example button should read value from the theme, not use hardcoded one.
Every component which might render a link could do it - all you have to provide a right Component instead of default. That's not always handy and _working_ for everyone.
Recently I've seen:
onClick={()=>history.push()} to make them play nicely together with router.All that might be fixed with a single configuration option.
It still would be not possible to use, for example, react-router/Link as a link, due to interface differences, however the wrapper to make Link an a-compatible is just a few lines, and could be provided in MUI documentation as well.
@theKashey Interesting idea. So far, the best answer to this problem is with a wrapper, as documented in https://material-ui.com/guides/composition/#button. It also helps with tree-shaking.
From my experience so far using links with react-router, Next.js, and Gatsby, each of the Link component have their own subtleties. Do you have something like this in mind?
diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.js b/packages/material-ui/src/ButtonBase/ButtonBase.js
index 52f43ad2a..aedf63f22 100644
--- a/packages/material-ui/src/ButtonBase/ButtonBase.js
+++ b/packages/material-ui/src/ButtonBase/ButtonBase.js
@@ -68,6 +68,7 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
disableTouchRipple = false,
focusRipple = false,
focusVisibleClassName,
+ LinkComponent = 'a',
onBlur,
onClick,
onFocus,
@@ -262,7 +263,7 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
let ComponentProp = component;
if (ComponentProp === 'button' && other.href) {
- ComponentProp = 'a';
+ ComponentProp = LinkComponent;
}
const buttonProps = {};
It should not be a part of Button props, to remove requirement to add that extra prop to every Button used in the project.
It's not a problem if you control all Buttons in your own code base, but you can consume third party components, for example Pagination, which might have "buttons as links" inside them. Or, well, just links, as long as links are very friendly citizens at the page - I personally trying to use them as much as I can.
There are actually some complications with using "links" in "third party" code, as long as not only _router Link_, but _route scheme_ is unique from project to project, but still there are cases when it could be useful.
It's also "safe" from tree shaking prospective as long as "link" is usually the part of the router and router is the top-most thing, ie it's already loaded anyway.
So I want to have an ability to set a project + design system wide opinion on "what link is". I could do from the "user-side", but that would affect only the code I control, which is not "all code".
Actually I don't need it for the Button - it shall use Link component, and that is the Link component which shall be configurable via theme
Let's take an example as an example
import { Link as RouterLink } from 'react-router-dom';
import Link from '@material-ui/core/Link';
const LinkBehavior = React.forwardRef((props, ref) => (
<RouterLink ref={ref} to="/getting-started/installation/" {...props} />
));
export default function LinkRouter() {
return (
<Link component={RouterLink} to="/">
With prop forwarding
</Link>
);
}
猬囷笍猬囷笍猬囷笍猬囷笍猬囷笍
const theme = createMuiTheme({
props: {
Link: {
component: RouterLink, // 馃憟 this is what I am asking for
},
},
});
@theKashey I would expect your example to already work.
You are absolutely right - it's working. At least for Link - https://codesandbox.io/s/material-demo-9utdp
It would be great to update docs a bit
However, in this case we are back to the initial problem - Buttons are not using Links, but just a. And I am not sure that they actually could use Links, as long as it brings some styles with it, which Button will prefer not to have.
what about one line change, just to make this moment configurable via theme? I would still prefer to have only one option to be set, but open to any variants :)
const theme = createMuiTheme({
props: {
MuiLink: {
component: RouterLink,
},
MuiButton: {
link: RouterLink, // 馃憟 explicit option for "links-as-buttons"
}
},
});
what about one-line change, just to make this moment configurable via theme? I would still prefer to have only one option to be set, but open to any variants :)
href prop to trigger this behavior is good enough. It could feel weird to the default prop (to?) Maybe wrapping is the best option.It could feel weird to the default prop (to?)
"Link" component should be a-compatible, not Link compatible. __HTML-first__ approach 馃
const ReactRouterAnchor = React.forwardRef(({ href = '', children, ...rest }, ref) => (
<RouterLink
{...rest}
to={href} // 馃憟 now compatible
innerRef={ref}
>
{children}
</Link>
))
By fact it should be a bit more complicated, for example it should keep a for any external links, however in this case it's better explicitly use some ExternalLink component with some rel=noopener applied as well.
Yeah, in this case, the diff in https://github.com/mui-org/material-ui/issues/21533#issuecomment-647411584 would be interesting. We could use it for Gatsby too cc @hupe1980.
Can I take this issue?
Most helpful comment
Can I take this issue?