-
Notifications
You must be signed in to change notification settings - Fork 3
Update the button component #18
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
base: master
Are you sure you want to change the base?
Conversation
|
@ayushjainrksh I started a review but have to jump off and continue later. Posted few comments, more to follow |
|
Okay, no problem. |
jevakallio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking good! We need to change the multiline comment format, and get a decision from Moti regarding the default props annotations, but after those this is good to go
jevakallio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! One minor question in line notes
* Update the title and prop description from docs. * Add default value to props. * Update platforms in props with platform annotation.. * Remove asterisks from the comments. * Add snack player and update multiple platform format. * Add example annotation to snackplayer
759463f to
5147bb0
Compare
a05efc6 to
cc92ab4
Compare
Summary: Fixes MLH-Fellowship#33 The PR is part of an effort to update the code comments to match the current documentation on the React Native website. The project is a part of MLH fellowship program and involves automatic generation of the website docs from code comments and flow types as the end result. To learn more about the project you can visit the project wiki: - [Project details](https://github.com/MLH-Fellowship/0.4.x-projects/wiki/React-Native-Flowtype-API-Docs-Generator) - [RN Docs Standards](https://github.com/MLH-Fellowship/react-native/wiki/RN-Docs-standards) Link to the documentation(the source of truth): - [button.md](https://github.com/MLH-Fellowship/react-native-website/blob/master/docs/button.md) ## Changes * Update the title and prop description from docs. * Remove unnecessary `*` from the code comments. * Add default value to props. * Specify the default value of a prop in the code comments with `default` annotation. * For multiple defaults (different devices) use `default {platform android/ios}`. * Update platforms in props. * Use comma separated string with `platform` decorator. * Add Snack player example specified in the docs to the code comments with `example` annotation. ## Changelog [Internal] Pull Request resolved: #29155 Test Plan: All changes are made to the code comments and thus there is no need for testing. [Reviewed by jevakallio](MLH-Fellowship#18) Reviewed By: cpojer Differential Revision: D22767877 Pulled By: motiz88 fbshipit-source-id: ccad8d58dc2888d44d34fdbb94b3c187542e9f2b
Fixes #33
Summary
*from the code comments.An attempt to solve the problem of specifying the default value of a prop in a component.I added the default values to the props whiledestructuringtheir actual values.This would not only help the docs to get the default prop value(when automated) but also reduce hardcoded styles/values in the code.@defaultannotation.@default {@platform android/ios}.@platformdecorator.@exampleannotation.