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

[IconButton] add callback signatures to docs #3709

Merged
merged 1 commit into from
Mar 17, 2016
Merged

[IconButton] add callback signatures to docs #3709

merged 1 commit into from
Mar 17, 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, PR for #3096 .

*/
onBlur: React.PropTypes.func,

/**
* Callback function for when the component gains focus.
* Callback function fired when the element gains focus.
* @param {object} event `focus` event targeting the lement.
Copy link
Member

Choose a reason for hiding this comment

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

element

@theosherry
Copy link
Contributor Author

Oops sry. Fixed the typo.

@mbrookes
Copy link
Member

Thanks for continuing through these, it's really appreciated!

Would you be happy to standardise the wording on the other prop descriptions while you're there? If you do you can squash onto this commit.

@mbrookes mbrookes added the docs Improvements or additions to the documentation label Mar 16, 2016
@theosherry
Copy link
Contributor Author

@mbrookes No problem. I'll give it a go when I get home tonight.

@mbrookes mbrookes added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Mar 16, 2016
@mbrookes
Copy link
Member

Thanks! 👍

@theosherry
Copy link
Contributor Author

Ok, let me know what you think. I tried to be consistent with what I've done so far.

@mbrookes
Copy link
Member

Thanks @theosherry - I should have mentioned it (but I assumed you were already working from it as your callback changes were in line): https://github.com/callemall/material-ui/wiki/Documentation#description-wording

Could you follow the 'If true, ... ' pattern for the bools? And perhaps double check the others?

Doesn't have to be word for word, as there are inconsistencies everywhere due to how Material-UI has evolved, but the more cleanups the better!

@theosherry
Copy link
Contributor Author

Of course. Sorry, I forgot about those guidelines.

@mbrookes mbrookes 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 17, 2016
@mbrookes
Copy link
Member

Thanks, looks good! 👍

@alitaheri
Copy link
Member

Thanks a lot @theosherry 👍

alitaheri added a commit that referenced this pull request Mar 17, 2016
[IconButton] add callback signatures to docs
@alitaheri alitaheri merged commit e6952c8 into mui:master Mar 17, 2016
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.

4 participants