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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add universal chakra link component #243

Merged
merged 6 commits into from
Jan 10, 2022
Merged

Conversation

tundera
Copy link
Contributor

@tundera tundera commented Dec 8, 2021

Description of PR that completes issue here...

Changes

  • Adds a new Link component for both dynamic and external links compatible with Next.js and Chakra UI
  • Maintains theming support and as casting (i.e. <Button as={Link} href="/" variant="navigation" />)

Checklist

  • Requires dependency update?
  • Generating a new app works

Copy link

@mcavaliere mcavaliere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but ask others who are more active on the project to look as well.

One recommendation: you might want to wrap other clickable components as well for convenience rather than forcing buttons to use the as prop - I did something like this for the CITJS website https://github.com/echobind/jamstack-ebook-website/blob/main/components/Link/index.tsx

@tundera
Copy link
Contributor Author

tundera commented Dec 8, 2021

@mcavaliere Cool idea, I like it 💯 I updated the PR to have a convenience wrapper for Button and BreadcrumbLink as well

Copy link
Contributor

@kgajera kgajera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed lint and TypeScript errors which are causing the CI failure.

packages/create-bison-app/template/layouts/LoggedOut.tsx Outdated Show resolved Hide resolved
packages/create-bison-app/template/components/Link.tsx Outdated Show resolved Hide resolved
@tundera tundera requested a review from kgajera January 1, 2022 20:21
@tundera tundera merged commit 4265bfa into canary Jan 10, 2022
@tundera tundera deleted the universal-chakra-link branch January 10, 2022 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants