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: extract calendar component and refactor scheduling flow #94

Merged
merged 61 commits into from
Jan 23, 2022

Conversation

carissa-tang
Copy link
Contributor

@carissa-tang carissa-tang commented Jan 19, 2022

Brief description. What is this change?

Scheduling Flow: Extract calendar component

Implementation description. How did you make this change?

  • Extracted calendar from input field
  • Made form progressive (reveals one section at a time)
  • Moved back/next buttons & disable feature added to "next"
  • "End date" input field was changed to date picker
  • Overall styling was tweaked to match Figma designs
  • Note: Was unable to remove "," from Calendar and to only display two weeks -> limitation of the library used (sorry Stacy!)

Steps to test

  1. Click through the form to see if each question is revealed one after the other
  2. Ensure all buttons are clickable ("end date" field should only appear if not one time donation)
  3. Cross-check both Desktop and Mobile view with Figma designs

Desktop View: https://user-images.githubusercontent.com/69697180/150065707-0ebb5bdc-8727-47d1-a2ab-bbcc6f5ffffa.mp4

  • @hanlinc27 Inconsistency with radio buttons is apparent from 0:24-0:30

Mobile View: https://user-images.githubusercontent.com/69697180/150065486-d421a912-963d-42bb-a499-b04f58014fbb.mov

Update: Steps 2 and 3 are now refactored as well

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages follow conventional commits and are descriptive. 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
  • The appropriate tests if necessary have been written

@carissa-tang carissa-tang requested a review from a team January 19, 2022 05:18
@carissa-tang carissa-tang self-assigned this Jan 19, 2022
@github-actions
Copy link

github-actions bot commented Jan 19, 2022

Visit the preview URL for this PR (updated for commit b7dd31a):

https://communityfridgekw-staging--pr94-carissa-refactor-sch-n2wxi48w.web.app

(expires Sun, 30 Jan 2022 21:44:45 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Member

@hanlinc27 hanlinc27 left a comment

Choose a reason for hiding this comment

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

this is a really good first pr!!! @carissa-tang
first pass comments, will test it out locally tomorrow or friday! :)

@carissa-tang

This comment has been minimized.

Copy link
Member

@xsharonhe xsharonhe left a comment

Choose a reason for hiding this comment

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

Just some general thoughts 😄 Except for that one bug, everything is working as intended so that's fantastic! Really nice work 😸

feel free to add to the discussion and agree / disagree with some things i said

@hanlinc27 hanlinc27 force-pushed the carissa/refactor-scheduling-flow branch from 1c1915d to 7cacb8e Compare January 23, 2022 07:32
@hanlinc27 hanlinc27 force-pushed the carissa/refactor-scheduling-flow branch from 7cacb8e to 62dacd4 Compare January 23, 2022 07:43
@hanlinc27 hanlinc27 merged commit 26c0f90 into staging Jan 23, 2022
@hanlinc27 hanlinc27 deleted the carissa/refactor-scheduling-flow branch January 23, 2022 22:43
@hanlinc27 hanlinc27 restored the carissa/refactor-scheduling-flow branch February 13, 2022 23:27
@hanlinc27 hanlinc27 deleted the carissa/refactor-scheduling-flow branch April 6, 2022 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants