Skip to content
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

Add selection CSS classes to tick labels #752

Merged
merged 2 commits into from
Nov 25, 2018
Merged

Add selection CSS classes to tick labels #752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddc8261
Add selection CSS classes to tick labels
duggi May 29, 2017
a3c645b
Add 'label-is-selection' and 'label-in-selection' tests
jespirit Nov 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/js/bootstrap-slider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,24 @@ const windowIsDefined = (typeof window === "object");
}
this.tickLabelContainer.style[styleMargin] = this.sliderElem.offsetWidth / 2 * -1 + 'px';
}

/* Set class labels to indicate tick labels are in the selection or selected */
this._removeClass(this.tickLabels[i], 'label-in-selection label-is-selection');
if (!this.options.range) {
if (this.options.selection === 'after' && percentage >= positionPercentages[0]) {
this._addClass(this.tickLabels[i], 'label-in-selection');
} else if (this.options.selection === 'before' && percentage <= positionPercentages[0]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

anyway we can add some unit tests for this? something as simple as ensuring the tick label elements that get rendered have the expected class tag?

Copy link
Contributor Author

@duggi duggi May 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

will give it a go. using this in production now so had to create a fork and our own bower package. obvi not ideal so will get this done!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@duggi any chance to add the tests? i would love to merge this in!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@duggi any follow up on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rovolution hey sorry for long absence, been busy on other things. could not get the tests authored/working (not my real area of expertise), so put it on hold.

i can ask someone else on my team, or maybe you can give me some quick pointers? i've been working in test/specs/TickMarksSpec.js:

	it("Should show the correct tick labels as 'in-selection', according to the `selection` property", function() {
		var options = {
			ticks: [100, 200, 300, 400, 500],
			value: 250,
			selection: 'after'
		},
		$el = $("#testSlider1");

		testSlider = $el.slider(options);
		expect($el.find('.label-is-selection').length).toBe(1);

		testSlider.slider('destroy');

		options.selection = 'before';
		testSlider = $el.slider(options);
		expect($el.siblings('div.slider').find('.label-is-selection').length).toBe(1);
	});

Copy link
Collaborator

@rovolution rovolution Aug 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@duggi so based on this example snippet, these would quality as 2 different test cases...one to check if the classes are applied if the selection value is 'after', and one to check if the classes are applied if the selection value is 'before'. Therefore, each of these would be in their own it block.

You would want to have tests for checking if .label-is-selection is applied in the proper cases and if .label-in-selection is applied in the proper cases.

Any common setup (like options) should be moved into a beforeEach function, which will execute before each test is run to ensure the common state is reset properly. Likewise, any common teardown setups should be moved into an afterEach block. See the Jasmine docs for further details

You should be working in the /test/specs/TickLabelSpec.js file, since this is more tick label related. You should nest these under a new describe block so that you can add setup/teardown blocks that only run before your new tests for each use-case.

Your assertion's look fine to me, so I definitely think you are on the right track. Here is what I think it would roughly look like.

/* /test/specs/TickLabelSpec.js */

describe("'.label-is-selection' tests", function() {
 var options;
  var testSlider

 // setup that is executed before each test case in this 'describe' function
 beforeEach(function() {
    var options = {
       ticks: [100, 200, 300, 400, 500],
       value: 250,
       selection: 'after'
    };
  });

 // teardown that is executed after each test case in this 'describe' function
 afterEach(function() {
   testSlider.slider('destroy');
 });

 // test cases
 it("Should show the correct tick labels as 'in-selection' if selection is 'after'", function() {
     // Create a new slider
     var testSlider = $("#testSlider1").slider(options);

    // Assert
   expect($el.find('.label-is-selection').length).toBe(1);
 });

 it("Should show the correct tick labels as 'in-selection' if selection is 'before'", function() {
     // TODO
 });
});

describe("'.label-in-selection' tests", function() {
   // TODO
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

@duggi have you had a chance to look at this? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the detailed example, i'll give it a shot soon. been busy as we're launching a new product line, featuring bootstrap-slider!

this is a priority, since we don't want our app dependent on my fork; just gotta clear out some time to put in some work

Copy link
Collaborator

Choose a reason for hiding this comment

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

@duggi thats awesome! if its possible, we would love to see a screenshot or GIF of the slider in your new product! Its always exciting to see how the project is being used :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screenshot in my initial comment ... repasted at bottom.

you can also see it live here, click on "Free Consultation" for any advisor. Note this impl is single value only, not range. also uses different styles.

(please don't actually submit a request)

--

image

this._addClass(this.tickLabels[i], 'label-in-selection');
}
if (percentage === positionPercentages[0]) {
this._addClass(this.tickLabels[i], 'label-is-selection');
}
} else if (percentage >= positionPercentages[0] && percentage <= positionPercentages[1]) {
this._addClass(this.tickLabels[i], 'label-in-selection');
if (percentage === positionPercentages[0] || positionPercentages[1]) {
this._addClass(this.tickLabels[i], 'label-is-selection');
}
}
}
}
}
Expand Down
179 changes: 179 additions & 0 deletions test/specs/TickLabelSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,183 @@ describe("Tick Label Render Tests", function() {
//test both horizontal and vertical orientations
testOrientation('horizontal');
testOrientation('vertical');
});

