Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[examples] Nextjs Link missing passHref #28588

Closed
2 tasks done
zzossig opened this issue Sep 25, 2021 · 3 comments 路 Fixed by #28661
Closed
2 tasks done

[examples] Nextjs Link missing passHref #28588

zzossig opened this issue Sep 25, 2021 · 3 comments 路 Fixed by #28661
Labels
bug 馃悰 Something doesn't work docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@zzossig
Copy link

zzossig commented Sep 25, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

mui recently updated nextjs-with-typescript example that changes Link component.
The new Link component wraps a tag with styled and this component lost it's native behavior.

Expected Behavior 馃

Native link should

  1. ctrl(or cmd) + click to open browser in a new tab
  2. when mouse hover to a link, it's href displays in the bottom left of a browser.

Steps to Reproduce 馃暪

https://codesandbox.io/s/cranky-bash-2tddh?file=/pages/index.tsx

Context 馃敠

Your Environment 馃寧

same with the codesandbox example.

`npx @mui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@zzossig zzossig added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 25, 2021
@siriwatknp siriwatknp added bug 馃悰 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 27, 2021
@siriwatknp siriwatknp changed the title Link with styled a tag not looks like native link. Possibly bug. [examples] Link with styled a tag not looks like native link. Possibly bug. Sep 27, 2021
@siriwatknp siriwatknp changed the title [examples] Link with styled a tag not looks like native link. Possibly bug. [examples] Nextjs Link missing passHref Sep 27, 2021
@siriwatknp
Copy link
Member

siriwatknp commented Sep 27, 2021

Suggested Fix

diff --git a/examples/nextjs-with-typescript/src/Link.tsx b/examples/nextjs-with-typescript/src/Link.tsx
index 2f54fb257e..7318f42a94 100644
--- a/examples/nextjs-with-typescript/src/Link.tsx
+++ b/examples/nextjs-with-typescript/src/Link.tsx
@@ -28,6 +28,7 @@ export const NextLinkComposed = React.forwardRef<HTMLAnchorElement, NextLinkComp
         replace={replace}
         scroll={scroll}
         shallow={shallow}
+        passHref
         locale={locale}
       >
         <Anchor ref={ref} {...other} />

and in the other /examples

https://codesandbox.io/s/sad-mendeleev-324fk?file=/src/LinkNew.tsx

@siriwatknp siriwatknp added the good first issue Great for first contributions. Enable to learn the contribution process. label Sep 27, 2021
@zzossig
Copy link
Author

zzossig commented Sep 27, 2021

That fixes it.
Thank you.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 27, 2021

It's a regression from #28178. I removed the passHref prop from NextLinkComposed because it has no effects, but I forgot to keep the prop applied inside. I should have manually tested that it keeps working.

Regarding the fix proposed in #28588 (comment), we also need to update the other /examples.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 Something doesn't work docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants