Skip to content

docs: clean up and clarify theming docs #13435

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

Merged
merged 1 commit into from
Oct 5, 2018

Conversation

jelbourn
Copy link
Member

@jelbourn jelbourn commented Oct 4, 2018

This makes some minor wording changes to the main theming guide and mostly rewrites the theming-your-own-component guide. Edited for clarify, brevity, and language.

Fixes #13206

@jelbourn jelbourn added docs This issue is related to documentation pr: merge safe target: patch This PR is targeted for the next patch release labels Oct 4, 2018
@jelbourn jelbourn requested a review from josephperrott October 4, 2018 22:15
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 4, 2018
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM overall

@@ -1,80 +1,81 @@
# Theming your custom components
In order to style your own components with Angular Material's tooling, the component's styles must be defined with Sass.
### Theming your own component the Angular Material way
Copy link
Member

Choose a reason for hiding this comment

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

Should this be something like

Theming your component using Angular Material's theming system

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

We can better theme our custom components by adding a `@mixin` function to its theme file, and then call this function to apply a theme.

All you need is to create a `@mixin` function in the custom-component-theme.scss
#### 1. Define all color and typography styles in a "theme mixin" for the component
Copy link
Member

Choose a reason for hiding this comment

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

This section should include examples for both theme and typography, currently we mention typography but don't show it in the examples.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

This makes some minor wording changes to the main theming guide and
mostly rewrites the theming-your-own-component guide. Edited for
clarify, brevity, and language.

Fixes angular#13206
@jelbourn jelbourn force-pushed the theming-docs-tweaks branch from 49ba6a3 to 0d37cf8 Compare October 5, 2018 18:14
@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Oct 5, 2018
@vivian-hu-zz vivian-hu-zz merged commit 4fc5ee1 into angular:master Oct 5, 2018
roboshoes pushed a commit to roboshoes/material2 that referenced this pull request Oct 23, 2018
This makes some minor wording changes to the main theming guide and
mostly rewrites the theming-your-own-component guide. Edited for
clarify, brevity, and language.

Fixes angular#13206
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement docs This issue is related to documentation target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theming documents confusion or possible conflict
4 participants