Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Support for tick in Slider #1529

Open
wants to merge 5 commits into from

2 participants

@customcommander

Hi,

Following up #1500 and the discussion with @lsmith, here is an attempt to add support for tick in Slider.

This PR doesn't include documentation yet as I'm not entirely sure about the implementation. (But will do if you're happy with it.)

Thanks

@customcommander

Hey @lsmith just wondering whether you've had a chance to look at this?

@lsmith

Sorry for the delay @customcommander. Looks like the value will properly increment, but the thumb won't jump to the corresponding location of that tick either during drag or after drag. Is that true?

Otherwise, looks good.

@customcommander

No worries.

Just had a look and at first I noticed a behavior similar to what you described but then I rebuilt the module and now it looks fine to me:

When I click on the rail, both value and thumb represent the logical tick value. During and after drag, both value and thumb look good.

Could it be that you didn't rebuild the module? Or am I misunderstanding something?

@customcommander

@lsmith Could you make sure that the module has been rebuilt?

@lsmith

I was judging from the code. I didn't build or test a local merge. My schedule is full today. I'll look at it or ping you back Monday.

@customcommander

I assumed that you cherry-picked my changes. There is a manual test page at tests/manual/tick.page if needed.

@lsmith

Ok, I looked it over, and confirmed the UX is (almost) what I anticipated given the code. That is, the thumb can rest in any position on the rail, but the value rounds to increments of tick. What I'm hoping to see is either:

A) the thumb snaps between locations on the rail that correspond to the tick value intervals while sliding (ideal), or
B) the thumb follows the mouse, but after mouseup snaps to the appropriate value tick location

Additionally, I saw some jankiness while dragging across value thresholds where the thumb would jump around momentarily.

@customcommander

Thanks for the feedback.

Actually I thought that the thumb snapping to the "next value" would be a feature for a slider with steps. So I intentionally left it for another PR.

It looks like the two features could be merged although I am not sure whether they will always work well together.

How should a Slider react with the given config?
(Assuming that there is a step attribute)

new Y.Slider({
    min: 0, 
    max: 500,
    tick: 5,
    step: 10
});

The step attribute kind of restrict the value to be 0, 100, 200, 300, 400 or 500 while the tickattribute allows the value to be anything that is a multiple of 5. Or would you implement the step feature in a sub-class? e.g. step-range.js

Your comment does make sense but I can also see some overlap with step. I'll see what I can do anyway.

Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 5, 2014
  1. @customcommander

    Slider: Defines a tick attribute

    customcommander authored
    This commit doesn't implement a tick behaviour yet.
    It only defines a new attribute and add a few unit tests for its validator.
  2. @customcommander
  3. @customcommander
  4. @customcommander
  5. @customcommander
