jenkins-bot has submitted this change and it was merged.
Change subject: Allow widgets to reuse parts of the DOM when infusing, use it
for InputWidget's $input
......................................................................
Allow widgets to reuse parts of the DOM when infusing, use it for InputWidget's
$input
Reusing InputWidget's $input will improve interoperability with
browsers' form autofilling features and allow them to preserve
inputted values across page reloads (T114134).
Reusing parts of DOM in general is likely to improve performance,
although that is not the main use case, and little work is done here
towards that.
Bug: T114134
Bug: T114408
Change-Id: I3c21b3710d16dbb4dbcbdd3871a70fdfd0e7b536
---
M src/Element.js
M src/widgets/ButtonInputWidget.js
M src/widgets/CheckboxInputWidget.js
M src/widgets/DropdownInputWidget.js
M src/widgets/InputWidget.js
M src/widgets/RadioInputWidget.js
M src/widgets/TextInputWidget.js
7 files changed, 64 insertions(+), 33 deletions(-)
Approvals:
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/Element.js b/src/Element.js
index f1299fd..9f5e015 100644
--- a/src/Element.js
+++ b/src/Element.js
@@ -201,17 +201,25 @@
}
}
} );
+ // allow widgets to reuse parts of the DOM
+ data = cls.static.reusePreInfuseDOM( $elem[ 0 ], data );
// pick up dynamic state, like focus, value of form inputs, scroll
position, etc.
- state = cls.static.gatherPreInfuseState( $elem, data );
+ state = cls.static.gatherPreInfuseState( $elem[ 0 ], data );
+ // rebuild widget
// jscs:disable requireCapitalizedConstructors
- obj = new cls( data ); // rebuild widget
+ obj = new cls( data );
+ // jscs:enable requireCapitalizedConstructors
// now replace old DOM with this new DOM.
if ( top ) {
- $elem.replaceWith( obj.$element );
- // This element is now gone from the DOM, but if anyone is
holding a reference to it,
- // let's allow them to OO.ui.infuse() it and do what they
expect (T105828).
- // Do not use jQuery.data(), as using it on detached nodes
leaks memory in 1.x line by design.
- $elem[ 0 ].oouiInfused = obj;
+ // An efficient constructor might be able to reuse the entire
DOM tree of the original element,
+ // so only mutate the DOM if we need to.
+ if ( $elem[ 0 ] !== obj.$element[ 0 ] ) {
+ $elem.replaceWith( obj.$element );
+ // This element is now gone from the DOM, but if anyone
is holding a reference to it,
+ // let's allow them to OO.ui.infuse() it and do what
they expect (T105828).
+ // Do not use jQuery.data(), as using it on detached
nodes leaks memory in 1.x line by design.
+ $elem[ 0 ].oouiInfused = obj;
+ }
top.resolve();
}
obj.$element.data( 'ooui-infused', obj );
@@ -223,6 +231,22 @@
};
/**
+ * Pick out parts of `node`'s DOM to be reused when infusing a widget.
+ *
+ * This method **must not** make any changes to the DOM, only find interesting
pieces and add them
+ * to `config` (which should then be returned). Actual DOM juggling should
then be done by the
+ * constructor, which will be given the enhanced config.
+ *
+ * @protected
+ * @param {HTMLElement} node
+ * @param {Object} config
+ * @return {Object}
+ */
+OO.ui.Element.static.reusePreInfuseDOM = function ( node, config ) {
+ return config;
+};
+
+/**
* Gather the dynamic state (focus, value of form inputs, scroll position,
etc.) of a HTML DOM node
* (and its children) that represent an Element of the same class and the
given configuration,
* generated by the PHP implementation.
diff --git a/src/widgets/ButtonInputWidget.js b/src/widgets/ButtonInputWidget.js
index 06cfa3d..f7be83b 100644
--- a/src/widgets/ButtonInputWidget.js
+++ b/src/widgets/ButtonInputWidget.js
@@ -80,9 +80,12 @@
* @protected
*/
OO.ui.ButtonInputWidget.prototype.getInputElement = function ( config ) {
- var type = [ 'button', 'submit', 'reset' ].indexOf( config.type ) !==
-1 ?
- config.type :
- 'button';
+ var type;
+ // See InputWidget#reusePreInfuseDOM about config.$input
+ if ( config.$input ) {
+ return config.$input.empty();
+ }
+ type = [ 'button', 'submit', 'reset' ].indexOf( config.type ) !== -1 ?
config.type : 'button';
return $( '<' + ( config.useInputTag ? 'input' : 'button' ) + ' type="'
+ type + '">' );
};
diff --git a/src/widgets/CheckboxInputWidget.js
b/src/widgets/CheckboxInputWidget.js
index 853ef81..8540c47 100644
--- a/src/widgets/CheckboxInputWidget.js
+++ b/src/widgets/CheckboxInputWidget.js
@@ -64,11 +64,8 @@
* @inheritdoc
*/
OO.ui.CheckboxInputWidget.static.gatherPreInfuseState = function ( node,
config ) {
- var
- state =
OO.ui.CheckboxInputWidget.parent.static.gatherPreInfuseState( node, config ),
- $input = $( node ).find( '.oo-ui-inputWidget-input' );
- state.$input = $input; // shortcut for performance, used in InputWidget
- state.checked = $input.prop( 'checked' );
+ var state =
OO.ui.CheckboxInputWidget.parent.static.gatherPreInfuseState( node, config );
+ state.checked = config.$input.prop( 'checked' );
return state;
};
diff --git a/src/widgets/DropdownInputWidget.js
b/src/widgets/DropdownInputWidget.js
index 29c39f1..7891374 100644
--- a/src/widgets/DropdownInputWidget.js
+++ b/src/widgets/DropdownInputWidget.js
@@ -66,7 +66,11 @@
* @inheritdoc
* @protected
*/
-OO.ui.DropdownInputWidget.prototype.getInputElement = function () {
+OO.ui.DropdownInputWidget.prototype.getInputElement = function ( config ) {
+ // See InputWidget#reusePreInfuseDOM about config.$input
+ if ( config.$input ) {
+ return config.$input.addClass( 'oo-ui-element-hidden' );
+ }
return $( '<input type="hidden">' );
};
diff --git a/src/widgets/InputWidget.js b/src/widgets/InputWidget.js
index f7aa7e2..429790a 100644
--- a/src/widgets/InputWidget.js
+++ b/src/widgets/InputWidget.js
@@ -75,13 +75,21 @@
/**
* @inheritdoc
*/
+OO.ui.InputWidget.static.reusePreInfuseDOM = function ( node, config ) {
+ config = OO.ui.InputWidget.parent.static.reusePreInfuseDOM( node,
config );
+ // Reusing $input lets browsers preserve inputted values across page
reloads (T114134)
+ config.$input = $( node ).find( '.oo-ui-inputWidget-input' );
+ return config;
+};
+
+/**
+ * @inheritdoc
+ */
OO.ui.InputWidget.static.gatherPreInfuseState = function ( node, config ) {
- var
- state = OO.ui.InputWidget.parent.static.gatherPreInfuseState(
node, config ),
- $input = state.$input || $( node ).find(
'.oo-ui-inputWidget-input' );
- state.value = $input.val();
+ var state = OO.ui.InputWidget.parent.static.gatherPreInfuseState( node,
config );
+ state.value = config.$input.val();
// Might be better in TabIndexedElement, but it's awkward to do there
because mixins are awkward
- state.focus = $input.is( ':focus' );
+ state.focus = config.$input.is( ':focus' );
return state;
};
@@ -107,8 +115,9 @@
* @param {Object} config Configuration options
* @return {jQuery} Input element
*/
-OO.ui.InputWidget.prototype.getInputElement = function () {
- return $( '<input>' );
+OO.ui.InputWidget.prototype.getInputElement = function ( config ) {
+ // See #reusePreInfuseDOM about config.$input
+ return config.$input || $( '<input>' );
};
/**
diff --git a/src/widgets/RadioInputWidget.js b/src/widgets/RadioInputWidget.js
index 77498f4..485847f 100644
--- a/src/widgets/RadioInputWidget.js
+++ b/src/widgets/RadioInputWidget.js
@@ -64,11 +64,8 @@
* @inheritdoc
*/
OO.ui.RadioInputWidget.static.gatherPreInfuseState = function ( node, config )
{
- var
- state =
OO.ui.RadioInputWidget.parent.static.gatherPreInfuseState( node, config ),
- $input = $( node ).find( '.oo-ui-inputWidget-input' );
- state.$input = $input; // shortcut for performance, used in InputWidget
- state.checked = $input.prop( 'checked' );
+ var state = OO.ui.RadioInputWidget.parent.static.gatherPreInfuseState(
node, config );
+ state.checked = config.$input.prop( 'checked' );
return state;
};
diff --git a/src/widgets/TextInputWidget.js b/src/widgets/TextInputWidget.js
index 10f21d5..577cf8d 100644
--- a/src/widgets/TextInputWidget.js
+++ b/src/widgets/TextInputWidget.js
@@ -183,12 +183,9 @@
* @inheritdoc
*/
OO.ui.TextInputWidget.static.gatherPreInfuseState = function ( node, config ) {
- var
- state =
OO.ui.TextInputWidget.parent.static.gatherPreInfuseState( node, config ),
- $input = $( node ).find( '.oo-ui-inputWidget-input' );
- state.$input = $input; // shortcut for performance, used in InputWidget
+ var state = OO.ui.TextInputWidget.parent.static.gatherPreInfuseState(
node, config );
if ( config.multiline ) {
- state.scrollTop = $input.scrollTop();
+ state.scrollTop = config.$input.scrollTop();
}
return state;
};
--
To view, visit https://gerrit.wikimedia.org/r/243082
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3c21b3710d16dbb4dbcbdd3871a70fdfd0e7b536
Gerrit-PatchSet: 5
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
