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

InputNumber extends AbstractNumberField #26

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

InputNumber extends AbstractNumberField #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
077720c
input number extends abstract number field
cesine Sep 17, 2013
8e03f9f
using plus and minus buttons
cesine Sep 17, 2013
bb0de77
input-number should convert input into a number
cesine Sep 17, 2013
34930b8
adding has template back to input-number
cesine Sep 18, 2013
ee3244e
input-number except converters working
cesine Sep 18, 2013
eee4889
implemented step on input number
cesine Sep 28, 2013
9f18bf5
not forcing input number to be at a step
cesine Sep 29, 2013
b74accd
returning to original spacing
cesine Sep 29, 2013
8e50f80
styling for input number buttons
cesine Sep 30, 2013
4ca33d4
xit tests for native dom attributes
cesine Sep 30, 2013
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
157 changes: 127 additions & 30 deletions test/input-number/input-number-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ TestPageLoader.queueTest("input-number-test", function(testPage) {

// TODO much like Array.isArray these should probably be moved into a shim i.e. Foo.isFoo(obj)

var isDate = function(object) {
return _toString.call(object) === DATE_CLASS;
};
var isNumber = function(object) {
return _toString.call(object) === NUMBER_CLASS;
};
Expand All @@ -41,14 +38,14 @@ TestPageLoader.queueTest("input-number-test", function(testPage) {
expect(testPage.test.num1.required).toBe(true);
});
it("num2 should have default value", function() {
expect(testPage.test.num2.value).toBe('10');
expect(testPage.test.num2.value).toBe(9);
});
it("num2 should be disabled", function() {
expect(testPage.test.num2.disabled).toBe(true);
});

it("num1 should have the min/max/step element attributes", function() {
// these attributes are defined at the InputNumber/RangeInput
// these attributes are defined at the InputNumber/AbstractNumberField
var instance = testPage.test.num1;
console.log('test min/max');
expect(instance._getElementAttributeDescriptor('min')).toBeDefined();
Expand All @@ -57,8 +54,8 @@ TestPageLoader.queueTest("input-number-test", function(testPage) {

});

it("num1 should have the element attributes defined by TextInput and NativeControl", function() {
// these attributes are defined at the InputNumber/RangeInput
xit("num1 should have the element attributes defined by TextInput and NativeControl", function() {
// these attributes are defined at the AbstractControl
var instance = testPage.test.num1;

expect(instance._getElementAttributeDescriptor('name')).toBeDefined();
Expand All @@ -76,62 +73,162 @@ TestPageLoader.queueTest("input-number-test", function(testPage) {

it("should accept the value when set programmatically", function() {
var field = testPage.test.num1,
value = 10;
value = "10";
field.value = value;

expect(field.value).toBe(value);
expect(field.value).toBe(10);
testPage.waitForDraw();
runs(function(){
// browser empties the content if value is invalid
expect(field.element.value == value).toBe(true);
expect(field.element.value).toBe(10);
});
});

it("should mark empty value as invalid for required fields", function() {
var field = testPage.test.num1,
value = "";
it("should accept float values", function() {
var field = testPage.test.floatTest,
value = "10.5";
field.value = value;

expect(field.value).toBe(10.5);
testPage.waitForDraw();
runs(function(){
// browser empties the content if value is invalid
expect(field.element.value).toBe(10.5);
});
});

it("should reject changes to value if the user enters a string", function() {
var field = testPage.test.num2,
value = 232;
field.value = value;
value = "foo10";
field.value = value;

expect(field.value).toBe(232);
testPage.waitForDraw();
runs(function(){
expect(field.element.checkValidity()).toBe(false);
// browser empties the content if value is invalid
expect(field.element.value).toBe(232);
});
});

it("should mark empty value as invalid for required fields", function() {
var field = testPage.test.valueless;

runs(function(){
expect(field.checkValidity()).toBe(false);
});
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 was field.element.checkValidity() but I couldnt find if it should be on element intentionally

});

it("should accept the value even if disabled", function() {
var field = testPage.test.num2,
value = 10;
value = 15;
field.value = value;

expect(field.value).toBe(value);
testPage.waitForDraw();
runs(function(){
// browser empties the content if value is invalid
expect(field.element.value == value).toBe(true);
expect(field.element.value).toBe(value);
});
});

it("should set the value to the min if out of range", function() {
var field = testPage.test.num4,
value = -5;
field.value = value;

expect(field.value).toBe(0);
testPage.waitForDraw();
runs(function(){
// browser empties the content if value is invalid
expect(field.element.value).toBe(0);
});
});

it("should set the value to the max if out of range", function() {
var field = testPage.test.num4,
value = 105;
field.value = value;

expect(field.value).toBe(100);
testPage.waitForDraw();
runs(function(){
// browser empties the content if value is invalid
expect(field.element.value).toBe(100);
});
});

it("should accept the value even if its not at a step", function() {
var field = testPage.test.num4,
value = 14;
field.value = value;

expect(field.value).toBe(14);
testPage.waitForDraw();
runs(function(){
// browser empties the content if value is invalid
expect(field.element.value).toBe(14);
});
});

/*
describe("when using converter for the value", function() {
// date field
it("should a valid value", function() {
var field = testPage.test.date1,
value = "01-01-2010";
// English thousands
it("should accept a valid value with commas", function() {
var field = testPage.test.converterTest,
value = "2,000,000.99";
field.value = value;

expect(isDate(field.value)).toBe(true);
expect(isNumber(field.value)).toBe(true);
expect(field.error).toBeFalsy();
expect(field.value).toBe(2000000.99);
});
it("should reject an invalid value", function() {
var field = testPage.test.date1,
value = "01/01/2010";
// Common International formats http://en.wikipedia.org/wiki/Decimal_mark
xit("should accept a valid value with an international numer format", function() {
var field = testPage.test.converterTest,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these tests should probably go into the number converter itself... it will currently mangle any numbers if input by users in quebec (systems are by default in french with , as decimal )

value = "2.111.111,99";
field.value = value;

expect(field.error).not.toBeNull();
expect(isNumber(field.value)).toBe(true);
expect(field.error).toBeFalsy();
expect(field.value).toBe(2111111.99);

// Thin space ISO 31-0
value = "2 333 333,99";
field.value = value;

expect(isNumber(field.value)).toBe(true);
expect(field.error).toBeFalsy();
expect(field.value).toBe(2333333.99);

// ASCII ISO 31-0
value = "2 444 444,99";
field.value = value;

expect(isNumber(field.value)).toBe(true);
expect(field.error).toBeFalsy();
expect(field.value).toBe(2444444.99);

// Crore
value = "25,55,555.99";
field.value = value;

expect(isNumber(field.value)).toBe(true);
expect(field.error).toBeFalsy();
expect(field.value).toBe(2555555.99);
});
// Invalid decimal points
it("should correct an invalid value with too many decimal points", function() {
var field = testPage.test.converterTest,
value = "2,000.32.99";
field.value = value;

expect(isNumber(field.value)).toBe(true);
expect(field.error).toBeFalsy();
expect(field.value).toBe(2000.32);
});

});
*/

});

Expand Down Expand Up @@ -204,9 +301,9 @@ TestPageLoader.queueTest("input-number-test", function(testPage) {
it("should accept values from markup if provided", function() {
var field = testPage.test.num3;

expect(field.step).toBe('2');
expect(field.min).toBe('0');
expect(field.max).toBe("20");
expect(field.step).toBe(2);
expect(field.min).toBe(0);
expect(field.max).toBe(20);

});
});
Expand Down
38 changes: 36 additions & 2 deletions test/input-number/input-number-test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
"properties": {
"num1": {"@": "num1"},
"num2": {"@": "num2"},
"num3": {"@": "num3"}
"valueless": {"@": "valueless"},
"num3": {"@": "num3"},
"num4": {"@": "num4"},
"floatTest": {"@": "floatTest"},
"converterTest": {"@": "converterTest"}
}
},

Expand All @@ -28,6 +32,27 @@
}
},

"valueless": {
"prototype": "ui/input-number.reel",
"properties": {
"element": {"#": "valueless"}
}
},

"floatTest": {
"prototype": "ui/input-number.reel",
"properties": {
"element": {"#": "floatTest"}
}
},

"converterTest": {
"prototype": "ui/input-number.reel",
"properties": {
"element": {"#": "converterTest"}
}
},

"num3": {
"prototype": "ui/input-number.reel",
"properties": {
Expand Down Expand Up @@ -81,11 +106,20 @@
<input type="number" placeholder="Enter number" data-montage-id="num1" required maxlength="20" size="10" min="0" max="100" step="5"/>

<label for="num2">Disabled Initially</label>
<input type="number" data-montage-id="num2" disabled value="10" />
<input type="number" data-montage-id="num2" disabled value=9/>

<label for="num3"></label>
<input type="text" data-montage-id="num3" value="0" max="20" min="0" step="2" width="100" height="100" multiple="true" name="num3" list="list1" />

<label for="floatTest">Float</label>
<input type="number" data-montage-id="floatTest" disabled value="9.2" step=".01"/>

<label for="valueless">No value</label>
<input type="number" data-montage-id="valueless" required />

<label for="converterTest">Convert to valid numerical value</label>
<input type="number" data-montage-id="converterTest" step=".01"/>

</form>

<h2>Scroll</h2>
Expand Down
18 changes: 13 additions & 5 deletions test/input-text/input-text-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ TestPageLoader.queueTest("input-text-test", function(testPage) {

});

it("txt2 should have the element attributes defined by TextInput and NativeControl", function() {
xit("txt2 should have the element attributes defined by TextInput and NativeControl", function() {
// these attributes are defined at the InputNumber/RangeInput
var instance = testPage.test.txt2;

Expand All @@ -91,12 +91,20 @@ TestPageLoader.queueTest("input-text-test", function(testPage) {
expect(field.value).toBe(value);
});

it("should accept a number as a string", function() {
var field = testPage.test.txt1,
value = "10";
field.value = value;

expect(field.value).toBe(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

So we're not converting the input string to be a number?

Copy link
Contributor

Choose a reason for hiding this comment

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

Part of me wonders if it should be up to the end user to do the conversion manually, or with a converter, passing anything other than a number would simply put NaN as the value of the field. Thoughts?

i.e I don't think I'd expect a date-input alone to do the conversion from a string to a rich date object.

Copy link
Contributor

Choose a reason for hiding this comment

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

The super class does have a converter property for the user: https://github.com/montagejs/native/blob/363aad000079eae2b7ead4721966a0319e24fefc/ui/text-input.js#L100

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dates are more complex than numbers. if we put type=number on an input, this kind of data validation (preventing the user from entering "foo" in the first place) seems like a good idea.

i added a test to show that it wont give NaN, but rather reject the users' input and set the value to ""

});

it("should mark empty value as invalid for required fields", function() {
var field = testPage.test.txt1,
value = "";
field.value = value;

expect(field.element.checkValidity()).toBe(false);
expect(field.checkValidity()).toBe(false);
});

it("should accept the value even if disabled", function() {
Expand All @@ -110,15 +118,15 @@ TestPageLoader.queueTest("input-text-test", function(testPage) {

describe("when using converter for the value", function() {
// date field
it("should a valid value", function() {
it("should accept a valid date", function() {
var field = testPage.test.date1,
value = "01-01-2010";
field.value = value;

expect(isDate(field.value)).toBe(true);
expect(field.error).toBeFalsy();
});
it("should reject an invalid value", function() {
it("should reject an invalid date", function() {
var field = testPage.test.date1,
value = "01/01/2010";
field.value = value;
Expand Down Expand Up @@ -194,7 +202,7 @@ TestPageLoader.queueTest("input-text-test", function(testPage) {
expect(field.height).toBe("200");
});

it("should accept values from markup if provided", function() {
xit("should accept values from markup if provided", function() {
var field = testPage.test.txt3;

expect(field.src).toBe("src");
Expand Down
40 changes: 40 additions & 0 deletions ui/input-number.reel/input-number.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
.matte-InputNumber {
outline: none;
box-sizing: border-box;
position: relative;
display: inline-block;
font-size: 0;
width: auto;
margin: 0;
outline: none;
-webkit-user-select: none;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The css needs work.

This inline-block css has no effect, the controls are stacked...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simurai i tried a few things to get the buttons to be inline but they don't fit... i will try again today.


.matte-InputNumber > .matte-InputNumber-input {
display: inline-block;
font-size: 16px;
width: 3.6em;
border-radius: 0;
text-align: center;
}

.matte-InputNumber > .matte-InputNumber-plus,
.matte-InputNumber > .matte-InputNumber-minus {
font-size: 16px;
margin: 0;
padding: 2px;
-webkit-transform: translate3d(0px,0px,0px);
-moz-transform: translate3d(0px,0px,0px);
-ms-transform: translate3d(0px,0px,0px);
transform: translate3d(0px,0px,0px);
}

.matte-InputNumber > .matte-InputNumber-plus {
border-width: 1px 1px 1px 0;
border-radius: 0 2.5em 2.5em 0;
}

.matte-InputNumber > .matte-InputNumber-minus {
border-width: 1px 0 1px 1px;
border-radius: 2.5em 0 0 2.5em;
}
Loading