This page is out of date. Refresh to see the latest.
View
94 src/slider/js/value-range.js
@@ -10,6 +10,7 @@
// Constants for compression or performance
var MIN = 'min',
MAX = 'max',
+ TICK = 'tick',
VALUE = 'value',
// MINORSTEP = 'minorStep',
// MAJORSTEP = 'majorStep',
@@ -292,6 +293,46 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
},
/**
+ * Validates new values assigned to `tick` attribute.
+ *
+ * A valid value must be a number and must evenly divide the length of
+ * the slider.
+ *
+ * @method _validateNewTick
+ * @param value { any } Value assigned to `tick` attribute.
+ * @return { Boolean } True for a valid tick value. False otherwise.
+ * @protected
+ */
+ _validateNewTick: function ( value ) {
+ var length = parseInt(this.get( 'length' ), 10);
+ return Y.Lang.isNumber( value ) && length % value === 0;
+ },
+
+ /**
+ * Getter for the `minorStep` attribute.
+ *
+ * @method _getMinorStep
+ * @param value { Number } The value of the attribute before it is massaged/normalized.
+ * @return { Number } The value of the `tick` attribute if it is defined, otherwise returns the `minorStep` value.
+ * @protected
+ */
+ _getMinorStep: function ( value ) {
+ return this.get( TICK ) || value;
+ },
+
+ /**
+ * Getter for the `majorStep` attribute.
+ *
+ * @method _getMajorStep
+ * @param value { Number } The value of the attribute before it is massaged/normalized.
+ * @return { Number } The value of the `tick` attribute if it is defined, otherwise returns the `majorStep` value.
+ * @protected
+ */
+ _getMajorStep: function ( value ) {
+ return this.get( TICK ) || value;
+ },
+
+ /**
* Restricts new values assigned to <code>value</code> attribute to be
* between the configured <code>min</code> and <code>max</code>.
* Rounds to nearest integer value.
@@ -311,8 +352,12 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
/**
* Returns the nearest valid value to the value input. If the provided
* value is outside the min - max range, accounting for min > max
- * scenarios, the nearest of either min or max is returned. Otherwise,
- * the provided value is returned.
+ * scenarios, the nearest of either min or max is returned.
+ *
+ * If `tick` is defined and value is not a multiple of tick, the nearest
+ * multiple value is returned.
+ *
+ * Otherwise the provided value is returned.
*
* @method _nearestValue
* @param value { mixed } Value to test against current min - max range
@@ -320,8 +365,9 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
* @protected
*/
_nearestValue: function ( value ) {
- var min = this.get( MIN ),
- max = this.get( MAX ),
+ var min = this.get( MIN ),
+ max = this.get( MAX ),
+ tick = this.get( TICK ),
tmp;
// Account for reverse value range (min > max)
@@ -329,11 +375,21 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
min = ( max > min ) ? min : max;
max = tmp;
- return ( value < min ) ?
- min :
- ( value > max ) ?
- max :
- value;
+ value = ( value < min )
+ ? min
+ : ( value > max )
+ ? max
+ : value;
+
+ // if tick is defined and value is not a multiple of tick
+ // we find the nearest multiple value.
+ if ( tick && ( value % tick ) ) {
+ // e.g. tick is 7 and value is 15; wrong!
+ // round(15/7) = 2; 2*7=14; best candidate!
+ value = tick * round( value / tick );
+ }
+
+ return value;
}
},
@@ -376,6 +432,20 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
},
/**
+ * If defined (i.e. greater than zero) it constrains the value of the slider
+ * to always be a multiple of _tick_. This effectively defines a whitelisted
+ * set of values.
+ *
+ * @attribute tick
+ * @type { Number }
+ * @default 0
+ */
+ tick: {
+ value : 0,
+ validator: '_validateNewTick'
+ },
+
+ /**
* amount to increment/decrement the Slider value
* when the arrow up/down/left/right keys are pressed
*
@@ -384,7 +454,8 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
* @default 1
*/
minorStep : {
- value: 1
+ value : 1,
+ getter: '_getMinorStep'
},
/**
@@ -396,7 +467,8 @@ Y.SliderValueRange = Y.mix( SliderValueRange, {
* @default 10
*/
majorStep : {
- value: 10
+ value : 10,
+ getter: '_getMajorStep'
},
/**
View
30 src/slider/tests/manual/tick.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <script src="../../../../build/yui/yui.js"></script>
+ <script>
+ YUI().use('console', 'slider', function (Y) {
+
+ Y.one(window).on('load', function () {
+
+ var slider = new Y.Slider({ tick: 10, value: 50, length: '300px' });
+ var log = new Y.Console({ style: 'block' });
+
+ slider.after('valueChange', function () {
+ Y.log('value is now '+slider.getValue());
+ });
+
+ slider.render('#slider');
+
+ log.render('#log');
+ });
+ });
+ </script>
+ </head>
+ <body class="yui3-skin-sam">
+ <h3>Slider + tick</h3>
+ <p>Regardless of whether you used the keyboard or the mouse, the value of the slider should always be a multiple of <em>tick</em> at any time!</p>
+ <div id="slider"></div>
+ <div id="log"></div>
+ </body>
+</html>
View
167 src/slider/tests/unit/assets/slider-tests.js
@@ -593,6 +593,24 @@ suite.add( new Y.Test.Case({
slider.destroy();
},
+ "tick should default to 0": function () {
+ var slider = new Y.Slider();
+ Y.Assert.areSame(0, slider.get('tick'));
+ slider.destroy();
+ },
+
+ "tick should reject a non-number value": function () {
+ var slider = new Y.Slider({ tick: '12' });
+ Y.Assert.areSame(0, slider.get('tick'));
+ slider.destroy();
+ },
+
+ "tick should reject a value that doesn't evenly divide length": function () {
+ var slider = new Y.Slider({ length: '100px', tick: 7 });
+ Y.Assert.areSame(0, slider.get('tick'));
+ slider.destroy();
+ },
+
"test value": function () {
var slider = new Y.Slider({ min: 0, max: 100, value: 50 });
@@ -642,6 +660,18 @@ suite.add( new Y.Test.Case({
slider.destroy();
},
+ "setting the value outside of the tick 'whitelist' should constrain it": function () {
+ var slider = new Y.Slider({ tick: 7, length: '70px' });
+
+ slider.setValue(15);
+ Y.Assert.areSame(14, slider.getValue(), 'Expected 15 to be constrained to 14');
+
+ slider.setValue(20);
+ Y.Assert.areSame(21, slider.getValue(), 'Expected 20 to be constrained to 21');
+
+ slider.destroy();
+ },
+
"setting the min or max should update the value if necessary": function () {
var slider = new Y.Slider({ min: 0, max: 100, value: 50 });
@@ -746,6 +776,143 @@ suite.add( new Y.Test.Case({
}));
suite.add( new Y.Test.Case({
+
+ name: "Mouse - with tick",
+
+ tick: 10,
+
+ /**
+ * Simulate a click on the rail.
+ * @param distance { Number } Distance relative to the start of the rail
+ */
+ clickOnRail: function (distance) {
+ var region = this.slider.rail.get('region');
+ this.slider._onRailMouseDown({
+ clientX: region.left + distance,
+ clientY: region.top + Math.floor(region.height / 2),
+ pageX: region.left + distance,
+ pageY: region.top + Math.floor(region.height / 2),
+ halt: function () {}
+ });
+ },
+
+ setUp: function () {
+ Y.one('body').append('<div id="mouse_tick"></div>');
+ this.slider = new Y.Slider({ tick: this.tick, length: '100px' });
+ this.slider.render('#mouse_tick');
+ },
+
+ tearDown: function () {
+ this.slider.destroy();
+ Y.one('#mouse_tick').remove();
+ },
+
+ "clicking on the rail should set the value to a multiple of tick": function () {
+
+ // we should still be close to the start of the rail
+
+ this.clickOnRail(7);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(14);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(21);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(28);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(35);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ // we should be half-way
+
+ this.clickOnRail(42);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(49);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(56);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ // we should get closer to the end of the rail
+
+ this.clickOnRail(63);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(70);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+
+ this.clickOnRail(77);
+ Y.Assert.areSame(0, this.slider.getValue() % this.tick);
+ },
+
+ _should: {
+ ignore: {
+ "clicking on the rail should set the value to a multiple of tick": Y.UA.phantomjs || Y.UA.touchEnabled
+ }
+ }
+}));
+
+suite.add( new Y.Test.Case({
+
+ name: "Keyboard - with tick",
+
+ tick: 10,
+
+ value: 50,
+
+ init: function () {
+ Y.one('body').append('<div id="slider_tick"></div>');
+ },
+
+ destroy: function () {
+ Y.one('#slider_tick').remove();
+ },
+
+ setUp: function () {
+ this.slider = new Y.Slider({ tick: this.tick, length: '100px', value: this.value });
+ this.slider.render('#slider_tick');
+ },
+
+ tearDown: function () {
+ this.slider.destroy(true);
+ },
+
+ "page up increments by tick": function () {
+ this.slider.thumb.key(33);
+ Y.Assert.areSame(this.value + this.tick, this.slider.getValue());
+ },
+
+ "arrow up increments by tick": function () {
+ this.slider.thumb.key(38);
+ Y.Assert.areSame(this.value + this.tick, this.slider.getValue());
+ },
+
+ "arrow right increments by tick": function () {
+ this.slider.thumb.key(39);
+ Y.Assert.areSame(this.value + this.tick, this.slider.getValue());
+ },
+
+ "page down decrements by tick": function () {
+ this.slider.thumb.key(34);
+ Y.Assert.areSame(this.value - this.tick, this.slider.getValue());
+ },
+
+ "arrow down decrements by tick": function () {
+ this.slider.thumb.key(40);
+ Y.Assert.areSame(this.value - this.tick, this.slider.getValue());
+ },
+
+ "arrow left decrements by tick": function () {
+ this.slider.thumb.key(37);
+ Y.Assert.areSame(this.value - this.tick, this.slider.getValue());
+ }
+}));
+
+suite.add( new Y.Test.Case({
name: "Keyboard",
setUp: function () {
Something went wrong with that request. Please try again.