From daa92bb06e26d2c05d76f1b5926fcf853a9fb892 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 9 May 2014 10:44:34 +0200 Subject: [PATCH] mediawiki.htmlform: Refactor and clean up * Use .filter function instead of concatenating strings. The end result is the same. It's faster and more stable and semantically correct this way because using the string syntax is subject to bugs when using special characters (you'd have to escape it first, which is non-trivial to do in CSS selectors, sometimes impossible). So comparing the .name property against a string value directly is much easier. * Use .prop() instead of .attr() where appropiate. attr() changes attribute nodes only in the DOM, properties reflect the actual live rendered value. This works because of a back-compat layer in jQuery, but has been deprecated. * Use a simple array and create 1 jQuery object, instead of creating various temporary objects only to have jQuery merge them via .add(). .add() is relatively expensive in that it keeps a reference in pushStack and does a lot of logic. Fine for convenience, but since we have all the data right here, might as well take a more direct approach. * Remove ill-informed $() calls. This is already a jQuery object for the method is a jQuery method. Cloning the object first leads to unexpected side-effects and is simply not neccecary. * Remove use of .live() Change-Id: Icfe63a4111026661c53639b72e67a4d4fa3d6ac2 --- resources/src/mediawiki/mediawiki.htmlform.js | 77 ++++++++++++------- 1 file changed, 50 insertions(+), 27 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.htmlform.js b/resources/src/mediawiki/mediawiki.htmlform.js index f7aa7f8bd0..8be1321ea4 100644 --- a/resources/src/mediawiki/mediawiki.htmlform.js +++ b/resources/src/mediawiki/mediawiki.htmlform.js @@ -20,14 +20,18 @@ * @return {jQuery|null} */ function hideIfGetField( $el, name ) { - var sel, $found, $p; + var $found, $p, + suffix = name.replace( /^([^\[]+)/, '[$1]' ); + + function nameFilter() { + return this.name === name || + ( this.name === ( 'wp' + name ) ) || + this.name.slice( -suffix.length ) === suffix; + } - sel = '[name="' + name + '"],' + - '[name="wp' + name + '"],' + - '[name$="' + name.replace( /^([^\[]+)/, '[$1]' ) + '"]'; for ( $p = $el.parent(); $p.length > 0; $p = $p.parent() ) { - $found = $p.find( sel ); - if ( $found.length > 0 ) { + $found = $p.find( '[name]' ).filter( nameFilter ); + if ( $found.length ) { return $found; } } @@ -43,7 +47,7 @@ * @return {Array} 2 elements: jQuery of dependent fields, and test function */ function hideIfParse( $el, spec ) { - var op, i, l, v, $field, $fields, func, funcs, getVal; + var op, i, l, v, $field, $fields, fields, func, funcs, getVal; op = spec[0]; l = spec.length; @@ -53,15 +57,16 @@ case 'NAND': case 'NOR': funcs = []; - $fields = $(); + fields = []; for ( i = 1; i < l; i++ ) { if ( !$.isArray( spec[i] ) ) { throw new Error( op + ' parameters must be arrays' ); } v = hideIfParse( $el, spec[i] ); - $fields = $fields.add( v[0] ); + fields.push( v[0] ); funcs.push( v[1] ); } + $fields = $( fields ); l = funcs.length; switch ( op ) { @@ -143,12 +148,12 @@ } v = spec[2]; - if ( $field.first().attr( 'type' ) === 'radio' || - $field.first().attr( 'type' ) === 'checkbox' + if ( $field.first().prop( 'type' ) === 'radio' || + $field.first().prop( 'type' ) === 'checkbox' ) { getVal = function () { var $selected = $field.filter( ':checked' ); - return $selected.length > 0 ? $selected.val() : ''; + return $selected.length ? $selected.val() : ''; }; } else { getVal = function () { @@ -185,9 +190,9 @@ */ $.fn.goIn = function ( instantToggle ) { if ( instantToggle === true ) { - return $( this ).show(); + return this.show(); } - return $( this ).stop( true, true ).fadeIn(); + return this.stop( true, true ).fadeIn(); }; /** @@ -199,31 +204,41 @@ */ $.fn.goOut = function ( instantToggle ) { if ( instantToggle === true ) { - return $( this ).hide(); + return this.hide(); } - return $( this ).stop( true, true ).fadeOut(); + return this.stop( true, true ).fadeOut(); }; /** * Bind a function to the jQuery object via live(), and also immediately trigger * the function on the objects with an 'instant' parameter set to true. + * + * @method liveAndTestAtStart + * @deprecated since 1.24 Use .on() and .each() directly. * @param {Function} callback * @param {boolean|jQuery.Event} callback.immediate True when the event is called immediately, * an event object when triggered from an event. + * @return jQuery + * @chainable */ - $.fn.liveAndTestAtStart = function ( callback ) { - $( this ) + mw.log.deprecate( $.fn, 'liveAndTestAtStart', function ( callback ) { + this + // Can't really migrate to .on() generically, needs knowledge of + // calling code to know the correct selector. Fix callers and + // get rid of this .liveAndTestAtStart() hack. .live( 'change', callback ) .each( function () { callback.call( this, true ); } ); - }; + } ); function enhance( $root ) { - // Animate the SelectOrOther fields, to only show the text field when - // 'other' is selected. - $root.find( '.mw-htmlform-select-or-other' ).liveAndTestAtStart( function ( instant ) { + /** + * @ignore + * @param {boolean|jQuery.Event} instant + */ + function handleSelectOrOther( instant ) { var $other = $root.find( '#' + $( this ).attr( 'id' ) + '-other' ); $other = $other.add( $other.siblings( 'br' ) ); if ( $( this ).val() === 'other' ) { @@ -231,13 +246,21 @@ } else { $other.goOut( instant ); } - } ); + } + + // Animate the SelectOrOther fields, to only show the text field when + // 'other' is selected. + $root + .on( 'change', '.mw-htmlform-select-or-other', handleSelectOrOther ) + .each( function () { + handleSelectOrOther.call( this, true ); + } ); // Set up hide-if elements $root.find( '.mw-htmlform-hide-if' ).each( function () { - var $el = $( this ), - spec = $el.data( 'hideIf' ), - v, $fields, test, func; + var v, $fields, test, func, + $el = $( this ), + spec = $el.data( 'hideIf' ); if ( !spec ) { return; @@ -253,7 +276,7 @@ $el.show(); } }; - $fields.change( func ); + $fields.on( 'change', func ); func(); } ); -- 2.20.1