Closed sureshvv closed 4 years ago
Why not just check if max < min instead of adding a new prop?
Why not just check if max < min instead of adding a new prop?
yeah that would be pretty neat, altho it would kind of defeat the purpose of "max" and "min" as concepts. maybe the props should've been called start
and end
🤷♂
Why not just check if max < min instead of adding a new prop?
yeah that would be pretty neat, altho it would kind of defeat the purpose of "max" and "min" as concepts. maybe the props should've been called
start
andend
or left and right would be even better :D
This is to give you an idea of why it may be useful. Or if you know how this can be done without a new property.
@sureshvv Where would you display left
value in RTL? On the left (like in LTR) or on the right?
@sureshvv Where would you display
left
value in RTL? On the left (like in LTR) or on the right?
The beauty of left and right is they don't change with rtl. Left is left is left.
And the cons are that
And the cons are that
- you'll need another props for vertical slider (there's no left/right)
- i'm not sure about this, so correct me if i'm wrong, but i guess that the default layout in RTL mode is just the mirror image of LTR, so in multilanguage site if you want to switch from LTR to RTL you'll need to switch left and right values
yep.. may be top and bottom for vertical sliders
yep.. we are punting on rtl and let the individual programmer worry about it - but it may be all right for most users. not really sure about that though :)
@sh7dm Can we merge?
Have we settled on the extra prop in the discussion above?
Merging #7215 into next will decrease coverage by
0.01%
. The diff coverage is91.66%
.
@@ Coverage Diff @@
## next #7215 +/- ##
==========================================
- Coverage 85.79% 85.78% -0.02%
==========================================
Files 329 329
Lines 8610 8616 +6
Branches 2157 2160 +3
==========================================
+ Hits 7387 7391 +4
- Misses 1133 1134 +1
- Partials 90 91 +1
Impacted Files | Coverage Δ | |
---|---|---|
packages/vuetify/src/components/VSlider/VSlider.ts | 97.19% <91.66%> (-0.54%) |
:arrow_down: |
packages/vuetify/src/components/VSelect/VSelect.ts | 93.99% <0%> (-0.43%) |
:arrow_down: |
...ages/vuetify/src/components/VTextarea/VTextarea.ts | 100% <0%> (ø) |
:arrow_up: |
...es/vuetify/src/components/VTextField/VTextField.ts | 95.41% <0%> (+0.1%) |
:arrow_up: |
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 2a57376...1b51ee1. Read the comment docs.
Have we settled on the extra prop in the discussion above?
You can check out the discussion on #6814 which was the previous version of this.
This Pull Request is being closed due to inactivity.
Thank you for your contribution and interest in improving Vuetify!
This is ready to be merged. Please merge. @sh7dm has our moment passed? :D
There's still no consensus on the props names
There's still no consensus on the props names
Let us vote and settle on one please. We have used "reverse", "decreasing", "reversed" so far.
My vote is for whatever we've already used before. Which I think is reverse
?
My vote is also for "reverse".
I'm ok with reverse. The min/max thing sounds confusing, and left/right doesn't make sense for rtl or vertical.
The problem with reverse
is that for text fields it aligns label and input texts, it could be used for placing label on the opposite side also in v-slider and selection controls (there's an issue for this afaik).
Definitely I agree that min/max and left/right are not the best, but there is still start/end option, could be also from-value/to-value or just from/to or maybe even something better
And another thing - with this playground:
<template>
<v-app>
<v-content>
<v-container>
<v-slider min="5" max="15" :tick-labels="[5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]" />
<v-slider min="5" max="15" :tick-labels="[5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]" reverse />
<v-slider min="5" max="15" :tick-labels="[5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]" vertical />
<v-slider min="5" max="15" :tick-labels="[5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]" vertical reverse />
</v-container>
</v-content>
</v-app>
</template>
i got this:
So first of all labels are wrong in reverse mode, second - I'd expect that the initial value will be the left side value (with from
/to
it would be less confusing), so for normal slider it'd be 5, for reverse - 15, third - keyboard navigation with up/down keys is reversed in reverse mode (up
moves the slider indicator down)
@jacekkarczmarczyk... your playground works correctly for me now.
Great, ty, still there are 2 other issues in my comment
Hey @sureshvv, Please read the following log in order to understand the failure reason. It'll be awesome if you fix what's wrong and commit the changes.
Description
Motivation and Context
How Has This Been Tested?
Markup:
Types of changes
Checklist:
master
for bug fixes and documentation updates,dev
for new features and breaking changes).