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] Grid docs should refer to Hidden component/demo #6963

Merged
merged 3 commits into from May 26, 2017
Merged

[docs] Grid docs should refer to Hidden component/demo #6963

merged 3 commits into from May 26, 2017

Conversation

kgregory
Copy link
Member

@kgregory kgregory commented May 26, 2017

The Grid component API docs list a type of HiddenProps for the hidden prop, but this should really be a link to the Layout demo. I've modified markdown generation to look for this type specifically and return a link.

Added the ability to link to components from proptype descriptions. By including [Hidden component] in the description of Grid's hidden property, a link to the component API is generated when docs are built. Grid's hidden property includes a markdown link to the Hidden component's api docs

I also made the following changes:

  • Omit classes section for components that do not export a stylesheet (e.g. TextField)
  • Strip carriage returns introduced from doc generation on windows (generating docs from Windows caused the markdown madness referred to in the comments)

- Allow proptype descriptions to covert to component API links
- Omit classes section for components that do not export a stylesheet
- Strip carriage returns introduced from doc generation on windows
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Good idea!

@@ -188,7 +194,7 @@ section for more detail.

If using the \`overrides\` key of the theme as documented
[here](/customization/themes#customizing-all-instances-of-a-component-type),
you need to use the following style sheet name: \`${styles.name}\`.`;
you need to use the following style sheet name: \`${styles.name}\`.` : '';
Copy link
Member

Choose a reason for hiding this comment

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

I think that styles.name should always be defined. No need to be defensive.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just the other side of the ternary that checks for styles.classes.length

@@ -177,7 +183,7 @@ function generateProps(props) {
}

function generateClasses(styles) {
return `## Classes
return styles.classes.length ? `## Classes
Copy link
Member

Choose a reason for hiding this comment

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

Oops 👍

src/Grid/Grid.js Outdated
@@ -190,7 +190,7 @@ type Props = {
*/
gutter?: 0 | 8 | 16 | 24 | 40,
/**
* If provided, will wrap with Hidden component and given properties.
* If provided, will wrap with [Hidden component] and given properties.
Copy link
Member

Choose a reason for hiding this comment

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

What about expliciting the Path? I'm not sure we need to be that smart.

/**
 * If provided, will wrap with [Hidden](/component-api/Hidden) component and given properties.
 */

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that and it was my original approach, but wasn't sure if it would add too much noise to the comments. I will go with this! 👍

@kgregory
Copy link
Member Author

Updated this PR, eliminating the functionality that would generate component links. Following recommendation from @oliviertassinari , links can be directly expressed in propType comments (e.g. Grid's hidden prop)

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation next labels May 26, 2017
@oliviertassinari oliviertassinari merged commit 6f46e4d into mui:next May 26, 2017
@oliviertassinari
Copy link
Member

@kgregory Thanks

@kgregory kgregory deleted the next branch June 28, 2017 13:49
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.

None yet

2 participants