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: Set up ShadCN-UI #164

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

feat: Set up ShadCN-UI #164

wants to merge 2 commits into from

Conversation

chadstewart
Copy link
Collaborator

Description

With the migration from Chakra-UI to ShadCN-UI, it needed to be set up for the project. This PR is intended to do that.

Related Issue

Resolves:
Resolves #133

Motivation and Context

See issue #133

How Has This Been Tested?

Installed a component using the ShadCN-UI CLI and added installed component into a previously completed component.

Screenshots (if appropriate):

@vercel
Copy link

vercel bot commented Sep 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
techishiring-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2023 1:33pm

@chadstewart
Copy link
Collaborator Author

Hey @marktnoonan, do you mind taking a look at this please? It's nothing crazy but would appreciate a second pair of eyes.

@Wayne-Jones
Copy link

Wayne-Jones commented Oct 5, 2023

@chadstewart Until this is merged into the dev branch could I branch off of 133-set-up-shadcn to work on the other ShadCN implementation related issues?

Comment on lines -17 to -24
xs: "425px",
// => @media (min-width: 425px) { ... }
"2xl": "1440px",
// => @media (min-width: 1440px) { ... }
"3xl": "1920px",
// => @media (min-width: 1920px) { ... }
"pass-max-screen": "1921px"
// => @media (min-width: 1921px) { ... }

Choose a reason for hiding this comment

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

It seems like the screen breakpoints were overridden. I didn't see any use of xs breakpoint but there are some components that use the 2xl, 3xl and pass-max-screen breakpoints

Comment on lines -29 to -36
colors: {
primary: "#165C9C",
secondary: "#101828",
gray: "#667085",
dark: "#000000",
altDark: "#222222",
altWhite: "#F0F0F0"
}

Choose a reason for hiding this comment

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

Colors were also overridden and there are some elements that utilize colors for text

Comment on lines -26 to -27
fontFamily: {
inter: ["Inter", "sans-serif"]

Choose a reason for hiding this comment

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

Not sure if this matters but before the implementation of ShadCN UI, the text used for the website was the default font established by the browser and not the Inter font. So visually there's no change font-wise across the site, but maybe it might be good to use the Inter font with NextJS so that you can use next/font across the website: https://nextjs.org/docs/pages/api-reference/components/font

Copy link

@Wayne-Jones Wayne-Jones left a comment

Choose a reason for hiding this comment

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

Added some comments regarding the ShadCN UI implementation

@Wayne-Jones
Copy link

@chadstewart Any idea when this PR could be implemented?

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.

[Feature]: Set up ShadCN
2 participants