feat(theme): Add light/dark variants for primary/secondary color, and rename `accent` to `secondary` #1116

acdvorak · 2017-08-11T00:35:04Z

In the spec, "accent" was renamed to "secondary".
This PR only changes "accent" to "secondary" for variables defined within the mdc-theme package. Modifier classes for individual components (e.g., mdc-button--accent) remain unchanged for now.
For backward compatibility, $mdc-theme-accent still exists, and $mdc-theme-secondary points to it. This ensures that clients that were previously overriding $mdc-theme-accent will continue to work.
Light/dark tonal variants are not yet being used, but will become used soon when we update our components to align with an upcoming version of the spec.

BREAKING CHANGE

…d secondary colors

…nts for all CSS classes/vars Modifier classes for individual components (e.g., `mdc-button--accent`) are **NOT** renamed. We are currently considering our options for renaming those at a later date.

codecov-io · 2017-08-11T01:30:51Z

Codecov Report

Merging #1116 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1116   +/-   ##
======================================
  Coverage    99.9%   99.9%           
======================================
  Files          69      69           
  Lines        3302    3302           
  Branches      405     405           
======================================
  Hits         3299    3299           
  Misses          3       3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d9023b...e7d5cd4. Read the comment docs.

# Conflicts: # packages/mdc-theme/_functions.scss # packages/mdc-theme/_variables.scss

lynnmercier · 2017-08-11T20:00:08Z

packages/mdc-button/mdc-button.scss

