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: Create custom radio select component #42

Merged
merged 6 commits into from
Nov 20, 2021

Conversation

AmandaMisjuwar
Copy link
Contributor

@AmandaMisjuwar AmandaMisjuwar commented Nov 18, 2021

Notion ticket link

Ticket Name

Implementation description

  • Created RadioSelectButton which is grouped into RadioSelectGroup
  • See Date and Time select as example to see how the radio select group component is used

Steps to test

Screen.Recording.2021-11-17.at.10.49.13.PM.mov
  1. Go to scheduling process's Select Date and Time page and select a daypart. It should update the formValues (you can test using console.log in SelectDateAndTime component)
  2. When you go 'Back' or 'Next' and have selected an option, it should remain selected when returning to this page.

What should reviewers focus on?

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

@AmandaMisjuwar AmandaMisjuwar changed the title feat: Create radio select component feat: Create custom radio select component Nov 18, 2021
Copy link
Contributor

@Shehryar21 Shehryar21 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. Functionality works properly!! Made small comments, but feel free to merge it in after looking at those.

value={daypart}
values={dayparts}
isRequired
onChange={(e: any) => handleChange(e, "daypart")}
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of any, I believe it should be e: React.ChangeEvent<HTMLInputElement>, same in handleChange function.


interface RadioSelectGroupProps {
name: string;
value: any;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
value: any;
value: string;

values: string[];
label: string;
isRequired: boolean;
onChange: any;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onChange: any;
onChange: () => void;

@github-actions
Copy link

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

https://community-fridge-kw--pr42-amanda-feat-radio-co-emrmau2d.web.app

(expires Sat, 27 Nov 2021 20:50:18 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@AmandaMisjuwar AmandaMisjuwar merged commit 656439b into main Nov 20, 2021
@hanlinc27 hanlinc27 deleted the amanda/feat/radio-component branch December 10, 2021 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants