Do not automatically infuse any OOjs UI widgets
authorBartosz Dziewoński <matma.rex@gmail.com>
Tue, 26 Jul 2016 12:12:21 +0000 (14:12 +0200)
committerBartosz Dziewoński <matma.rex@gmail.com>
Fri, 19 Aug 2016 01:29:31 +0000 (03:29 +0200)
This is not really what we had in mind when developing the infusion
feature and I think it's not helpful. Most of the time there is just
no benefit; a ButtonWidget generated in PHP and in JS behaves and
looks pretty much the same, and rebuilding it through infusion is a
small performance hit. If you're not adding any event handlers, it only
makes sense for various dropdowns, which have themed styling.

For the primary use case of adding JS behaviors to PHP widgets you
need to call OO.ui.infuse() anyway to get a reference to the JS
widget, and not infusing automatically should make it easier to reason
about your code. Infusion tries to be very transparent, but it can't
hide the fact that the DOM is re-built, making your references to DOM
nodes from before infusion useless and losing anything from PHP that
wasn't included in the config (e.g. custom attributes).

This commit removes automated infusion from mediawiki.page.ready
and adds some custom code in mediawiki.special.movePage and
mediawiki.htmlform. I see only two extensions using infusable OOjs UI
widgets in Gerrit (ArticlePlaceholder and ExtensionDistributor) and
neither should be affected by this change.

Change-Id: I56608c537fc57c5c54960b0603694f2612f45618

13 files changed:
RELEASE-NOTES-1.28
includes/htmlform/HTMLFormField.php
includes/htmlform/fields/HTMLComboboxField.php
includes/htmlform/fields/HTMLRadioField.php
includes/htmlform/fields/HTMLSelectField.php
includes/htmlform/fields/HTMLSelectNamespace.php
includes/htmlform/fields/HTMLTitleTextField.php
includes/htmlform/fields/HTMLUserTextField.php
includes/specials/SpecialMovepage.php
resources/Resources.php
resources/src/mediawiki.special/mediawiki.special.movePage.js
resources/src/mediawiki/htmlform/autoinfuse.js [new file with mode: 0644]
resources/src/mediawiki/page/ready.js

index f6c3530..ee5b394 100644 (file)
@@ -111,6 +111,9 @@ changes to languages because of Phabricator reports.
 * AuthenticationRequest::$required is now changed from REQUIRED to PRIMARY_REQUIRED
   on requests needed by primary providers even if all primaries need them.
   Primary providers are discouraged from returning multiple REQUIRED requests.
+* OOjs UI PHP widgets constructed with the `'infusable' => true` config option
+  will no longer be automatically infused. You should call `OO.ui.infuse()`
+  on them yourself from your JavaScript code.
 
 == Compatibility ==
 
index 5f6460d..b47bfa0 100644 (file)
@@ -626,6 +626,11 @@ abstract class HTMLFormField {
                        'infusable' => $infusable,
                ];
 
+               if ( $infusable && $this->shouldInfuseOOUI() ) {
+                       $this->mParent->getOutput()->addModules( 'oojs-ui-core' );
+                       $config['classes'][] = 'mw-htmlform-field-autoinfuse';
+               }
+
                // the element could specify, that the label doesn't need to be added
                $label = $this->getLabel();
                if ( $label ) {
@@ -655,6 +660,18 @@ abstract class HTMLFormField {
                return new OOUI\FieldLayout( $inputField, $config );
        }
 
+       /**
+        * Whether the field should be automatically infused. Note that all OOjs UI HTMLForm fields are
+        * infusable (you can call OO.ui.infuse() on them), but not all are infused by default, since
+        * there is no benefit in doing it e.g. for buttons and it's a small performance hit on page load.
+        *
+        * @return bool
+        */
+       protected function shouldInfuseOOUI() {
+               // Always infuse fields with help text, since the interface for it is nicer with JS
+               return $this->getHelpText() !== null;
+       }
+
        /**
         * Get the complete raw fields for the input, including help text,
         * labels, and whatever.
index 778aedb..0c3bc5a 100644 (file)
@@ -56,4 +56,8 @@ class HTMLComboboxField extends HTMLTextField {
                        'disabled' => $disabled,
                ] + $attribs );
        }
+
+       protected function shouldInfuseOOUI() {
+               return true;
+       }
 }
index e5b5e68..976befe 100644 (file)
@@ -57,6 +57,10 @@ class HTMLRadioField extends HTMLFormField {
                ) );
        }
 
+       protected function shouldInfuseOOUI() {
+               return true;
+       }
+
        function formatOptions( $options, $value ) {
                global $wgUseMediaWikiUIEverywhere;
 
index b6ad46c..40b31b5 100644 (file)
@@ -65,4 +65,8 @@ class HTMLSelectField extends HTMLFormField {
                        'disabled' => $disabled,
                ] + $attribs );
        }
+
+       protected function shouldInfuseOOUI() {
+               return true;
+       }
 }
index ef21969..ffa2500 100644 (file)
@@ -33,4 +33,8 @@ class HTMLSelectNamespace extends HTMLFormField {
                        'includeAllValue' => $this->mAllValue,
                ] );
        }
+
+       protected function shouldInfuseOOUI() {
+               return true;
+       }
 }
index fcf721a..5d5d765 100644 (file)
@@ -81,6 +81,10 @@ class HTMLTitleTextField extends HTMLTextField {
                return new TitleInputWidget( $params );
        }
 
+       protected function shouldInfuseOOUI() {
+               return true;
+       }
+
        public function getInputHtml( $value ) {
                // add mw-searchInput class to enable search suggestions for non-OOUI, too
                $this->mClass .= 'mw-searchInput';
index 5a7e0b9..f21b53d 100644 (file)
@@ -45,6 +45,10 @@ class HTMLUserTextField extends HTMLTextField {
                return new UserInputWidget( $params );
        }
 
+       protected function shouldInfuseOOUI() {
+               return true;
+       }
+
        public function getInputHtml( $value ) {
                // add the required module and css class for user suggestions in non-OOUI mode
                $this->mParent->getOutput()->addModules( 'mediawiki.userSuggest' );
index d0c44c3..9cc6745 100644 (file)
@@ -353,6 +353,7 @@ class MovePageForm extends UnlistedSpecialPage {
                                        'help' => new OOUI\HtmlSnippet( $this->msg( 'movepagetalktext' )->parseAsBlock() ),
                                        'align' => 'inline',
                                        'infusable' => true,
+                                       'id' => 'wpMovetalk-field',
                                ]
                        );
                }
index 1372a8e..3a5b002 100644 (file)
@@ -1071,6 +1071,7 @@ return [
                'scripts' => [
                        'resources/src/mediawiki/htmlform/htmlform.js',
                        'resources/src/mediawiki/htmlform/autocomplete.js',
+                       'resources/src/mediawiki/htmlform/autoinfuse.js',
                        'resources/src/mediawiki/htmlform/checkmatrix.js',
                        'resources/src/mediawiki/htmlform/cloner.js',
                        'resources/src/mediawiki/htmlform/hide-if.js',
index 6d88c51..9af81b8 100644 (file)
@@ -2,6 +2,10 @@
  * JavaScript for Special:MovePage
  */
 jQuery( function () {
+       // Infuse for pretty dropdown
        OO.ui.infuse( 'wpNewTitle' );
+       // Limit to 255 bytes, not characters
        OO.ui.infuse( 'wpReason' ).$input.byteLimit();
+       // Infuse for nicer "help" popup
+       OO.ui.infuse( 'wpMovetalk-field' );
 } );
diff --git a/resources/src/mediawiki/htmlform/autoinfuse.js b/resources/src/mediawiki/htmlform/autoinfuse.js
new file mode 100644 (file)
index 0000000..f77e367
--- /dev/null
@@ -0,0 +1,38 @@
+/*
+ * HTMLForm enhancements:
+ * Infuse some OOjs UI HTMLForm fields (those which benefit from always being infused).
+ */
+( function ( mw ) {
+
+       mw.hook( 'htmlform.enhance' ).add( function ( $root ) {
+               var $oouiNodes, modules;
+
+               $oouiNodes = $root.find( '.mw-htmlform-field-autoinfuse' );
+               if ( $oouiNodes.length ) {
+                       // The modules are preloaded (added server-side in HTMLFormField, and the individual fields
+                       // which need extra ones), but this module doesn't depend on them. Wait until they're loaded.
+                       modules = [ 'oojs-ui-core' ];
+                       if ( $oouiNodes.filter( '.mw-htmlform-field-HTMLTitleTextField' ).length ) {
+                               // FIXME: TitleInputWidget should be in its own module
+                               modules.push( 'mediawiki.widgets' );
+                       }
+                       if ( $oouiNodes.filter( '.mw-htmlform-field-HTMLUserTextField' ).length ) {
+                               modules.push( 'mediawiki.widgets.UserInputWidget' );
+                       }
+                       if (
+                               $oouiNodes.filter( '.mw-htmlform-field-HTMLSelectNamespace' ).length ||
+                               $oouiNodes.filter( '.mw-htmlform-field-HTMLSelectNamespaceWithButton' ).length
+                       ) {
+                               // FIXME: NamespaceInputWidget should be in its own module (probably?)
+                               modules.push( 'mediawiki.widgets' );
+                       }
+                       mw.loader.using( modules ).done( function () {
+                               $oouiNodes.each( function () {
+                                       OO.ui.infuse( this );
+                               } );
+                       } );
+               }
+
+       } );
+
+}( mediaWiki ) );
index 3b779d1..d228f3e 100644 (file)
@@ -36,7 +36,7 @@
 
        // Things outside the wikipage content
        $( function () {
-               var $nodes, $oouiNodes;
+               var $nodes;
 
                if ( !supportsPlaceholder ) {
                        // Exclude content to avoid hitting it twice for the (first) wikipage content
                // Add accesskey hints to the tooltips
                $( '[accesskey]' ).updateTooltipAccessKeys();
 
-               // Infuse OOUI widgets, if any are present
-               $oouiNodes = $( '[data-ooui]' );
-               if ( $oouiNodes.length ) {
-                       // FIXME: We should only load the widgets that are being infused
-                       mw.loader.using( [
-                               'mediawiki.widgets',
-                               'mediawiki.widgets.UserInputWidget',
-                               'mediawiki.widgets.SearchInputWidget'
-                       ] ).done( function () {
-                               $oouiNodes.each( function () {
-                                       OO.ui.infuse( this );
-                               } );
-                       } );
-               }
-
                $nodes = $( '.catlinks[data-mw="interface"]' );
                if ( $nodes.length ) {
                        /**