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

[Docs] Updated Card Docs to new format #2590

Merged
merged 1 commit into from
Dec 19, 2015

Conversation

Zadielerick
Copy link
Contributor

Cards documentation update

@alitaheri
Copy link
Member

That was fast 👍 👍 😁

@alitaheri alitaheri changed the title Updated Card Docs to new format [Docs] Updated Card Docs to new format Dec 18, 2015
@alitaheri alitaheri added docs Improvements or additions to the documentation PR: Needs Review labels Dec 18, 2015
@alitaheri alitaheri added this to the Improve the documentation milestone Dec 18, 2015
showExpandableButton: React.PropTypes.bool,

/**
* Override the inline-styles of the root element.
*/
* Override the inline-styles of the card's root element.
Copy link
Member

Choose a reason for hiding this comment

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

Leave this be, @oliviertassinari Will kill you 😆 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it when I had to rebase to master... Didn't realize it had already been changed, thanks!

@alitaheri
Copy link
Member

P.S. try running npm run lint in the root of repository before pushing. It's painful to wait for travis-ci to finish the long build 😄

@Zadielerick
Copy link
Contributor Author

👍 Thanks, didn't know I could do that :)

@alitaheri
Copy link
Member

Yeah, so many things have changed and improved recently. eslint is one of the best ones. thanks to @oliviertassinari, it was hard enforcing those rules >.>

@Zadielerick
Copy link
Contributor Author

Cool, I remember having to wait for Travis to finish all the time :) Great stuff

@Zadielerick
Copy link
Contributor Author

oops forgot some things, ima fix them

@@ -13,16 +13,42 @@ const Card = React.createClass({
},

propTypes: {
/**
* Whether a click on this card component expands the card. Can be set on any child of the Card component.
*/
actAsExpander: React.PropTypes.bool,
Copy link
Member

Choose a reason for hiding this comment

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

also, add default prop for all the bool types, it makes it more expressive in the docs. the code generator can pick them up :D

@@ -13,16 +13,42 @@ const Card = React.createClass({
},

propTypes: {
/**
* Whether a click on this card component expands the card. Can be set on any child of the Card component.
Copy link
Member

Choose a reason for hiding this comment

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

one more thing, sorry 😁

jsdoc better be formatted like this:

/**
 * Foo
 */

instead of

/**
* Foo
*/

that extra indentation.

@alitaheri
Copy link
Member

Sorry I keep bragging 😁 It's your first contribution after a long time apparently. After one PR or 2 these bragging will drop a lot. sorry 😁 😁

@alitaheri
Copy link
Member

😱 You code fast :D :D faster than I can review :P :P

@Zadielerick
Copy link
Contributor Author

Lol, sorry :) Have to get used to these things and learn the new ones. And no worries, you are being a great help!
So for the default props, the parser looks to getDefaultProps, but that method doesn't exist in Card. Should I add it?

@alitaheri
Copy link
Member

No problem, glad you don't get mad at me 😅 😅

So for the default props, the parser looks to getDefaultProps

Yes, that's true. and yup add it please. thanks 👍 👍

@Zadielerick
Copy link
Contributor Author

Sorry for the silly question, but if a bool prop is not defined, it is considered false, right? So in the getDefaultProps() I would just set it to false?

@alitaheri
Copy link
Member

@Zadielerick Yeah, that's right 😁 set it to false. doing this will tip the parser to generate documentation explicitly expressing that it will default to false. That's only good practice, no effect on the logic.

@Zadielerick
Copy link
Contributor Author

@subjectix Thanks alot! Almost done :)

import FlatButton from 'material-ui/lib/flat-button';
import CardText from 'material-ui/lib/card/card-text';

const CardExampleWithAvatar = React.createClass({
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the stateless function component pattern for those examples?

getDefaultProps() {
return {
expandable: false,
initiallyExpanded: true,
Copy link
Member

Choose a reason for hiding this comment

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

this defaults to false.

@alitaheri
Copy link
Member

@oliviertassinari I'm going to merge this and fix the issues myself, because I need these changes in my PR.

alitaheri added a commit that referenced this pull request Dec 19, 2015
[Docs] Updated Card Docs to new format
@alitaheri alitaheri merged commit edbc3c9 into mui:master Dec 19, 2015
@oliviertassinari
Copy link
Member

@subjectix Sure 👍. I'm working on moving the files for #2592

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