Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Make spinbox trigger changed after setValue #1960

Merged
merged 2 commits into from
May 3, 2017
Merged

Make spinbox trigger changed after setValue #1960

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
d9fd9b9
Make spinbox trigger changed after setValue
Apr 17, 2017
b0052c3
Merge branch 'master' into fix-spinbox-changed
May 3, 2017
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: 13 additions & 5 deletions js/spinbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,11 @@
},

render: function render() {
this.setValue(this.getDisplayValue());
this._setValue(this.getDisplayValue());
},

change: function change() {
this.setValue(this.getDisplayValue());
this._setValue(this.getDisplayValue());

this.triggerChangedEvent();
},
Expand Down Expand Up @@ -222,7 +222,7 @@

step: function step(isIncrease) {
//refresh value from display before trying to increment in case they have just been typing before clicking the nubbins
this.setValue(this.getDisplayValue());
this._setValue(this.getDisplayValue());
var newVal;

if (isIncrease) {
Expand All @@ -233,7 +233,7 @@

newVal = newVal.toFixed(5);

this.setValue(newVal + this.unit);
this._setValue(newVal + this.unit);
},

getDisplayValue: function getDisplayValue() {
Expand All @@ -255,6 +255,10 @@
},

setValue: function setValue(val) {
return this._setValue(val, true);
},

_setValue: function _setValue(val, shouldSetLastValue) {
//remove any i18n on the number
if (this.options.decimalMark !== '.') {
val = this.parseInput(val);
Expand All @@ -271,7 +275,7 @@

//make sure we are dealing with a number
if (isNaN(intVal) && !isFinite(intVal)) {
return this.setValue(this.options.value);
return this._setValue(this.options.value, shouldSetLastValue);
}

//conform
Expand All @@ -290,6 +294,10 @@
//display number
this.setDisplayValue(val);

if (shouldSetLastValue) {
this.lastValue = val;
}

return this;
},

Expand Down
15 changes: 15 additions & 0 deletions test/spinbox-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,21 @@ define( function ( require ) {

} );

QUnit.test('spinbox should trigger change after using setValue', function (assert) {
var $spinbox = $(html).find('#MySpinboxDecimal').spinbox({
value: '1'
});

$spinbox.spinbox('setValue', '2');

$spinbox.on('changed.fu.spinbox', function () {
assert.ok(true, 'spinbox triggers changed after input' );
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably not even getting validated. You need to use assert.async in here and call it to ensure this assertion gets asserted. This is an async test, so you need to tell QUnit it is async.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't asynchronous. The handler should be called synchronously on the focusout. I can make it asynchronous, but then the failure would happen because of a timeout rather than a 0-assertion test. I prefer the latter because it means the tests run more quickly, but I can change it if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is fine for now but for future reference when using an event listener an async test is more appropriately because it is unknown what is happening before the event is triggered.

});

$spinbox.find('.spinbox-input').val(1);
$spinbox.find('.spinbox-input').focusout();
});

QUnit.test( "should destroy control", function( assert ) {
var $el = $( html ).find( "#MySpinbox" );

Expand Down