jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/349125 )
Change subject: NumberInputWidget: Remake as an actual TextInputWidget child
......................................................................
NumberInputWidget: Remake as an actual TextInputWidget child
As the name of it suggests, it should be a descendant of InputWidget
and, more specifically, of TextInputWidget. This solves a bunch of
compatibility issues.
To demonstrate this, a demo was added with TagMultiselectWidget
using NumberInputWidget; in its previous version the NumberInputWidget
could not have been used inside the TagMultiselectWidget because it didn't
have the expected API of TextInputWidget.
Reasons to use TextInputWidget rather than InputWidget:
* Appearance/logic: NumberInputWidget has a textbox in it and basically
looks and feels just like a TextInputWidget except it is numbers-only
and has the up/down buttons. Appearance-wise, it makes sense to inherit
the styles of TextInputWidget, and override specifics.
* Properties: TextInputWidget uses the same type input as NumberInputWidget
which means that things like 'indicator' and 'icon' and 'placeholder'
can be seamlessly used and be consistent with the other TextInputWidget
children. For example, adding a "star" indicator to denote that the
widget is required (like we do in PHP) is seamless if we use
TextInputWidget, but if we use InputWidget, we'll require redoing the
IndicatorElement and IconElement logic.
Another example is 'read only' states, which refer to the $input prop
and 'setRequired' which also does the same. Using InputWidget would
mean reapplying these.
* Methods like 'setValidityFlag' change the attr of the $input, which is
also very useful, coming from TextInputWidget
* Label: InputWidget is not LabeledElement (perhaps it should be) while
TextInputWidget is.
* Events: TextInputWidget listens to relevant events from the input,
like 'change', keypress, focus and blur events on the input - all
of which make sense to use in the NumberInputWidget as well.
In general, the NumberInputWidget -- having its main functionality an
actual <input type="number"> that allows typing -- is similar enough to
TextInputWidget to inherit from it, rather than from the parent InputWidget.
There are a few configuration options that come from TextInputWidget and
either don't make sense for the NumberInputWidget or are redundant but
we can either cancel those out (override them in the $.extend( config...)
in the constructor) or discourage their use in general. Overall, though,
it seemed to make more sense to inherit the useful aspects of
TextInputWidget rather than re-implement them when inheriting from
InputWidget.
Bonus: Renamed the horribly named 'setIsInteger' to 'setAllowInteger'
(and the getter as well) and added backward compatibility (mostly
at James_F's behest)
Bug: T124856
Bug: T149644
Change-Id: I00164fcaf5092b60243d4e3cdc8992285cee33b4
---
M demos/pages/widgets.js
M src/styles/widgets/NumberInputWidget.less
M src/themes/apex/widgets.less
M src/themes/mediawiki/widgets.less
M src/widgets/NumberInputWidget.js
M tests/index.php
A tests/widgets/NumberInputWidget.test.js
7 files changed, 144 insertions(+), 82 deletions(-)
Approvals:
Bartosz Dziewoński: Looks good to me, approved
jenkins-bot: Verified
diff --git a/demos/pages/widgets.js b/demos/pages/widgets.js
index c139838..c5dc41f 100644
--- a/demos/pages/widgets.js
+++ b/demos/pages/widgets.js
@@ -1388,10 +1388,10 @@
new OO.ui.TagMultiselectWidget( {
allowArbitrary: true,
inputPosition: 'outline',
- inputWidget: new
OO.ui.SearchInputWidget()
+ inputWidget: new
OO.ui.NumberInputWidget()
} ),
{
- label: 'TagMultiselectWidget
(inputwidget: OO.ui.SearchInputWidget, inputPosition:outline)',
+ label: 'TagMultiselectWidget
(inputwidget: OO.ui.NumberInputWidget, inputPosition:outline)',
align: 'top'
}
),
diff --git a/src/styles/widgets/NumberInputWidget.less
b/src/styles/widgets/NumberInputWidget.less
index 428d891..ee4abca 100644
--- a/src/styles/widgets/NumberInputWidget.less
+++ b/src/styles/widgets/NumberInputWidget.less
@@ -6,12 +6,8 @@
&-buttoned {
.oo-ui-buttonWidget,
- .oo-ui-textInputWidget {
+ .oo-ui-inputWidget-input {
display: table-cell;
- height: 100%;
- }
-
- .oo-ui-textInputWidget input {
height: 100%;
}
}
diff --git a/src/themes/apex/widgets.less b/src/themes/apex/widgets.less
index 2d5a14f..dbb9a86 100644
--- a/src/themes/apex/widgets.less
+++ b/src/themes/apex/widgets.less
@@ -474,7 +474,7 @@
border-left-width: 0;
}
- &-buttoned .oo-ui-textInputWidget input {
+ &-buttoned .oo-ui-inputWidget-input {
border-radius: 0;
}
}
diff --git a/src/themes/mediawiki/widgets.less
b/src/themes/mediawiki/widgets.less
index fa5156c..651a3a2 100644
--- a/src/themes/mediawiki/widgets.less
+++ b/src/themes/mediawiki/widgets.less
@@ -814,7 +814,7 @@
padding-right: 0;
}
- .oo-ui-textInputWidget input {
+ .oo-ui-inputWidget-input {
border-radius: 0;
}
}
@@ -1830,11 +1830,13 @@
text-shadow: @text-shadow-disabled;
border-color: @border-color-disabled;
}
+
.oo-ui-iconElement-icon,
.oo-ui-indicatorElement-indicator {
opacity: @opacity-disabled;
}
- .oo-ui-labelElement-label {
+
+ > .oo-ui-labelElement-label {
color: @color-disabled;
text-shadow: @text-shadow-disabled;
}
diff --git a/src/widgets/NumberInputWidget.js b/src/widgets/NumberInputWidget.js
index ac8e6fd..e320989 100644
--- a/src/widgets/NumberInputWidget.js
+++ b/src/widgets/NumberInputWidget.js
@@ -14,14 +14,13 @@
* $( 'body' ).append( numberInput.$element );
*
* @class
- * @extends OO.ui.Widget
+ * @extends OO.ui.TextInputWidget
*
* @constructor
* @param {Object} [config] Configuration options
- * @cfg {Object} [input] Configuration options to pass to the {@link
OO.ui.TextInputWidget text input widget}.
* @cfg {Object} [minusButton] Configuration options to pass to the {@link
OO.ui.ButtonWidget decrementing button widget}.
* @cfg {Object} [plusButton] Configuration options to pass to the {@link
OO.ui.ButtonWidget incrementing button widget}.
- * @cfg {boolean} [isInteger=false] Whether the field accepts only integer
values.
+ * @cfg {boolean} [allowInteger=false] Whether the field accepts only integer
values.
* @cfg {number} [min=-Infinity] Minimum allowed value
* @cfg {number} [max=Infinity] Maximum allowed value
* @cfg {number} [step=1] Delta when using the buttons or up/down arrow keys
@@ -29,6 +28,9 @@
* @cfg {boolean} [showButtons=true] Whether to show the plus and minus
buttons.
*/
OO.ui.NumberInputWidget = function OoUiNumberInputWidget( config ) {
+ var $field = $( '<div>' )
+ .addClass( 'oo-ui-numberInputWidget-field' );
+
// Configuration initialization
config = $.extend( {
isInteger: false,
@@ -39,17 +41,15 @@
showButtons: true
}, config );
- // Parent constructor
- OO.ui.NumberInputWidget.parent.call( this, config );
+ // For backward compatibility
+ $.extend( config, config.input );
+ this.input = this;
- // Properties
- this.input = new OO.ui.TextInputWidget( $.extend(
- {
- disabled: this.isDisabled(),
- type: 'number'
- },
- config.input
- ) );
+ // Parent constructor
+ OO.ui.NumberInputWidget.parent.call( this, $.extend( config, {
+ type: 'number'
+ } ) );
+
if ( config.showButtons ) {
this.minusButton = new OO.ui.ButtonWidget( $.extend(
{
@@ -72,11 +72,7 @@
}
// Events
- this.input.connect( this, {
- change: this.emit.bind( this, 'change' ),
- enter: this.emit.bind( this, 'enter' )
- } );
- this.input.$input.on( {
+ this.$input.on( {
keydown: this.onKeyDown.bind( this ),
'wheel mousewheel DOMMouseScroll': this.onWheel.bind( this )
} );
@@ -89,40 +85,31 @@
} );
}
- // Initialization
- this.setIsInteger( !!config.isInteger );
- this.setRange( config.min, config.max );
- this.setStep( config.step, config.pageStep );
-
- this.$field = $( '<div>' ).addClass( 'oo-ui-numberInputWidget-field' )
- .append( this.input.$element );
- this.$element.addClass( 'oo-ui-numberInputWidget' ).append( this.$field
);
+ // Build the field
+ $field.append( this.$input );
if ( config.showButtons ) {
- this.$field
+ $field
.prepend( this.minusButton.$element )
.append( this.plusButton.$element );
- this.$element.addClass( 'oo-ui-numberInputWidget-buttoned' );
}
- this.input.setValidation( this.validateNumber.bind( this ) );
+
+ // Initialization
+ this.setAllowInteger( config.isInteger || config.allowInteger );
+ this.setRange( config.min, config.max );
+ this.setStep( config.step, config.pageStep );
+ // Set the validation method after we set isInteger and range
+ // so that it doesn't immediately call setValidityFlag
+ this.setValidation( this.validateNumber.bind( this ) );
+
+ this.$element
+ .addClass( 'oo-ui-numberInputWidget' )
+ .toggleClass( 'oo-ui-numberInputWidget-buttoned',
config.showButtons )
+ .append( $field );
};
/* Setup */
-OO.inheritClass( OO.ui.NumberInputWidget, OO.ui.Widget );
-
-/* Events */
-
-/**
- * A `change` event is emitted when the value of the input changes.
- *
- * @event change
- */
-
-/**
- * An `enter` event is emitted when the user presses 'enter' inside the text
box.
- *
- * @event enter
- */
+OO.inheritClass( OO.ui.NumberInputWidget, OO.ui.TextInputWidget );
/* Methods */
@@ -131,19 +118,23 @@
*
* @param {boolean} flag
*/
-OO.ui.NumberInputWidget.prototype.setIsInteger = function ( flag ) {
+OO.ui.NumberInputWidget.prototype.setAllowInteger = function ( flag ) {
this.isInteger = !!flag;
- this.input.setValidityFlag();
+ this.setValidityFlag();
};
+// Backward compatibility
+OO.ui.NumberInputWidget.prototype.setIsInteger =
OO.ui.NumberInputWidget.prototype.setAllowInteger;
/**
* Get whether only integers are allowed
*
* @return {boolean} Flag value
*/
-OO.ui.NumberInputWidget.prototype.getIsInteger = function () {
+OO.ui.NumberInputWidget.prototype.getAllowInteger = function () {
return this.isInteger;
};
+// Backward compatibility
+OO.ui.NumberInputWidget.prototype.getIsInteger =
OO.ui.NumberInputWidget.prototype.getAllowInteger;
/**
* Set the range of allowed values
@@ -157,7 +148,7 @@
}
this.min = min;
this.max = max;
- this.input.setValidityFlag();
+ this.setValidityFlag();
};
/**
@@ -198,30 +189,12 @@
};
/**
- * Get the current value of the widget
- *
- * @return {string}
- */
-OO.ui.NumberInputWidget.prototype.getValue = function () {
- return this.input.getValue();
-};
-
-/**
* Get the current value of the widget as a number
*
* @return {number} May be NaN, or an invalid number
*/
OO.ui.NumberInputWidget.prototype.getNumericValue = function () {
- return +this.input.getValue();
-};
-
-/**
- * Set the value of the widget
- *
- * @param {string} value Invalid values are allowed
- */
-OO.ui.NumberInputWidget.prototype.setValue = function ( value ) {
- this.input.setValue( value );
+ return +this.getValue();
};
/**
@@ -251,7 +224,6 @@
this.setValue( n );
}
};
-
/**
* Validate input
*
@@ -261,6 +233,10 @@
*/
OO.ui.NumberInputWidget.prototype.validateNumber = function ( value ) {
var n = +value;
+ if ( value === '' ) {
+ return !this.isRequired();
+ }
+
if ( isNaN( n ) || !isFinite( n ) ) {
return false;
}
@@ -295,7 +271,7 @@
OO.ui.NumberInputWidget.prototype.onWheel = function ( event ) {
var delta = 0;
- if ( !this.isDisabled() && this.input.$input.is( ':focus' ) ) {
+ if ( !this.isDisabled() && this.$input.is( ':focus' ) ) {
// Standard 'wheel' event
if ( event.originalEvent.deltaMode !== undefined ) {
this.sawWheelEvent = true;
@@ -360,9 +336,6 @@
// Parent method
OO.ui.NumberInputWidget.parent.prototype.setDisabled.call( this,
disabled );
- if ( this.input ) {
- this.input.setDisabled( this.isDisabled() );
- }
if ( this.minusButton ) {
this.minusButton.setDisabled( this.isDisabled() );
}
diff --git a/tests/index.php b/tests/index.php
index 650e684..f8a4b7f 100644
--- a/tests/index.php
+++ b/tests/index.php
@@ -40,6 +40,7 @@
<script src="./mixins/FlaggedElement.test.js"></script>
<script src="./widgets/TagMultiselectWidget.test.js"></script>
<script src="./widgets/MenuTagMultiselectWidget.test.js"></script>
+ <script src="./widgets/NumberInputWidget.test.js"></script>
<!-- JS/PHP comparison tests -->
<script>OO.ui.JSPHPTestSuite = <?php echo $testSuiteJSON; ?></script>
<script src="./JSPHP.test.standalone.js"></script>
diff --git a/tests/widgets/NumberInputWidget.test.js
b/tests/widgets/NumberInputWidget.test.js
new file mode 100644
index 0000000..21abf64
--- /dev/null
+++ b/tests/widgets/NumberInputWidget.test.js
@@ -0,0 +1,90 @@
+( function () {
+ QUnit.module( 'NumberInputWidget' );
+
+ QUnit.test( 'validate number', 7, function ( assert ) {
+ var widget = new OO.ui.NumberInputWidget( {
+ allowInteger: true,
+ min: -10,
+ max: 10,
+ step: 1,
+ required: false
+ } );
+
+ assert.ok(
+ widget.validateNumber( 0 ),
+ 'Zero is valid as an integer.'
+ );
+
+ assert.ok(
+ widget.validateNumber( 5 ),
+ 'Integer within range is valid.'
+ );
+ assert.ok(
+ !widget.validateNumber( 2.5 ),
+ 'Non-integer within range is invalid when
allowInteger:true.'
+ );
+ assert.ok(
+ !widget.validateNumber( 11 ),
+ 'Integer larger than the range is invalid.'
+ );
+ assert.ok(
+ !widget.validateNumber( -11 ),
+ 'Integer smaller than the range is invalid.'
+ );
+
+ assert.ok(
+ widget.validateNumber( '' ),
+ 'Empty value is valid when required:false'
+ );
+
+ widget.setRequired( true );
+ assert.ok(
+ !widget.validateNumber( '' ),
+ 'Empty value is invalid when required:true'
+ );
+ } );
+
+ QUnit.test( 'adjust value', 4, function ( assert ) {
+ var widget = new OO.ui.NumberInputWidget( {
+ allowInteger: false,
+ min: -10,
+ max: 10,
+ step: 1,
+ required: false
+ } );
+
+ widget.adjustValue( 1 );
+ assert.equal(
+ widget.getValue(),
+ 1,
+ 'Adjusting value by 1 to an initial value (0) is 1'
+ );
+
+ widget.adjustValue( 0.5 );
+ assert.equal(
+ widget.getValue(),
+ 1.5,
+ 'Adjusting value by 0.5 outputs correct result'
+ );
+
+ widget.setAllowInteger( true );
+ widget.setValue( 1 );
+
+ widget.adjustValue( 0.5 );
+ assert.equal(
+ widget.getValue(),
+ 2,
+ 'Adjusting value by 0.5 for integer-only, rounds up the
increment'
+ );
+
+ widget.setValue( 1 );
+ widget.adjustValue( 1.3 );
+
+ assert.equal(
+ widget.getValue(),
+ 2,
+ 'Adjusting value by 0.3 for integer-only, rounds down
the increment'
+ );
+ } );
+
+}() );
--
To view, visit https://gerrit.wikimedia.org/r/349125
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I00164fcaf5092b60243d4e3cdc8992285cee33b4
Gerrit-PatchSet: 10
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
Gerrit-Reviewer: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Mooeypoo <[email protected]>
Gerrit-Reviewer: Prtksxna <[email protected]>
Gerrit-Reviewer: VolkerE <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