describe("Tick Labels 'is-selection' and 'in-selection' Tests", function() {
var $inputSlider;
var options;
var keyboardEvent;
var $slider;
var $handle1;
var $handle2;
var $tickLabels;
var tickLabelCount;

// Setup
beforeEach(function() {
options = {
id: 'slider1',
ticks: [0, 1, 2, 3, 4],
value: 2,
ticks_labels:['$0', '$1', '$2', '$3', '$4'],
};

tickLabelCount = options.ticks_labels.length;

// Create keyboard event
keyboardEvent = document.createEvent('Event');
keyboardEvent.initEvent('keydown', true, true);
});

// Cleanup
afterEach(function() {
$inputSlider.slider('destroy');
$inputSlider = null;
});

describe("Tick Labels 'is-selection' Tests", function() {

describe("'options.selection = 'before'", function() {

beforeEach(function() {
options.selection = 'before';
$inputSlider = $('#testSlider1').slider(options);
$slider = $('#slider1');
$tickLabels = $slider.find('.slider-tick-label-container div.slider-tick-label');
});

it("Should show the correct tick labels as 'is-selection'", function() {
// There should only be one tick label with the 'label-is-selection' class
expect($slider.find('.label-is-selection').length).toBe(1);
// Only the third tick label should have the 'label-is-selection' class
expect($tickLabels.eq(2).hasClass('label-is-selection')).toBe(true);
});

it("Should show the correct tick labels as 'is-selection' when keying to the left", function(done) {
$handle1 = $('#slider1').find('.slider-handle:first');

expect($slider.find('.label-is-selection').length).toBe(1);
expect($tickLabels.eq(2).hasClass('label-is-selection')).toBe(true);

$handle1.on('keydown', function() {
expect($slider.find('.label-is-selection').length).toBe(1);
expect($tickLabels.eq(1).hasClass('label-is-selection')).toBe(true);
done();
});

// Move handle1 to the left with keyboard
$handle1.focus();
keyboardEvent.keyCode = keyboardEvent.which = 37;
$handle1[0].dispatchEvent(keyboardEvent);
});
});

describe("'options.selection = 'after'", function() {

beforeEach(function() {
options.selection = 'after';
$inputSlider = $('#testSlider1').slider(options);
$slider = $('#slider1');
$tickLabels = $slider.find('.slider-tick-label-container div.slider-tick-label');
});

it("Should show the correct tick labels as 'is-selection'" , function() {
expect($slider.find('.label-is-selection').length).toBe(1);
expect($tickLabels.eq(2).hasClass('label-is-selection')).toBe(true);
});

it("Should show the correct tick labels as 'is-selection' when keying to the right" , function(done) {
$handle1 = $('#slider1').find('.slider-handle:first');

expect($slider.find('.label-is-selection').length).toBe(1);
expect($tickLabels.eq(2).hasClass('label-is-selection')).toBe(true);

$handle1.on('keydown', function() {
expect($slider.find('.label-is-selection').length).toBe(1);
expect($tickLabels.eq(3).hasClass('label-is-selection')).toBe(true);
done();
});

// Move handle1 to the right with keyboard
$handle1.focus();
keyboardEvent.keyCode = keyboardEvent.which = 39;
$handle1[0].dispatchEvent(keyboardEvent);
});
});
});

describe("Tick Labels 'in-selection' Tests", function() {

function checkTickLabels($labels, expectedLabels) {
var next = 0;

// There are only 5 tick labels.
expect($labels.length).toBe(tickLabelCount);

for (var i = 0; i < tickLabelCount; i++) {
if (i === expectedLabels[next]) {
expect($labels.eq(i).hasClass('label-in-selection')).toBe(true);
next++;
}
else {
expect($labels.eq(i).hasClass('label-in-selection')).toBe(false);
}
}
}

// Setup
beforeEach(function() {
options.value = [1, 3];
$inputSlider = $('#testSlider1').slider(options);
$slider = $('#slider1');
$tickLabels = $slider.find('.slider-tick-label-container div.slider-tick-label');
});

it("Should show the correct tick labels as 'in-selection'", function() {
expect($slider.find('.label-is-selection').length).toBe(3);
checkTickLabels($tickLabels, [1, 2, 3]);
});

it("Should show the correct tick labels as 'in-selection' when keying to the left", function(done) {
$handle1 = $('#slider1').find('.slider-handle:first');

// There should be 3 tick labels with the 'label-in-selection' class
expect($slider.find('.label-in-selection').length).toBe(3);

// Check that the correct tick labels have the 'label-in-selection' class
checkTickLabels($tickLabels, [1, 2, 3]);

$handle1.on('keydown', function() {
expect($slider.find('.label-in-selection').length).toBe(4);

// Check the labels again
checkTickLabels($tickLabels, [0, 1, 2, 3]);
done();
});

// Move handle1 to the left with keyboard
$handle1.focus();
keyboardEvent.keyCode = keyboardEvent.which = 37;
$handle1[0].dispatchEvent(keyboardEvent);
});

it("Should show the correct tick labels as 'in-selection' when keying to the right" , function(done) {
$handle2 = $('#slider1').find('.slider-handle:last');

expect($slider.find('.label-in-selection').length).toBe(3);

checkTickLabels($tickLabels, [1, 2, 3]);

$handle2.on('keydown', function() {
expect($slider.find('.label-in-selection').length).toBe(4);
checkTickLabels($tickLabels, [1, 2, 3, 4]);
done();
});

// Move handle2 to the right with keyboard
$handle2.focus();
keyboardEvent.keyCode = keyboardEvent.which = 39;
$handle2[0].dispatchEvent(keyboardEvent);
});
});
});