Skip to content

feat(material-experimental/mdc-slider): add slider styles #21934

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Feb 22, 2021
Merged

feat(material-experimental/mdc-slider): add slider styles #21934

merged 4 commits into from
Feb 22, 2021

Conversation

wagnermaciel
Copy link
Contributor

  • implement _MatSliderMixinBase
  • add color input to MatSlider
  • extend _MatSliderMixinBase from MatSlider
  • use without-ripple mixin for slider.scss
  • @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
  • implement primary, accent, and warn colors in _slider-theme.scss

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 17, 2021
jelbourn
jelbourn approved these changes Feb 17, 2021
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the target: feature This PR is targeted for a feature branch (outside of main and semver branches) label Feb 17, 2021
mmalerba
mmalerba reviewed Feb 17, 2021
View reviewed changes
crisbeto
crisbeto approved these changes Feb 18, 2021
View reviewed changes
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@wagnermaciel
Copy link
Contributor Author

@mmalerba I made those changes you requested and also changed the demo to show all the different colors in their normal & disabled state. This is what it looks like

Screen Shot 2021-02-18 at 3 21 41 PM

mmalerba
mmalerba reviewed Feb 19, 2021
View reviewed changes
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

The ticks on the disabled slider look a little weird, I wonder if they should be black rather than white? The spec (of course) doesn't show a disabled slider with ticks, so we can probably just leave it like this and bring it up with MDC if we don't like it.

@wagnermaciel
Copy link
Contributor Author

wagnermaciel commented Feb 20, 2021 •
edited
Loading

@mmalerba You were right about the disabled inactive tick marks (great eye, btw). I think the styles are all correct now but lmk if you spot anything else 👍🏽

Light theme

Screen Shot 2021-02-19 at 5 30 10 PM

Dark theme

Screen Shot 2021-02-19 at 5 29 55 PM

@wagnermaciel
feat(material-experimental/mdc-slider): add slider styles
dfc76e4 
* implement _MatSliderMixinBase
* add color input to MatSlider
* extend _MatSliderMixinBase from MatSlider
* use without-ripple mixin for slider.scss
* @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
* implement primary, accent, and warn colors in _slider-theme.scss
@wagnermaciel
fix(material-experimental/mdc-slider): code review changes
b167da2 
* use @use instead of @import for @material/slider styles
* make tooltip color change when in dark theme
* add underscore to custom-slider-color to ensure it doesn't get exported
@wagnermaciel
fix(material-experimental/mdc-slider): code review changes
c85e62a 
* move tick-mark-active-color into _custom-slider-color to ensure it uses the correct on-color
* add an -color arg to _custom-slider-color so we don't just use on-primary for everything
* added more examples to mdc-slider-demo.html to make a pretty example from these changes
@wagnermaciel
fix(material-experimental/mdc-slider): code review changes
1e4f842 
* increase opacity of value indicator tooltip when in dark mode
* use surface and on-surface for tick-mark-active and tick-mark-inactive disabled colors
mmalerba
mmalerba approved these changes Feb 22, 2021
View reviewed changes
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@wagnermaciel wagnermaciel added the action: merge The PR is ready for merge by the caretaker label Feb 22, 2021
@wagnermaciel wagnermaciel merged commit e5cbc8c into angular:mdc-slider Feb 22, 2021
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 4, 2021
@wagnermaciel
feat(material-experimental/mdc-slider): add slider styles (angular#21934
2e30ab0 
)

* feat(material-experimental/mdc-slider): add slider styles

* implement _MatSliderMixinBase
* add color input to MatSlider
* extend _MatSliderMixinBase from MatSlider
* use without-ripple mixin for slider.scss
* @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
* implement primary, accent, and warn colors in _slider-theme.scss
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 11, 2021
@wagnermaciel
feat(material-experimental/mdc-slider): add slider styles (angular#21934
bd59cf8 
)

* feat(material-experimental/mdc-slider): add slider styles

* implement _MatSliderMixinBase
* add color input to MatSlider
* extend _MatSliderMixinBase from MatSlider
* use without-ripple mixin for slider.scss
* @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
* implement primary, accent, and warn colors in _slider-theme.scss
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 11, 2021
@wagnermaciel
feat(material-experimental/mdc-slider): add slider styles (angular#21934
5788d4f 
)

* feat(material-experimental/mdc-slider): add slider styles

* implement _MatSliderMixinBase
* add color input to MatSlider
* extend _MatSliderMixinBase from MatSlider
* use without-ripple mixin for slider.scss
* @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
* implement primary, accent, and warn colors in _slider-theme.scss
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 12, 2021
@wagnermaciel
feat(material-experimental/mdc-slider): add slider styles (angular#21934
3d84822 
)

* feat(material-experimental/mdc-slider): add slider styles

* implement _MatSliderMixinBase
* add color input to MatSlider
* extend _MatSliderMixinBase from MatSlider
* use without-ripple mixin for slider.scss
* @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
* implement primary, accent, and warn colors in _slider-theme.scss
wagnermaciel added a commit to wagnermaciel/components that referenced this pull request Mar 23, 2021
@wagnermaciel
feat(material-experimental/mdc-slider): add slider styles (angular#21934
cbbfd2b 
)

* feat(material-experimental/mdc-slider): add slider styles

* implement _MatSliderMixinBase
* add color input to MatSlider
* extend _MatSliderMixinBase from MatSlider
* use without-ripple mixin for slider.scss
* @include all other mdc-slider mixins except thumb-ripple-color in _slider-theme.scss
* implement primary, accent, and warn colors in _slider-theme.scss
@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 Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn approved these changes

@crisbeto crisbeto crisbeto approved these changes

@mmalerba mmalerba mmalerba approved these changes

@devversion devversion Awaiting requested review from devversion devversion is a code owner

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: feature This PR is targeted for a feature branch (outside of main and semver branches)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wagnermaciel @jelbourn @crisbeto @mmalerba