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

Install Chakra UI #29

Merged
merged 10 commits into from
Mar 8, 2024
Merged

Install Chakra UI #29

merged 10 commits into from
Mar 8, 2024

Conversation

carolynzhang18
Copy link
Contributor

@carolynzhang18 carolynzhang18 commented Feb 26, 2024

Notion Ticket Link

Install Chakra UI

Implementation Description

  • The actual installation changes are mostly in the frontend package.json and yarn.lock. The rest of the changes were just small fixes to accommodate the updated packages.

Steps to Test

  1. Delete your frontend/node_modules folder. In the frontend directory (cd frontend), run yarn install to get a fresh copy of the packages. This won't affect what Docker gives you, but it'll make the local package errors go away.
  2. After running docker compose down, rebuild the containers with docker compose up --build.
  3. Expected: app compiles with no errors*.

* The login page will probably show "Script error" (and you'll see something about missing the client_id parameter in the console). Don't worry about it, the "Remove Google Auth for Login" ticket will make the error go away eventually.

Screenshots (optional)

Here's what the Chakra UI button looks like:
image
If you want to also plop a component somewhere to try it out, check out the docs: https://chakra-ui.com/docs/components

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

Linting & Formatting

Mac

docker exec -it CWC-backend /bin/bash -c "yarn fix"
docker exec -it CWC-frontend /bin/bash -c "yarn fix"

Windows

docker exec -it CWC-backend bash -c "yarn fix"
docker exec -it CWC-frontend bash -c "yarn fix"

@owen-sellner owen-sellner requested a review from SohailSayed March 1, 2024 01:32
Copy link
Contributor

@owen-sellner owen-sellner left a comment

Choose a reason for hiding this comment

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

I realized while testing your code that the functions in frontend/src/APIClients are never called in our application 😭

frontend/.eslintrc.js Outdated Show resolved Hide resolved
frontend/src/APIClients/SimpleEntityAPIClient.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@owen-sellner owen-sellner left a comment

Choose a reason for hiding this comment

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

I left some comments about the error handling and alert issues 🫡

frontend/src/components/auth/Login.tsx Show resolved Hide resolved
frontend/src/APIClients/SimpleEntityAPIClient.ts Outdated Show resolved Hide resolved
@owen-sellner owen-sellner requested a review from debs415 March 8, 2024 00:10
@carolynzhang18 carolynzhang18 force-pushed the carolyn-aathi/install-chakra-ui branch from 01f4730 to 9baab63 Compare March 8, 2024 00:46
@owen-sellner owen-sellner self-requested a review March 8, 2024 00:46
@SohailSayed
Copy link
Contributor

Followed the steps, had no issues compiling on my end

Copy link
Contributor

@debs415 debs415 left a comment

Choose a reason for hiding this comment

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

Great work on this Chakra UI ticket! The changes look good, and following the steps you laid out to test, I could run smoothly and get a few Chakra components added to the login page as well, so it seems to work. Once again, great job on this one! :)

Copy link
Contributor

@owen-sellner owen-sellner left a comment

Choose a reason for hiding this comment

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

I double-checked that the try-catch statement passes the linter. This is ready to merge, good job!!!

@carolynzhang18 carolynzhang18 merged commit cbafe5f into main Mar 8, 2024
1 check passed
@carolynzhang18 carolynzhang18 deleted the carolyn-aathi/install-chakra-ui branch March 8, 2024 03:27
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