-  @each $theme-style in (primary, accent) {
-    &.mdc-button--#{$theme-style} {
+  @each $theme-style in (primary, secondary) {
+    $modifier: if($theme-style == "secondary", "accent", $theme-style);


Add a comment about why this is around...basically about backwards compatibility

lynnmercier · 2017-08-11T20:00:13Z

packages/mdc-button/mdc-button.scss

-    @each $theme-style in (primary, accent) {
-      &.mdc-button--#{$theme-style} {
+    @each $theme-style in (primary, secondary) {
+      $modifier: if($theme-style == "secondary", "accent", $theme-style);


lynnmercier · 2017-08-11T20:01:19Z

packages/mdc-linear-progress/README.md

@@ -59,7 +59,7 @@ The provided modifiers are:
 | --------------------- | ------------------------------------------------------- |
 | `mdc-linear-progress--indeterminate`   | Puts the linear progress indicator in an indeterminate state. |
 | `mdc-linear-progress--reversed`  | Reverses the direction of the linear progress indicator.   |
-| `mdc-linear-progress--accent` | Colors the button with the accent color. |
+| `mdc-linear-progress--accent` | Colors the button with the secondary color. |


colors the "button"? Is this correct? Or some old copy/paste bug?

Done - looks like an old copy/paste bug 🙂

lynnmercier · 2017-08-11T20:03:14Z

packages/mdc-theme/_functions.scss

@@ -16,6 +16,11 @@

 @import "./constants";

+// Used by the functions below to shift a color's lightness by approximately


I dont think you need these new functions in this PR, can you move it to another one?

Will be re-added in a separate PR

acdvorak · 2017-08-11T22:03:06Z

Addressed comments, PTAL!

lynnmercier

Are there other components, more than just MDC Button, which need updating? Some might still be using "accent"

lynnmercier

LGTM

kfranqueiro · 2017-08-14T22:01:57Z

packages/mdc-theme/_variables.scss

 $mdc-theme-accent: #ff4081 !default; // Pink A200
+$mdc-theme-secondary: $mdc-theme-accent !default;


Should this be the other way around? i.e. assign the actual value to secondary, then assign secondary to accent?

That would break backward compatibility for existing clients who are already overriding accent, because we would no longer be reading the value that they're setting.

Oh yeah, that's true. Since the comment above already explains that accent only exists now for backcompat, this should be fine.

kfranqueiro · 2017-08-14T22:02:15Z

packages/mdc-theme/_variables.scss

+$mdc-theme-primary-light: lighten($mdc-theme-primary, 14%) !default;
+$mdc-theme-primary-dark: darken($mdc-theme-primary, 14%) !default;
+
+// The $mdc-theme-secondary variable is DEPRECATED - it exists purely for backward compatibility.


Overzealous find/replace? The accent variable is what's deprecated, right?

Why do you always insist on catching my mistakes, Ken? 😛

kfranqueiro · 2017-08-14T22:06:27Z

docs/getting-started.md

@@ -216,7 +216,7 @@ hit the button, and get a pleasant greeting :wave:
 You may have noticed that the button background, as well as the label and underline on focused text
 input fields, defaults to the Indigo 500 (`#673AB7`) color from the [Material Design color palette](https://material.io/guidelines/style/color.html#color-color-palette).
 This is part of the default theme that ships with MDC-Web; it uses Indigo 500 for a primary color, and
-Pink A200 (`#FF4081`) for an accent color. Let's change the theme's primary color.
+Pink A200 (`#FF4081`) for an secondary color. Let's change the theme's primary color.


an secondary -> a secondary

kfranqueiro · 2017-08-14T22:13:20Z

packages/mdc-theme/_variables.scss

+  text-hint-on-secondary-dark: map-get(map-get($mdc-theme-text-colors, $mdc-theme-secondary-dark-tone), hint),
+  text-disabled-on-secondary-dark: map-get(map-get($mdc-theme-text-colors, $mdc-theme-secondary-dark-tone), disabled),
+  text-icon-on-secondary-dark: map-get(map-get($mdc-theme-text-colors, $mdc-theme-secondary-dark-tone), icon),
+  // Text-primary on background background


This looks kind of funny... maybe for each of these comments surround each background name in quotes or backticks? e.g. "background" background or `background` background

Done. Good idea!

… instead of `accent` (#1156) The build was accidentally broken in PR #1074 due to a merge conflict with PR #1116.

- 💥 Change "accent" to "secondary" globally, as done by MDC-Web. material-components/material-components-web#1116 - ✨ `{{mdc-button}}` now has `unelevated` boolean attribute (default: `false`) - ✨ `{{mdc-button}}` now has `dense` boolean attribute (default: `false`) - ✨ `{{mdc-button}}` now has `compact` boolean attribute (default: `false`) - ✨ `{{mdc-textfield}}` now has `box` boolean attribute (default: `false`)

@Material

- ⬆️ Upgrade @Material packages to 0.19 versions - 💥 Change "accent" to "secondary" globally, as done by MDC-Web. material-components/material-components-web#1116 - ✨ `{{mdc-button}}` now has `unelevated` boolean attribute (default: `false`) - ✨ `{{mdc-button}}` now has `dense` boolean attribute (default: `false`) - ✨ `{{mdc-button}}` now has `compact` boolean attribute (default: `false`) - ✨ `{{mdc-textfield}}` now has `box` boolean attribute (default: `false`) - 🐛 `{{mdc-checkbox}}`, `{{mdc-radio}}`, and `{{mdc-switch}}` now respect checked attr (#29) - 💚 Switch CI to use Headless Chrome (#28) - ✨ Event bubbling can be disabled for checkboxes, radio buttons, and switches (#27)

acdvorak added 2 commits August 10, 2017 14:24

feat(theme): Add SCSS variables for dark/light variants of primary an…

d78060f

…d secondary colors

feat(theme): Rename accent to secondary, and add light/dark varia…

a72b100

…nts for all CSS classes/vars Modifier classes for individual components (e.g., `mdc-button--accent`) are **NOT** renamed. We are currently considering our options for renaming those at a later date.

acdvorak added this to the Theme Improvements milestone Aug 11, 2017

acdvorak assigned lynnmercier Aug 11, 2017

acdvorak added 2 commits August 10, 2017 17:42

chore(theme): Use // comments instead of /** for new functions

3ceb631

chore(theme): Use // comments instead of /** for new variable

819de3e

Merge branch 'master' into feat/m2-theme/step02-light-dark-variants

7dccbd4

acdvorak mentioned this pull request Aug 11, 2017

Change theme's SCSS comment style from /** to // #1112

Merged

acdvorak and others added 2 commits August 11, 2017 12:53

Merge branch 'master' into feat/m2-theme/step02-light-dark-variants

601c426

# Conflicts: # packages/mdc-theme/_functions.scss # packages/mdc-theme/_variables.scss

Merge branch 'master' into feat/m2-theme/step02-light-dark-variants

3e3b81e

lynnmercier suggested changes Aug 11, 2017

View reviewed changes

acdvorak added 3 commits August 11, 2017 14:58

chore(theme): Add comments about backward compatibility

e498fba

chore(linear-progress): Fix old typo in README.md

91afa2a

feat(theme): Remove mdc-theme-{light,dark}-variant functions

14810b6

Will be re-added in a separate PR

lynnmercier suggested changes Aug 14, 2017

View reviewed changes

acdvorak and others added 5 commits August 14, 2017 09:32

Merge branch 'master' into feat/m2-theme/step02-light-dark-variants

8ba93b3

chore(demos): Reword "accent" to "secondary"

f0693e8

docs(button): Reword "accent" to "secondary" in README

fe40de0

chore(demos): Rename "accent" vars/IDs to "secondary"

4d72b7f

chore(framework-examples): Rename "accent" to "secondary"

e7e8b2f

lynnmercier approved these changes Aug 14, 2017

View reviewed changes

kfranqueiro suggested changes Aug 14, 2017

View reviewed changes

acdvorak added 3 commits August 15, 2017 07:25

chore(theme): Fix typo in SCSS comment

4684d58

chore(theme): Clarify SCSS comments for text on background colors

18e984e

docs: Fix grammatical typo from search/replace

e7d5cd4

kfranqueiro approved these changes Aug 15, 2017

View reviewed changes

acdvorak merged commit 2314ad5 into master Aug 15, 2017

acdvorak deleted the feat/m2-theme/step02-light-dark-variants branch August 16, 2017 07:49

acdvorak added a commit that referenced this pull request Aug 17, 2017

fix(button): Un-break the build by referencing secondary theme prop…

d3ff8fc

… instead of `accent` (#1156) The build was accidentally broken in PR #1074 due to a merge conflict with PR #1116.

Kerrick mentioned this pull request Sep 5, 2017

⬆️ Upgrade @material packages to 0.19 versions secondstreet/ember-material-components-web#30

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(theme): Add light/dark variants for primary/secondary color, and rename `accent` to `secondary` #1116

feat(theme): Add light/dark variants for primary/secondary color, and rename `accent` to `secondary` #1116

acdvorak commented Aug 11, 2017

codecov-io commented Aug 11, 2017 •

edited

Loading

lynnmercier Aug 11, 2017

acdvorak Aug 11, 2017

lynnmercier Aug 11, 2017

acdvorak Aug 11, 2017

lynnmercier Aug 11, 2017

acdvorak Aug 11, 2017

lynnmercier Aug 11, 2017

acdvorak Aug 11, 2017

acdvorak commented Aug 11, 2017

lynnmercier left a comment

lynnmercier left a comment

kfranqueiro Aug 14, 2017

acdvorak Aug 15, 2017

kfranqueiro Aug 15, 2017

kfranqueiro Aug 14, 2017

acdvorak Aug 15, 2017 •

edited

Loading

kfranqueiro Aug 14, 2017

acdvorak Aug 15, 2017

kfranqueiro Aug 14, 2017

acdvorak Aug 15, 2017

		@@ -16,6 +16,11 @@

		@import "./constants";

		// Used by the functions below to shift a color's lightness by approximately

		$mdc-theme-accent: #ff4081 !default; // Pink A200
		$mdc-theme-secondary: $mdc-theme-accent !default;

feat(theme): Add light/dark variants for primary/secondary color, and rename accent to secondary #1116

feat(theme): Add light/dark variants for primary/secondary color, and rename accent to secondary #1116

Conversation

acdvorak commented Aug 11, 2017

codecov-io commented Aug 11, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acdvorak commented Aug 11, 2017

lynnmercier left a comment

Choose a reason for hiding this comment

lynnmercier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acdvorak Aug 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat(theme): Add light/dark variants for primary/secondary color, and rename `accent` to `secondary` #1116

feat(theme): Add light/dark variants for primary/secondary color, and rename `accent` to `secondary` #1116

codecov-io commented Aug 11, 2017 •

edited

Loading

acdvorak Aug 15, 2017 •

edited

Loading