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] Further template the CSS API descriptions #24360

Merged
merged 10 commits into from
Jan 12, 2021

Conversation

mbrookes
Copy link
Member

Minimise manual translations on Crowdin

@mbrookes mbrookes added the docs Improvements or additions to the documentation label Jan 11, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Jan 11, 2021

Details of bundle changes

Generated by 🚫 dangerJS against befcc67

const TEST = false;
// const TEST = 'Accordion';
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me we can remove this and related code entirely in favor of yarn docs:api --grep ButtonBase which got introduced in #21731.

Or is you workflow not supported by --grep?

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 had missed that it's now supported by yarn docs:api. That works for me! 👍🏼

Copy link
Member

Choose a reason for hiding this comment

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

I'll try to remember to mention script changes in slack (as a simple form of a changelog).

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.

The review of the API generation is a bit more challenging now. I have felt the need to display the HTML output to review these changes.

Comment on lines 27 to 28
/** Class name applied to the root element if `color="primary"`. */
/** Styles applied to the root element if `color="primary"`. */
colorPrimary?: string;
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 "Class name" makes more sense in v5. The component does no longer apply any style with these classes cc @mnajdova.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, we should use "Class name" - another bullet point for me to check if it is updated in all components.

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 had missed that "Class name" was used for unstyled components. I'll revert that commit.

Copy link
Member

Choose a reason for hiding this comment

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

Should it also be used for the styled components? (ahead of time of the whole migration to emotion)

@mbrookes
Copy link
Member Author

The review of the API generation is a bit more challenging now. I feel the need to display the HTML output.

I was conscious of the trade-off. If it proves to be a problem, it should be straightforward to test that the "reconstructed" string matches the source.

@mbrookes
Copy link
Member Author

The review of the API generation is a bit more challenging now.

I've simplified it, so it should be much clearer now.

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.

5 participants