feat(slider): md-reverse

[soooooot]

make min value to top/right, max value to bottom/left, details on #7666

[devversion]

Very well implemented. LGTM.

@EladBezalel Do we want to support this?

[devversion]

Just one little thing (nitpicky): The arithmetic operator should be spaced, just to be consistent.

[EladBezalel]

@soooooot thanks for this, please next time open an issue just to make order,
This seems good, please fix @devversion comment, squash into 1 commit and follow our commit guideline.

thanks again, LGTM.

@devversion do you mind manual test it?

[soooooot]

@EladBezalel I fired an issue here issue #7666, next time I will mark it clear if I have a PR to create.

There's a CodePen link posted in issue, maybe it can help you guys to manual test it.

I will fix the points you mentioned.

[devversion]

@EladBezalel Just tested it locally and it works like a charm.

[topherfangio]

I'd like to have a bit more discussion about this before we merge it. I'm not opposed to adding this functionality, but I think the terminology "reverse" is a bit confusing and would like to discuss some other options.

When I first saw this PR, I assumed it meant that the entire component would be reversed as if it were being used in an RTL setting. However, visually, the slider works identically to the existing slider, it's just that it counts downward with the value instead of upward.

So, rather than md-reverse, I'm thinking it might be more appropriate to call it md-invert-value or md-count-down.

@EladBezalel @devversion Thoughts?

[EladBezalel]

Oh, isn't that as RTL?

[devversion]

@topherfangio Yes, md-invert-value sounds for me very good. But what about md-invert this sounds even better for me and is not confusingly

[topherfangio]

@EladBezalel In my mind, RTL would start the slider at the right all grey, and the accent color (pink in our demo) would grow from the right to the left. Currently, it's still left-to-right even in reverse:

That said, I unfortunately don't know much about how visual elements are represented in RTL cultures.

@devversion I worry that without the -value, people may think it affects the visual display of the element as well and thus has a similar problem to md-reverse (IMO 😄).

[soooooot]

md-invert-value makes more sense to me.

I have no any experience on RTL too. I describe my use-case FYI.

what I wanna do is a vertical, value increasing top-to-down, pagination slider. unfortunately the current md-slider does not support it. For such a small code change, I have to copy all the md-slider js code and paste it to another new md-reverse-slider (maybe md-invert-value-slider is better, 😆 ). And as you know, the css is generated and inject in run-time, and the css is all base on md-slider tagname, it hard for me to put my new md-reverse-slider directive into the run-time css injection list. Also it will takes time to copy all the css rules. So I did a little bit hack stuff in my compile function to change my md-reverse-slider tagname to md-slider to get the styles. and last to override the _md-track-fill(the accent pink part) color to transparent.

that's my story.

[topherfangio]

For reference, this is how I assume the RTL slider would work: http://demos.telerik.com/kendo-ui/slider/right-to-left-support

If you can update this PR to use md-invert-value, I think we can run it by Thomas and see if we can get it merged 😄

[soooooot]

it seems like md-invert-value will be temporarily stuff, sooner or later If we implemented rtl, it will be deprecated or obsolete.

I made a new CodePen to make it visual reversed( the accent pink part) just like the demo @topherfangio posted.

And I also updated the PR, so you guys can check the changes or you can check on my personal branch commit to get the visual part changes.

I didn't change the name yet, since we should rename it depends on the behaviour right? rtl, invert or reverse.

[topherfangio]

@soooooot If we implement real RTL support, could you use it on just that single slider for your purposes? If so, I think that would definitely be a better solution.

[soooooot]

@topherfangio yep, I think RTL single slider can implement a pagination slider.

The PR I updated yesterday did implement the RTL feature in my opinion, or I missed something?

[topherfangio]

@soooooot Sorry for the delay (was on vacation). True RTL support just looks at the dir="rtl" attribute on a parent element (usually the body) rather than using a md-reverse or md-invert-value attribute on the actual component.

For instance, it should work if you did something like this:

<div dir="rtl">
    <md-slider min="0" max="255"></md-slider>
</div>

@EladBezalel Does this sound correct to you?

[EladBezalel]

@topherfangio it won't work, the dir attribute is only taking place with our code if it's being applied on body or html..

[soooooot]

@topherfangio After some days to think about the issue again, I found that even in rtl mode, md-invert is also useful to make a regular ltr slider in some cases. So, can I just update the PR to rename md-reverse to md-invert. (I perfer to the one without -value since now it will be inverted both in value and visual display). Can we end up the PR with the md-invert feature only? if we need more discussion on the rtl mode, it's better to fire an new issue. Although the md-invert is related to rtl mode, but we need it in rtl mode too, we cannot merge the md-invert to rtl.

@EladBezalel I have a rough idea to make 'md-invert' to work with rtl mode, we should invert it depends on rtl XOR md-invert....(I just make a start if you don't agree to discuss the rtl in another issue..)

I will rename it as soon as I see your reply with a confirmed name. md-invert, md-invert-value or whatever.

[topherfangio]

@soooooot I think md-invert is good.

@EladBezalel Are you sure about RTL on body only? When I just tested it, all I did was add dir="rtl" to the demo wrapper (from within the Chrome inspector) and the Switch component swapped into RTL mode. Based on https://github.com/angular/material/blob/master/src/core/style/mixins.scss#L111, it looks like all of the RTL mixins use the attribute selector only and don't specify the body.

[EladBezalel]

@topherfangio so i guess it's fixed, rtl mixin was once only on html and body.. good for us! :D

also md-invert sounds good to me as well

[ThomasBurleson]

This issue is closed as part of our ‘Surge Focus on Material 2' efforts.
For details, see our forum posting @ http://bit.ly/1UhZyWs.