Closed
Conversation
❌ Deploy Preview for cheerful-crostata-e27410 failed.
|
❌ Deploy Preview for hover-design failed.
|
pruthvi2103
suggested changes
Aug 2, 2022
Contributor
pruthvi2103
left a comment
pruthvi2103
left a comment
There was a problem hiding this comment.
Some Comments @Ac-Srikanth
| | `id` | string | Yes | | ||
| | `name` | string | No | | ||
| | `value` | string | No | | ||
| | `label` | string | No | |
Contributor
There was a problem hiding this comment.
label should be provided as a separate component as <Label> it helps keep label and input far away and customisable layout wise
Author
There was a problem hiding this comment.
Yeah ok that makes sense. Will change it.
| | `value` | string | No | | ||
| | `label` | string | No | | ||
| | `isChecked` | boolean | No | | ||
| | `onChange` | function | No | |
Contributor
There was a problem hiding this comment.
other props look great, can we expose more props to customize the styling aspect of the component? eg: changing the color.
Also do we need a radioGroup? as other libraries use. Any merits demerits of those?
Author
There was a problem hiding this comment.
- Yeah about the color i wanted to have a discussion . Is there a mock for different color's and how much customisation should we offer in terms of color .
- About the Radio Group there are merits and demerits as well.
- Since most of the cases radio will be used in a group it makes sense.Very rarely we would use a single radio button on its own.
- But if do have radio group we should make it robust. Like a vertical aligned radio group, or horizontal aligned radio group and all kinds of customisation.But not really sure if this is what hover is looking for now.
- Yes over the long term radio group will make a lot of sense.
Contributor
|
Please pull main into your branch and commit? |
Contributor
|
closing in favour of #85 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Types of changes
Checklist: