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.

Sign up for GitHub

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

Jump to bottom

Add Chakra UI component library to frontend #65

Merged
merged 4 commits into from
Dec 9, 2021
Merged

Add Chakra UI component library to frontend #65

merged 4 commits into from
Dec 9, 2021

Conversation

sherryhli
Copy link
Member

Ticket link

N/A - this a prerequisite task for some upcoming frontend work.

Implementation description

  • Followed Chakra's installation guide
  • Defined theme according to our design system
  • Made minor modifications to existing starter-code components to improve display

Steps to test

  1. Run the application
docker-compose up --build
  1. Navigate to the frontend and login: http://localhost:3000
  2. Check the design system on the default page
    image

What should reviewers focus on?

  • Chakra best practices for theming

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

@albertlai431 albertlai431 self-requested a review December 8, 2021 01:35
albertlai431
albertlai431 previously approved these changes Dec 8, 2021
View reviewed changes
Copy link
Contributor

@albertlai431 albertlai431 left a comment

Choose a reason for hiding this comment

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

Tested it out and it works as expected! Not too sure about Chakra best practices but code looks good to me 👍

hujoseph99
hujoseph99 reviewed Dec 8, 2021
View reviewed changes
Copy link
Contributor

@hujoseph99 hujoseph99 left a comment

Choose a reason for hiding this comment

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

I've tried looking around to see if there's a better way to implement our text styles, but I don't think there is. I'll continue to take a look, but LGTM for now! 🔥

frontend/src/components/auth/DoneResetPassword.tsx Outdated Show resolved Hide resolved
frontend/src/theme/index.ts Show resolved Hide resolved
frontend/src/theme/index.ts Outdated Show resolved Hide resolved
@sherryhli
Copy link
Member Author

@hujoseph99 Thanks for the suggestions, updated PR with changes!

@sherryhli sherryhli requested a review from hujoseph99 December 9, 2021 00:48
hujoseph99
hujoseph99 approved these changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@hujoseph99 hujoseph99 left a comment

Choose a reason for hiding this comment

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

LGTM! 🔥🚢🇮🇹

@sherryhli sherryhli merged commit 58e6b01 into main Dec 9, 2021
@sherryhli sherryhli deleted the sherry/add-chakra-ui branch December 9, 2021 00:55
sherryhli added a commit that referenced this pull request Jan 6, 2022
@sherryhli
Add Chakra UI component library to frontend (#65)
9d186cb 
* Install Chakra UI packages

* Define custom theme with colours and typography

* Integrate Chakra into frontend

* Set default textStyle and minor refactoring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hujoseph99 hujoseph99 hujoseph99 approved these changes

@albertlai431 albertlai431 albertlai431 left review comments

@7tint 7tint Awaiting requested review from 7tint

@jiarucao jiarucao Awaiting requested review from jiarucao

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sherryhli @hujoseph99 @albertlai431