-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
BREAKING CHANGE: deleted `mdc-theme-light-or-dark` and `mdc-theme-dark`
Codecov Report
@@ Coverage Diff @@
## master #2169 +/- ##
=======================================
Coverage 99.14% 99.14%
=======================================
Files 90 90
Lines 3843 3843
Branches 497 497
=======================================
Hits 3810 3810
Misses 33 33 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There is a "Dark Themes" section in docs/theming.md that we should re-word or remove, and make sure there are no longer references to the mixin/classes we're removing
- Remove reminder about documenting dark theme support in readme_standards.md
@@ -47,32 +43,6 @@ fieldset { | |||
border: 0; | |||
} | |||
|
|||
// TODO mgoo: remove these dark theme when complete with removing other components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also remove $dark-button-color which is no longer used after this is removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nits on replacement docs for theming.md
docs/theming.md
Outdated
``` | ||
|
||
Alternatively, you can set your entire page (or a portion of it) to a dark theme by using the `mdc-theme--dark` class: | ||
Html Markup would then look like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Then apply the custom class to some of our buttons, like so:"
docs/theming.md
Outdated
|
||
## Dark Themes | ||
We also have provided a way to customize on a per component basis. We realize that you may want a button with a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be okay to focus on a single example here, and use mdc-button-filled-accessible
which automatically picks ink color based on fill color for best contrast.
Lead off with something like, "For example, let's say you want to make particular buttons stand out by changing their fill color."
docs/theming.md
Outdated
## Dark Themes | ||
We also have provided a way to customize on a per component basis. We realize that you may want a button with a | ||
blue background and another with a red background on the same page. Each component's package is documented with all | ||
our provided sass mixins and what arguments to pass them. For example, here is a link to our [Button Sass Mixins docs](https://github.com/material-components/material-components-web/tree/master/packages/mdc-button#advanced-sass-mixins). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "all our provided" -> "all available"
- "what arguments to pass them" -> "what parameters they accept"
- Use a relative link to the button docs (../packages/mdc-button#advanced-sass-mixins should work) and rephrase to "See the MDC Button Sass mixins for example." (with link around "MDC Button Sass mixins")
docs/theming.md
Outdated
Beyond what we've covered in this document so far, there's also the concept of a _dark theme_. All MDC Web components have | ||
been designed to work with both light themes (that assume a light-colored background) and dark themes (with dark-colored | ||
backgrounds), but the default is always light. | ||
If we continue the example stated above, here is what the sass would like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rephrase to "Here is an example of how we could add styles to change the color of some buttons:"
(For some reason when I read "the example stated above", I was thinking it was referring to further above this section of the document, rather than just a paragraph ago)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments about wording in theming.md
. We usually try to maintain a consistent "voice" across all of our docs 🙂
docs/theming.md
Outdated
|
||
## Dark Themes | ||
For example, let's say you want to make particular |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rephrase this paragraph to something like:
Most MDC Web components provide a set of Sass mixins to customize their appearance,
such as changing the fill color, ink color, stroke width, etc.
These mixins are documented in each component's README file
(e.g., the [Button readme](../packages/mdc-button/README.md#advanced-sass-mixins)).
And line wrap at 120 chars 🙂
docs/theming.md
Outdated
``` | ||
|
||
Alternatively, you can set your entire page (or a portion of it) to a dark theme by using the `mdc-theme--dark` class: | ||
Then apply the custom class to some of our buttons, like so: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Rephrase to "Then apply the custom class to the button elements:"
docs/theming.md
Outdated
<button class="mdc-button mdc-button--raised mdc-button--theme-dark"> | ||
Raised dark button | ||
</button> | ||
```css |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/css/scss/
docs/theming.md
Outdated
@@ -430,37 +430,25 @@ Since our cards only contain text and no components, let's keep it simple for no | |||
> Note: in the future we plan to provide a Javascript utility method for changing all derived colors and making this | |||
use-case easier. | |||
|
|||
## Custom Sass Mixins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Custom Sass Mixins -> Custom Themes
docs/theming.md
Outdated
Beyond what we've covered in this document so far, there's also the concept of a _dark theme_. All MDC Web components have | ||
been designed to work with both light themes (that assume a light-colored background) and dark themes (with dark-colored | ||
backgrounds), but the default is always light. | ||
Here is an example of how we could add styles to change the color of some buttons: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, to change the fill color of a button and automatically select an accessible ink color,
simply call the `mdc-button-filled-accessible` mixin inside a custom CSS class:
@acdvorak ya sounds a lot better than what I had :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Could you give some insight as to why this was removed? Is there an alternative method now to style a component using the dark theme? |
@langan - Dark theme required a lot of maintenance and bloated our scss. We have been investing time in creating sass mixins to customize our components. This gives the developer more flexibility to customizing components. Please refer to https://github.com/material-components/material-components-web/blob/master/docs/theming.md#custom-themes for more. |
BREAKING CHANGE: deleted
mdc-theme-light-or-dark
andmdc-theme-dark