Skip to content

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

Merged

Conversation

DanielSanudo
Copy link
Collaborator

Summary

Readme.md added with documentation included

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

Copy link
Collaborator

@Swaagie Swaagie left a 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

  1. Installing/adding the component to your project
  2. Using the React Component with examples
  3. API/props
  4. Migration examples
  5. 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.
Copy link
Collaborator

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
Copy link
Collaborator

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}

Copy link
Collaborator Author

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
Copy link
Collaborator

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()} />
Copy link
Collaborator

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()} />
Copy link
Collaborator

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()} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Copy link

@3rd-Eden 3rd-Eden left a 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`

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`

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`

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.

Choose a reason for hiding this comment

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

Allows changing of the

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.

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.

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.

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.

Choose a reason for hiding this comment

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

backticks around RNDateTimePicker

@Swaagie
Copy link
Collaborator

Swaagie commented Jun 13, 2019

Wrt to license reference, I'd prefer a badge over text

@DanielSanudo DanielSanudo merged commit 7f98d92 into react-native-datetimepicker:master Jun 13, 2019
@DanielSanudo DanielSanudo deleted the initial_docs branch June 13, 2019 21:22
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.

3 participants