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(material-experimental/mdc-slider): implement slider thumb ripples #21979
feat(material-experimental/mdc-slider): implement slider thumb ripples #21979
Conversation
wagnermaciel
commented
Feb 22, 2021
- create MatSliderVisualThumb
* create MatSliderVisualThumb
23e9348
to
e3314e7
Compare
@@ -110,4 +110,10 @@ | |||
base: mdc-theme-prop-value($color) | |||
), | |||
)); | |||
.mdc-slider-hover-ripple { |
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.
Should we document where we get these opacity values? Is it possible to grab em from MDC?
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.
As far as I'm aware, no. They don't seem to have a specific opacity for their ripples. I was just visually following the MDC Slider Spec
I did see that mdc does define a ripple opacity var for the mdc-radio, which we use in our implementation. Should I create an issue for them to do this
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.
Yeah, if they could expose the opacity then that would be helpful for us and one less thing we need to maintain
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 created Issue #6876 requesting they fix their current ripple implementation (they are missing the "pressed" state) and then expose the 'hover', 'focused', and 'pressed' ripple opacities
</div> | ||
<div class="mdc-slider__thumb-knob" #knob></div> | ||
</div> | ||
<mat-slider-visual-start-thumb |
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.
Should start a discussion on naming this thing- having a thumb directive and private thumb component is a bit confusing.
* create slider-thumb.scss and move mat-ripple styles there * specify mat-ripple selector so that it only styles mat-mdc-slider ripples * use mat-mdc-slider class instead of just mdc to avoid styling mdc components * move mouseenter and mouseleave event listeners outside of angular * unsubscribe from event emitters in ngOnDestroy of MatSliderVisualThumb
|
||
/** Whether the given rippleRef is currently fading in or visible. */ | ||
private _isShowingRipple(rippleRef?: RippleRef): boolean { | ||
return rippleRef?.state === RippleState.FADING_IN || rippleRef?.state === RippleState.VISIBLE; |
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.
how about rippleRef && rippleRef.state !== RippleSate.HIDDEN
(that covers fading out too)
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.
Wouldn't I also need to check rippleRef.state === RippleState.FADING_OUT
, too?
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.
isn't fading out also visible?
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.
Fading out is still visible but it's on the way out so we don't want to return true for that because that would prevent a new ripple from launching in the place of the one about to fade out
So we'd end up with the inverse of what is there now
return rippleRef?.state !== RippleState.HIDDEN && rippleRef?.state !== RippleState.FADING_OUT
* use mat-mdc prefix instead of just mdc for custom classes
* add @docs-private to MatSliderVisualThumbs component description * remove underscores from MatSLiderVisualThumbs @inputs * add mat-slider-visual-thumb class to MatSliderVisualThumbs host element * use mat-slider-visual-thumb class instead of mat-mdc-slider for the mat-ripple css selector
* use single selector for MatSliderVisualThumb * add @input() thumbPosition to MatSliderVisualThumb * replace old _thumbPosition var with new @input() thumbPosition * in MatSlider give @input() displayWith() a default value * slightly simplify value indicator text setter * create value indicator text getter * use *ngFor to display visual thumbs
* use mat-mdc- prefix instead of just mat- * bind this instead of using arrow fns
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
angular#21979) * feat(material-experimental/mdc-slider): implement slider thumb ripples * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * feat(material-experimental/mdc-slider): implement slider thumb ripples * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * feat(material-experimental/mdc-slider): implement slider thumb ripples * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
angular#21979) * create MatSliderVisualThumb * create slider-thumb.html & slider-thumb.scss
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |