Skip to content

(docs) update theming.md #1385

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 16 commits into from
Oct 18, 2016
Merged

(docs) update theming.md #1385

merged 16 commits into from
Oct 18, 2016

Conversation

vinagreti
Copy link
Contributor

@jelbourn, I've used /src/lib/input/_input-theme.scss as base to updat the custom component.

@jelbourn, I've used /src/lib/input/_input-theme.scss as base to updat the custom component.
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 30, 2016
@@ -109,15 +109,91 @@ dark theme.
In order to style your own components with our tooling, the component's styles must be defined
with Sass.

You can consume the theming functions and variables from the `@angular/material/core/theming`.
Copy link
Member

Choose a reason for hiding this comment

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

Rather than making this document much longer, I'd rather create another file theming-your-components.md that covers how to theme your own components with the material tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelbourn Ok, I'll do this! But should I use your tiny as possible example or can we provide the example with all that information to theming custom form components?

Copy link
Member

Choose a reason for hiding this comment

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

I still want to use as small of an example as possible that illustrates the point. Right now, there's just way too much here that distracts from the theming bits.

color: md-color($primary, default-contrast);
@import '~@angular/material/core/theming/theming';

@mixin custom-input-theme($theme) {
Copy link
Member

Choose a reason for hiding this comment

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

It's not really necessary to include this much css. We can communicate the idea with much less code, e.g.:

// Define a mixin that accepts a theme and outputs the color styles for the component.
@mixin candy-carousel-theme($theme) {
  // Extract whichever individual palettes you need from the theme.
  $primary: map-get($theme, primary);
  $accent: map-get($theme, accent);

  // Use md-color to extract individual colors from a palette as necessary.
  .candy-carousel {
    background-color: md-color($primary);
    border-color: md-color($accent, A400);
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelbourn I've placed this much of css because I was trying to help people creating their own form elements that use all input styles, from placeholder-color to underline-focused-color. Maybe I can point the _input-theme.scss as a base to customize inputs and show only this tiny format that you suggested. What do you think?
Or you are saying to me that we can use this tiny format in theming.md and in the new file called theming-your-components.md we place that much of code that I did in this PR?

Bruno da Silva joão added 3 commits October 3, 2016 14:26
As @jelbourn pointed, "Rather than making this document much longer, I'd rather create another file theming-your-components.md that covers how to theme your own components with the material tools."
As @jelbourn pointed out, We can communicate the idea of theming custo components with much less code. I added just enougth code to show how to customize custom component and I added a new file with a more deep answer.
More explanations about files and functions used.
@vinagreti
Copy link
Contributor Author

@jelbourn I've made the changes. Please, review it again!

Bruno da Silva joão added 6 commits October 5, 2016 10:39
Removed unnecessary code.
Removed unnecessary code.
Created two examples to showing customizations with and without @mixin.
No more custom theme used. Using only pre-built one.
Removed unnecessary text.
Removed unnecessary imports.
Shortest example.
@vinagreti
Copy link
Contributor Author

@jelbourn I've made the changes and I think that it is the shortest form. Please, review it again!

Removed unnecessary text

In order to style your own components with our tooling, the component's styles must be defined with Sass.

You can consume the theming functions from the `@angular/material/core/theming/all-theme` and theming variables from a pre-built theme or a custom one. You can use the `md-color` function to extract a specific color from a palette.
Copy link
Member

Choose a reason for hiding this comment

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

Wrap lines at 100 columns.


In order to style your own components with our tooling, the component's styles must be defined with Sass.

You can consume the theming functions from the `@angular/material/core/theming/all-theme` and theming variables from a pre-built theme or a custom one. You can use the `md-color` function to extract a specific color from a palette.
Copy link
Member

@jelbourn jelbourn Oct 5, 2016

Choose a reason for hiding this comment

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

The imports for theming should be
@angular/material/core/theming/theming for functions and
@angular/material/core/theming/palette for Material palette vars

I'd like to also link to the _theming.scss source file with something like

For more details about the theming functions, see the comments in the
[source](https://github.com/angular/material2/blob/master/src/lib/core/theming/_theming.scss).

// Import theming functions and variables
@import '~@angular/material/core/theming/all-theme';
// Import a pre-built theme
@import '~@angular/material/core/theming/prebuilt/deep-purple-amber';
Copy link
Member

@jelbourn jelbourn Oct 5, 2016

Choose a reason for hiding this comment

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

You shouldn't ever import a pre-built theme; those are meant to be leaf nodes in the sass build. Instead you could mention importing the theme variables from your own app's theme definition with something like $candy-app-primary, etc.

}
```

## Using @mixin to automatically apply a theme
Copy link
Member

@jelbourn jelbourn Oct 5, 2016

Choose a reason for hiding this comment

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

@mixin
(here and elsewhere)

.candy-carousel {
background-color: md-color($primary);
border-color: md-color($accent, A400);
}
Copy link
Member

Choose a reason for hiding this comment

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

I would move the mixin approach above this example. I want to encourage people to use the mixin approach in order to make it easier to switch between themes in their app.

}
```

Now you just have have to call the @mixin function to apply the theme:
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to explain somewhere in this doc the advantage of using a mixin (calling it with a different theme argument to allow multiple themes within the app).

We should also mention that, when doing this, the theme file should only contain the definitions that are affected by the passed-in theme.

// Import your custom input theme file so you can call the custom-input-theme function
@import 'app/candy-carousel/candy-carousel-theme.scss';

//Using the $theme variable from the pre-built theme you can call the theming function
Copy link
Member

Choose a reason for hiding this comment

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

Missing space between // and Using

@@ -109,15 +109,25 @@ dark theme.
In order to style your own components with our tooling, the component's styles must be defined
with Sass.

You can consume the theming functions and variables from the `@angular/material/core/theming`.
You can use the `md-color` function to extract a specific color from a palette. For example:
You can consume the theming functions and variables from the @angular/material/core/theming. You can use the `md-color` function to extract a specific color from a palette. For example:
Copy link
Member

Choose a reason for hiding this comment

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

I would just remove all of the content in this section now and point to the new doc.

@@ -0,0 +1,56 @@
#Theming your custom components

In order to style your own components with our tooling, the component's styles must be defined with Sass.
Copy link
Member

Choose a reason for hiding this comment

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

Let's change "our tooling" to "Angular Material's tooling"

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Oct 14, 2016
@googlebot
Copy link

CLAs look good, thanks!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Oct 14, 2016
@vinagreti
Copy link
Contributor Author

@jelbourn I've made the changes and cleaned up. Please, review it again!

Bruno da Silva joão added 3 commits October 14, 2016 10:21
Improved @mixin file name to match the candy-carousel example.
Improved `@mixin` paragraph.
@jelbourn
Copy link
Member

LGTM, thanks!

@jelbourn jelbourn merged commit 3c1e8d2 into angular:master Oct 18, 2016
@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 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants