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

fix: fixed inconsistent control type for 'radio' #309

Merged
merged 3 commits into from
Jan 8, 2022

Conversation

Karthik-B-06
Copy link
Contributor

@Karthik-B-06 Karthik-B-06 commented Dec 26, 2021

Issue: Fixing issue #308

What I did

Changed the radio control to work with both Mobile and Web by changing the ControlType string of radios to radio.

How to test

Please explain how to test your changes and consider the following questions

  • Does this need a new example in examples/native?

No, since the original docs of the storybook include the key with 'radio'

  • Does this need an update to the documentation?
    No, the changes are accommodated in the storybook original docs.
    Screenshot 2021-12-26 at 11 14 29 AM

If your answer is yes to any of these, please make sure to include it in your PR.

Changed the radio control to work with both Mobile and Web by changing the ControlType string of `radios` to `radio`.
@Karthik-B-06 Karthik-B-06 changed the title Fixed inconsistent control type for 'Radio' WIP : Fixed inconsistent control type for 'Radio' Dec 26, 2021
@Karthik-B-06 Karthik-B-06 marked this pull request as draft December 26, 2021 13:13
@dannyhw
Copy link
Member

dannyhw commented Dec 26, 2021

Thanks for your contribution 🙇‍♂️. Will properly review soon. Seems ok to me.

I was thinking it could work for both radios and radio though.

Also please update the example in the example app :).

@Karthik-B-06
Copy link
Contributor Author

I thought having it work with both radio and radios would create unnecessary confusion. So I chose to use the control type we have in Web. Added the change to Example App too :)

@dannyhw
Copy link
Member

dannyhw commented Dec 27, 2021

@Karthik-B-06 ok that makes sense. Looks good. Did you run the example to check it still works? I can't run it right now because I'm away for the holidays.

Looks good to me though.

Thanks again for contributing :).

If you can just confirm the radio example works on the example app I'm happy to merge it.

@Karthik-B-06 Karthik-B-06 marked this pull request as ready for review December 29, 2021 06:30
@Karthik-B-06
Copy link
Contributor Author

Hey @dannyhw I will get back after checking if the radio control type works on the example app.

@dannyhw dannyhw changed the title WIP : Fixed inconsistent control type for 'Radio' fix: fixed inconsistent control type for 'radio' Jan 8, 2022
Copy link
Member

@dannyhw dannyhw left a comment

Choose a reason for hiding this comment

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

Just tested the example its looking good, thanks for your contribution 🙏 .

Sorry it took so long I've been away for the Christmas break.

@dannyhw dannyhw merged commit df2a848 into storybookjs:next-6.0 Jan 8, 2022
dannyhw pushed a commit to raychanks/react-native that referenced this pull request Feb 7, 2022
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.

2 participants