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

[DatePicker] add function signatures to docs #3652

Merged
merged 1 commit into from
Mar 10, 2016
Merged

[DatePicker] add function signatures to docs #3652

merged 1 commit into from
Mar 10, 2016

Conversation

theosherry
Copy link
Contributor

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Hey, a PR towards #3096 .

I left the DateTimeFormat entry as it is because a signature for that seems beyond the scope of the docs.

I wasn't sure how to handle two situations:

  • functions, like onDismiss, that take no arguments and aren't expected to return anything: how should their signature be represented? I just left them out.
  • null arguments, like with onChange: how should they be represented? They have undefined type and require a name (null in this PR) with the current setup.

Anyway, let me know what you think.

* will always be null and the second argument will be the new Date instance.
* Callback function that is fired when the date value changes.
*
* @param {null} null Since there is no particular event associated with the change, the first argument will always
Copy link
Member

Choose a reason for hiding this comment

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

break this line in 2 please 😁

@alitaheri
Copy link
Member

Thanks a lot, this was really helpful 👍 👍 😍

@alitaheri alitaheri self-assigned this Mar 10, 2016
@alitaheri alitaheri added docs Improvements or additions to the documentation PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Mar 10, 2016
@theosherry
Copy link
Contributor Author

Thanks @alitaheri :)

@alitaheri
Copy link
Member

Looking good 👍 👍 Thanks 😁

@callemall/material-ui Good to go 🍏

@alitaheri alitaheri added PR: Review Accepted and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Mar 10, 2016
oliviertassinari added a commit that referenced this pull request Mar 10, 2016
[DatePicker] add function signatures to docs
@oliviertassinari oliviertassinari merged commit f433925 into mui:master Mar 10, 2016
@oliviertassinari
Copy link
Member

@theosherry Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants