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

feat(select): Move colors for default select to mixins #1934

Merged
merged 13 commits into from
Jan 17, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions demos/select.html
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ <h2 class="mdc-typography--title">Fully-Featured Component</h2>
<input type="checkbox" id="rtl">
<label for="rtl">RTL</label>
</div>
<div>
<input type="checkbox" id="alternate-colors">
<label for="alternate-colors">Alternate Colors</label>
</div>
<div>
<input type="checkbox" id="disabled">
<label for="disabled">Disabled</label>
Expand Down Expand Up @@ -197,6 +201,7 @@ <h2>Select Multiple - CSS Only</h2>
var boxDemoWrapper = document.getElementById('box-demo-wrapper');
var darkThemeCb = document.getElementById('dark-theme');
var rtlCb = document.getElementById('rtl');
var alternateColorsCb = document.getElementById('alternate-colors');
var disabledCb = document.getElementById('disabled');

darkThemeCb.addEventListener('change', function() {
Expand All @@ -209,6 +214,9 @@ <h2>Select Multiple - CSS Only</h2>
boxDemoWrapper.removeAttribute('dir');
}
});
alternateColorsCb.addEventListener('change', function() {
boxDemoWrapper.classList[alternateColorsCb.checked ? 'add' : 'remove']('demo-select-custom-colors');
});
disabledCb.addEventListener('change', function() {
select.disabled = disabledCb.checked;
});
Expand Down
14 changes: 14 additions & 0 deletions demos/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,17 @@
@import "../packages/mdc-list/mdc-list";
@import "../packages/mdc-menu/mdc-menu";
@import "../packages/mdc-select/mdc-select";

