-
-
Notifications
You must be signed in to change notification settings - Fork 422
Adding Readme.md and docs #7
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
Adding Readme.md and docs #7
Conversation
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.
Nice work, mostly small comments. The bigger one is ordering of sections. Imho I think the generic flow should be
- Installing/adding the component to your project
- Using the React Component with examples
- API/props
- Migration examples
- Contributing to the component
5a. clone, install
5b. how to test
5c. running the example
In addition a TOC might be useful on top (index 0) of the readme
- Using CocoaPods: | ||
|
||
1. Install CocoaPods, here the [installation guide](https://guides.cocoapods.org/using/getting-started.html). | ||
2. Inside the iOS folder run `pod init`, this will create the initial `pod` file. |
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.
This section is ok to have, but it feels like the focus should be more on adding RNDateTimePicker
from the podspec. Right now it is focused on setting up the consuming app, which most avid cocoapods users might have already done when they initialized their own project.
README.md
Outdated
end | ||
``` | ||
4. Run `pod install` inside the same folder where the `pod` file was created | ||
5. Open the `.xcworkspace` file that was created and run |
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.
run what? Also why go through xcode, the norm is using the react-native
CLI, e.g. react-native run-{ios,android}
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.
run from Xcode, I will change it to use CLI.
README.md
Outdated
4. Add the import and link the package in `MainApplication.java`: | ||
|
||
```java | ||
import com.reactcommunity.rndatetimepicker.RNDateTimePickerPackage; // <-- add this import |
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.
rather than using // <-- add this import
, use ```diff
to denote what should be added or changed
README.md
Outdated
Defines the maximum date that can be selected. | ||
|
||
```js | ||
<RNDateTimePicker maximumDate={new Date()} /> |
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.
Make this an actual example, not the current Date
. Year 2250 or something
Defines the date or time value used in the component. | ||
|
||
```js | ||
<RNDateTimePicker value={new Date()} /> |
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.
This is the default for value
, specifying a different value or an example with state/ HOC props would be nice to have.
README.md
Outdated
Defines the minimum date that can be selected. | ||
|
||
```js | ||
<RNDateTimePicker minimumDate={new Date()} /> |
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.
Same as above
cad606c
to
f5e4e8d
Compare
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.
- Limit your self to a 80 cols of text space, some of the code examples are impossible to read without scrolling sideways for example.
- Add a table of contents and link to the different API’s
- Would love to see a compatiblity table.
- Reference the license
- Screenshots? People love screenshots.
README.md
Outdated
|
||
## Getting started | ||
|
||
`npm install react-native-datetimepicker --save` |
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.
Use 3x backticks for code, instead of 1, add bash as language identifier.
README.md
Outdated
|
||
or | ||
|
||
`yarn add react-native-datetimepicker` |
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.
See backtick
README.md
Outdated
|
||
### Install using react-native link | ||
|
||
`react-native link react-native-datetimepicker` |
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.
See backtick
README.md
Outdated
|
||
### `timeZoneOffsetInMinutes` (`optional`, `iOS only`) | ||
|
||
Allows to change the timeZone of the date picker, by default it uses the device's time zone. |
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.
Allows changing of the
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.
I would also suggest replacing the comma with a .
README.md
Outdated
|
||
### `locale` (`optional`, `iOS only`) | ||
|
||
Allows to change the locale of the component, by default it uses the device's locale. |
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.
Allow schanging of the
README.md
Outdated
|
||
### `is24Hour` (`optional`, `Android only`) | ||
|
||
Allows to set the time picker to 24hour format. |
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.
Allows changing of the time picker to a 24 hour format
README.md
Outdated
|
||
RNDateTimePicker is the new common name used to represent the old versions of iOS and Android. | ||
|
||
On android, open picker modals will update the selected date and/or time if the prop `value` changes. For example, if a HOC holding state, updates the `value` prop. Previously the component used to close the modal and render a new one on consecutive calls. |
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.
Android
README.md
Outdated
|
||
## Migration from the older components | ||
|
||
RNDateTimePicker is the new common name used to represent the old versions of iOS and Android. |
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.
backticks around RNDateTimePicker
Wrt to license reference, I'd prefer a badge over text |
Summary
Readme.md added with documentation included
Compatibility
Checklist
README.md
CHANGELOG.md
example/App.js
)