-
-
Notifications
You must be signed in to change notification settings - Fork 432
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
fix: homepage - use Link
for internal links
#1001
fix: homepage - use Link
for internal links
#1001
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
<Button href="/docs/getting-started/introduction" size="lg" className="w-full whitespace-nowrap"> | ||
Get started <HiOutlineArrowRight className="ml-2 mt-1 h-4 w-4" /> | ||
</Button> | ||
<Link href="/docs/getting-started/introduction" tabIndex={-1}> |
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.
@SutuSebastian probably here we want to use <Button as={Link} ...>
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.
Already discussed in Discord in the react-library
, TS throws errors when doing that, it does not properly infer & extend the Link
component from NextJS. That is the reason I made it like this.
Do u think we should fix the underlying issue with TS first? Luckily this is the only place href
prop is used that is visible to the user.
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.
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.
As we discussed, I think that we should first fix the button. I'm converting this PR to draft till we get the 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.
#1244 should fix this now @SutuSebastian @rluders
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.
will push the fixes in #1246.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1001 +/- ##
==========================================
- Coverage 99.54% 93.56% -5.99%
==========================================
Files 163 205 +42
Lines 6621 8545 +1924
Branches 401 468 +67
==========================================
+ Hits 6591 7995 +1404
- Misses 30 550 +520
☔ View full report in Codecov by Sentry. |
deprecated by #1246, this one is too outdated. |
Use NextJS
Link
component for internal routes. Avoid full page refresh and improve UX.Before
Screen.Recording.2023-09-26.at.13.36.44.mov
After
Screen.Recording.2023-09-26.at.13.36.22.mov