// stylelint-disable selector-class-pattern
.demo-select-custom-colors {
@include mdc-select-fill-color(rgba(blue, .1));
@include mdc-select-ink-color(blue);
@include mdc-select-label-color(rgba(blue, .6));
@include mdc-select-bottom-line-color(rgba(blue, .5));
@include mdc-select-selected-text-color(rgba(blue, .8));

// Focused colors
@include mdc-select-focused-bottom-line-color(green);
@include mdc-select-focused-label-color(green);
}
// stylelint-enable selector-class-pattern
19 changes: 18 additions & 1 deletion packages/mdc-select/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ If you'd like to maintain the width or height outside of the attribute, you'll n
}
```

#### Classes
#### CSS Classes

| Class | Description |
| ------------------------ | ----------------------------------------------- |
Expand All @@ -269,6 +269,23 @@ If you'd like to maintain the width or height outside of the attribute, you'll n

It is advised that dividers also set `role="presentation"` to disable selection and not cloud accessibility.

### Sass Mixins

To customize the colors of any part of the select, use the following mixins. We recommend you use
these mixins within CSS selectors like `.foo-select` to apply styling.

Mixin | Description
--- | ---
`mdc-select-ink-color($color)` | Customizes the text entered into the select.*
`mdc-select-fill-color($color)` | Customizes the background color of the select.
Copy link
Contributor

Choose a reason for hiding this comment

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

container-fill-color, not just fill-color. Its a little more specific, since just "fill" is vague

`mdc-select-label-color($color)` | Customizes the label color of the select in the unfocused state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add that this only covers the JS only version....otherwise label-color is covered by ink-color for CSS only

`mdc-select-focused-label-color($color, $opacity: 0.87)` | Customizes the label color of the select when focused. Changing opacity for the label when floating is optional.
`mdc-select-bottom-line-color($color)` | Customizes the color of the default bottom line of the select.
`mdc-select-focused-bottom-line-color($color)` | Customizes the color of the bottom line of the select when focused.
`mdc-select-selected-text-color($color)` | Customizes the color of the option that has been selected.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you attach a screenshot of what you are coloring here? I cant figure out what this is.

Copy link
Contributor Author

@williamernest williamernest Jan 11, 2018

Choose a reason for hiding this comment

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

screen shot 2018-01-11 at 3 40 38 pm

The "Fruit Roll Ups" text is the mdc-select-selected-text-color

Maybe mdc-select-selected-item-ink-color? The css class is .mcd-select__selected-text so I left it similar for the mixin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Try and combine this with mdc-select-ink-color. And mdc-select-ink-color is the better name.


>_NOTE_: * - This style applies to the css only version.
Copy link
Contributor

Choose a reason for hiding this comment

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

this note seems unfinished?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It references the asterisk on line 279, the mdc-select-ink-color is only applied on the css only version. I'll put it in the description column.


### MDC Select Component API

The MDC Select component API is modeled after a subset of the `HTMLSelectElement` functionality, and
Expand Down
119 changes: 119 additions & 0 deletions packages/mdc-select/_mixins.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
//
// Copyright 2018 Google Inc. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

// Public

@mixin mdc-select-ink-color($color) {
:not(.mdc-select--disabled) {
@include mdc-select-ink-color_($color);
}
}

@mixin mdc-select-fill-color($color) {
:not(.mdc-select--disabled) {
@include mdc-select-fill-color_($color);
}
}

@mixin mdc-select-label-color($color) {
&:not(.mdc-select--disabled) {
@include mdc-select-label-color_($color);
}
}

@mixin mdc-select-focused-label-color($color, $opacity: .87) {
&:not(.mdc-select--disabled) {
@include mdc-select-focused-label-color_($color, $opacity);
}
}

@mixin mdc-select-bottom-line-color($color) {
&:not(.mdc-select--disabled) {
@include mdc-select-bottom-line-color_($color);
}
}

@mixin mdc-select-focused-bottom-line-color($color) {
&:not(.mdc-select--disabled) {
@include mdc-select-focused-bottom-line-color_($color);
}
}

@mixin mdc-select-selected-text-color($color) {
&:not(.mdc-select--disabled) {
@include mdc-select-selected-text-color_($color);
}
}

// Private

@mixin mdc-select-ink-color_($color) {
.mdc-select__surface {
@include mdc-theme-prop(color, $color);
}
}

@mixin mdc-select-fill-color_($color) {
.mdc-select__surface {
@include mdc-theme-prop(background-color, $color);
}
}

@mixin mdc-select-label-color_($color) {
.mdc-select__label {
@include mdc-theme-prop(color, $color);
}
}

@mixin mdc-select-focused-label-color_($color, $opacity: .87) {
.mdc-select__surface:focus .mdc-select__label {
@include mdc-theme-prop(color, $color);
}

// Separate parameter is used for opacity, because opacity is only applied when the
// label is floating, but the label is the same color when the select is focused
// but an option has not been selected.
.mdc-select__surface:focus .mdc-select__label--float-above {
opacity: $opacity;
}
}

@mixin mdc-select-bottom-line-color_($color) {
.mdc-select__bottom-line {
@include mdc-theme-prop(background-color, $color);
}
}

@mixin mdc-select-focused-bottom-line-color_($color) {
.mdc-select__surface:focus .mdc-select__bottom-line,
.mdc-select__surface:focus ~ .mdc-select__bottom-line {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need both these selectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JS version has bottom line as a child element. CSS version is a sibling element.

@include mdc-theme-prop(background-color, $color);
}

.mdc-select--open .mdc-select__bottom-line {
@include mdc-theme-prop(background-color, $color);
}
}

@mixin mdc-select-selected-text-color_($color) {
.mdc-select__selected-text {
@include mdc-theme-prop(color, $color);
}
}

@mixin mdc-select-dd-arrow-svg-bg_($fill-hex-number: 000000, $opacity: .54) {
background-image: url(data:image/svg+xml,%3Csvg%20width%3D%2210px%22%20height%3D%225px%22%20viewBox%3D%227%2010%2010%205%22%20version%3D%221.1%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%3E%0A%20%20%20%20%3Cpolygon%20id%3D%22Shape%22%20stroke%3D%22none%22%20fill%3D%22%23#{$fill-hex-number}%22%20fill-rule%3D%22evenodd%22%20opacity%3D%22#{$opacity}%22%20points%3D%227%2010%2012%2015%2017%2010%22%3E%3C%2Fpolygon%3E%0A%3C%2Fsvg%3E);
}
46 changes: 15 additions & 31 deletions packages/mdc-select/mdc-select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
//

@import "./mixins";
@import "@material/animation/functions";
@import "@material/animation/variables";
@import "@material/typography/mixins";
Expand All @@ -22,19 +23,26 @@
@import "@material/ripple/mixins";
@import "@material/rtl/mixins";

@mixin mdc-select-dd-arrow-svg-bg_($fill-hex-number: 000000, $opacity: .54) {
background-image: url(data:image/svg+xml,%3Csvg%20width%3D%2210px%22%20height%3D%225px%22%20viewBox%3D%227%2010%2010%205%22%20version%3D%221.1%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%3E%0A%20%20%20%20%3Cpolygon%20id%3D%22Shape%22%20stroke%3D%22none%22%20fill%3D%22%23#{$fill-hex-number}%22%20fill-rule%3D%22evenodd%22%20opacity%3D%22#{$opacity}%22%20points%3D%227%2010%2012%2015%2017%2010%22%3E%3C%2Fpolygon%3E%0A%3C%2Fsvg%3E);
}

$mdc-select-arrow-padding: 26px;
$mdc-select-label-padding: 16px;
$mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timing-function;

.mdc-select {
@include mdc-select-dd-arrow-svg-bg_;
@include mdc-select-fill-color(rgba(white, .1));
@include mdc-select-ink-color(primary);
@include mdc-select-label-color(rgba(black, .6));
@include mdc-select-bottom-line-color(rgba(black, .5));
@include mdc-select-selected-text-color(text-primary-on-light);

// Focused state colors
@include mdc-select-focused-bottom-line-color(primary);
@include mdc-select-focused-label-color(primary);
}

// postcss-bem-linter: define select
.mdc-select {
Copy link
Contributor

Choose a reason for hiding this comment

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

combine this with the above mdc-select...and move the postcss-bem-linter up as well

@include mdc-typography(subheading2);
@include mdc-theme-prop(color, text-primary-on-light);
@include mdc-select-dd-arrow-svg-bg_;

display: inline-flex;
position: relative;
Expand All @@ -54,12 +62,6 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
background-position: left 10px center;
}

@include mdc-theme-dark(".mdc-select") {
@include mdc-select-dd-arrow-svg-bg_("fff", .54);

background-color: rgba(white, .1);
}

&__menu {
position: fixed;
top: 0;
Expand All @@ -71,7 +73,6 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin

&__surface {
@include mdc-typography(subheading2);
@include mdc-theme-prop(color, text-primary-on-light);
@include mdc-rtl-reflexive-property(padding, $mdc-select-label-padding, $mdc-select-arrow-padding, ".mdc-select");
@include mdc-ripple-surface;
@include mdc-ripple-radius;
Expand All @@ -93,7 +94,6 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
border: none;
border-radius: 4px 4px 0 0;
outline: none;
background-color: rgba(black, .04);
// Resets for <select> element
appearance: none;
overflow: hidden;
Expand All @@ -106,14 +106,9 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
bottom: 12px;
transform-origin: left top;
transition: $mdc-select-menu-transition;
color: rgba(black, .6);
pointer-events: none;
will-change: transform;

@include mdc-theme-dark(".mdc-select") {
@include mdc-theme-prop(color, text-secondary-on-dark);
}

@include mdc-rtl(".mdc-select") {
transform-origin: right top;
}
Expand All @@ -132,10 +127,6 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
mdc-animation-exit-temporary(transform, 125ms);
white-space: nowrap;
overflow: hidden;

@include mdc-theme-dark(".mdc-select") {
@include mdc-theme-prop(color, text-secondary-on-dark);
}
}

&__bottom-line {
Expand All @@ -147,11 +138,8 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
transform: scaleY(1);
transform-origin: bottom;
transition: $mdc-select-menu-transition;
background-color: rgba(black, .5);

&::after {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see any ::after in the mixins...did we lose this logic?

Copy link
Contributor Author

@williamernest williamernest Jan 12, 2018

Choose a reason for hiding this comment

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

On focus/open, the bottom-line turned the primary color. Then the ::after psuedo element applied a transition from the center moving outward that is also the primary color.

Purple is the surface:focus .mdc-select__bottom-line element.
Green is the ::after psuedo element animating outward.

Removing the entire ::after element felt like a separate PR.

screen shot 2018-01-11 at 3 59 04 pm

@include mdc-theme-prop(background-color, primary);

position: absolute;
bottom: -1px;
left: 0;
Expand All @@ -175,8 +163,6 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
// JS-enhanced and CSS-only Selects require different selectors for focused bottom-line due to different DOM structure
&__surface:focus .mdc-select__bottom-line,
&__surface:focus ~ .mdc-select__bottom-line {
Copy link
Contributor

Choose a reason for hiding this comment

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

ah...see my comment above where I was confused why we need this.

I would move this to another mixin like this

@mixin mdc-select-focused-bottom-line {
// JS-enhanced and CSS-only Selects require different selectors for focused bottom-line due to different DOM structure
  .mdc-select__surface:focus .mdc-select__bottom-line,
  .mdc-select__surface:focus ~ .mdc-select__bottom-line {
    @content;
  }
}

And then use that mixin here in mdc-select.scss and in mdc-select/_mixins.scss

@include mdc-theme-prop(background-color, primary);

transform: scaleY(2);

&::after {
Expand All @@ -203,8 +189,6 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin
}

.mdc-select__bottom-line {
@include mdc-theme-prop(background-color, primary);

transform: scaleY(2);

&::after {
Expand All @@ -215,7 +199,7 @@ $mdc-select-menu-transition: transform 180ms $mdc-animation-standard-curve-timin

.mdc-select--disabled,
.mdc-select[disabled] {
@include mdc-theme-prop(color, text-disabled-on-light);
@include mdc-select-label-color_(text-disabled-on-light);
@include mdc-select-dd-arrow-svg-bg_(000000, .38);

border-bottom-width: 1px;
Expand Down