Bartosz Dziewoński has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/191214

Change subject: [WIP] Don't use positional arguments to constructors that take 
'config' objects
......................................................................

[WIP] Don't use positional arguments to constructors that take 'config' objects

However, retain the ability to do so for backwards compatibility.

WIP because should probably update the demo. Not going to update all
callers.

Skipped:
* Toolbar-related things.
* LookupInputWidget because we're killing it.
* TextInputMenuSelectWidget because, while it doesn't accept an
  'input' config option, its parent MenuSelectWidget does, so this
  would technically be a breaking change? Messed up, not touching it
  now.

Bug: T89687
Change-Id: Ice897687ac02cd22a6e8c969b483667a4669a5c2
---
M bin/testsuitegenerator.rb
M php/layouts/FieldLayout.php
M php/layouts/GridLayout.php
M src/layouts/ActionFieldLayout.js
M src/layouts/FieldLayout.js
M src/layouts/GridLayout.js
M src/layouts/PageLayout.js
M src/widgets/OutlineControlsWidget.js
8 files changed, 85 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/oojs/ui refs/changes/14/191214/1

diff --git a/bin/testsuitegenerator.rb b/bin/testsuitegenerator.rb
index 089aaba..20e438a 100644
--- a/bin/testsuitegenerator.rb
+++ b/bin/testsuitegenerator.rb
@@ -19,7 +19,7 @@
                .reject{|c| !c[:parent] || c[:parent] == 'ElementMixin' || 
c[:parent] == 'Theme' } # can't test abstract
                .reject{|c| %w[Element Widget Layout Theme].include? c[:name] } 
# no toplevel
                .reject{|c| c[:name] == 'DropdownInputWidget' } # different PHP 
and JS implementations
-               .select{|c| c[:methods][0][:params].empty? } # only without 
params :(
+               .reject{|c| %w[FieldLayout GridLayout].include? c[:name] } # 
required config options cause issues
 
        # values to test for each type
        expandos = {
diff --git a/php/layouts/FieldLayout.php b/php/layouts/FieldLayout.php
index b3ae9c8..68b8568 100644
--- a/php/layouts/FieldLayout.php
+++ b/php/layouts/FieldLayout.php
@@ -34,14 +34,20 @@
        private $field, $body, $help;
 
        /**
-        * @param Widget $fieldWidget Field widget
+        * For backwards compatibility, the `fieldWidget` configuration option 
can also be passed as the
+        * first positional parameter.
+        *
         * @param array $config Configuration options
+        * @param Widget $config['fieldWidget'] Field widget
         * @param string $config['align'] Alignment mode, either 'left', 
'right', 'top' or 'inline'
         *   (default: 'left')
         * @param string $config['help'] Explanatory text shown as a '?' icon.
         */
-       public function __construct( Widget $fieldWidget, array $config = 
array() ) {
-               $hasInputWidget = $fieldWidget instanceof InputWidget;
+       public function __construct( $config = array() ) {
+               if ( !is_array( $config ) ) {
+                       $config = func_get_arg( 1 ) ?: array();
+                       $config['fieldWidget'] = func_get_arg( 0 );
+               }
 
                // Config initialization
                $config = array_merge( array( 'align' => 'left' ), $config );
@@ -50,7 +56,8 @@
                parent::__construct( $config );
 
                // Properties
-               $this->fieldWidget = $fieldWidget;
+               $this->fieldWidget = $config['fieldWidget'];
+               $hasInputWidget = $this->fieldWidget instanceof InputWidget;
                $this->field = new Tag( 'div' );
                $this->body = new Tag( $hasInputWidget ? 'label' : 'div' );
                if ( isset( $config['help'] ) ) {
diff --git a/php/layouts/GridLayout.php b/php/layouts/GridLayout.php
index 0730766..45c71c2 100644
--- a/php/layouts/GridLayout.php
+++ b/php/layouts/GridLayout.php
@@ -33,18 +33,26 @@
        protected $heights = array();
 
        /**
-        * @param PanelLayout[] $panels Panels in the grid
+        * For backwards compatibility, the `panels` configuration option can 
also be passed as the
+        * first positional parameter.
+        *
         * @param array $config Configuration options
+        * @param PanelLayout[] $config['panels'] Panels in the grid
         * @param number[] $config['widths'] Widths of columns as ratios
         * @param number[] $config['heights'] Heights of rows as ratios
         */
-       public function __construct( array $panels, array $config = array() ) {
+       public function __construct( array $config = array() ) {
+               if ( isset( $config[0] ) ) {
+                       $config = func_get_arg( 1 ) ?: array();
+                       $config['panels'] = func_get_arg( 0 );
+               }
+
                // Parent constructor
                parent::__construct( $config );
 
                // Initialization
                $this->addClasses( array( 'oo-ui-gridLayout' ) );
-               foreach ( $panels as $panel ) {
+               foreach ( $config['panels'] as $panel ) {
                        $this->panels[] = $panel;
                        $this->appendContent( $panel );
                }
diff --git a/src/layouts/ActionFieldLayout.js b/src/layouts/ActionFieldLayout.js
index 7a0500d..e8f8917 100644
--- a/src/layouts/ActionFieldLayout.js
+++ b/src/layouts/ActionFieldLayout.js
@@ -5,25 +5,34 @@
  * @extends OO.ui.FieldLayout
  *
  * @constructor
- * @param {OO.ui.Widget} fieldWidget Field widget
- * @param {OO.ui.ButtonWidget} buttonWidget Button widget
+ * For backwards compatibility, the `fieldWidget` and `buttonWidget` 
configuration options can also
+ * be passed as the first and second positional parameters.
+ *
  * @param {Object} [config] Configuration options
+ * @cfg {OO.ui.Widget} fieldWidget Field widget
+ * @cfg {OO.ui.ButtonWidget} buttonWidget Button widget
  * @cfg {string} [align='left'] Alignment mode, either 'left', 'right', 'top' 
or 'inline'
  * @cfg {string} [help] Explanatory text shown as a '?' icon.
  */
-OO.ui.ActionFieldLayout = function OoUiActionFieldLayout( fieldWidget, 
buttonWidget, config ) {
+OO.ui.ActionFieldLayout = function OoUiActionFieldLayout( config ) {
+       if ( !OO.isPlainObject( config ) ) {
+               config = arguments[ 2 ] || {};
+               config.fieldWidget = arguments[ 0 ];
+               config.buttonWidget = arguments[ 1 ];
+       }
+
        // Configuration initialization
        config = $.extend( { align: 'left' }, config );
 
        // Parent constructor
-       OO.ui.ActionFieldLayout.super.call( this, fieldWidget, config );
+       OO.ui.ActionFieldLayout.super.call( this, config );
 
        // Mixin constructors
        OO.ui.LabelElement.call( this, config );
 
        // Properties
-       this.fieldWidget = fieldWidget;
-       this.buttonWidget = buttonWidget;
+       this.fieldWidget = config.fieldWidget;
+       this.buttonWidget = config.buttonWidget;
        this.$button = $( '<div>' )
                .addClass( 'oo-ui-actionFieldLayout-button' )
                .append( this.buttonWidget.$element );
diff --git a/src/layouts/FieldLayout.js b/src/layouts/FieldLayout.js
index 8a81e8d..e9b1e55 100644
--- a/src/layouts/FieldLayout.js
+++ b/src/layouts/FieldLayout.js
@@ -16,13 +16,19 @@
  * @mixins OO.ui.LabelElement
  *
  * @constructor
- * @param {OO.ui.Widget} fieldWidget Field widget
+ * For backwards compatibility, the `fieldWidget` configuration option can 
also be passed as the
+ * first positional parameter.
+ *
  * @param {Object} [config] Configuration options
+ * @cfg {OO.ui.Widget} fieldWidget Field widget
  * @cfg {string} [align='left'] Alignment mode, either 'left', 'right', 'top' 
or 'inline'
  * @cfg {string} [help] Explanatory text shown as a '?' icon.
  */
-OO.ui.FieldLayout = function OoUiFieldLayout( fieldWidget, config ) {
-       var hasInputWidget = fieldWidget instanceof OO.ui.InputWidget;
+OO.ui.FieldLayout = function OoUiFieldLayout( config ) {
+       if ( !OO.isPlainObject( config ) ) {
+               config = arguments[ 1 ] || {};
+               config.fieldWidget = arguments[ 0 ];
+       }
 
        // Configuration initialization
        config = $.extend( { align: 'left' }, config );
@@ -34,7 +40,8 @@
        OO.ui.LabelElement.call( this, config );
 
        // Properties
-       this.fieldWidget = fieldWidget;
+       this.fieldWidget = config.fieldWidget;
+       var hasInputWidget = this.fieldWidget instanceof OO.ui.InputWidget;
        this.$field = $( '<div>' );
        this.$body = $( '<' + ( hasInputWidget ? 'label' : 'div' ) + '>' );
        this.align = null;
diff --git a/src/layouts/GridLayout.js b/src/layouts/GridLayout.js
index ecffe48..cafa718 100644
--- a/src/layouts/GridLayout.js
+++ b/src/layouts/GridLayout.js
@@ -6,12 +6,20 @@
  * @deprecated Use OO.ui.MenuLayout or plain CSS instead.
  *
  * @constructor
- * @param {OO.ui.PanelLayout[]} panels Panels in the grid
+ * For backwards compatibility, the `panels` configuration option can also be 
passed as the
+ * first positional parameter.
+ *
  * @param {Object} [config] Configuration options
+ * @cfg {OO.ui.PanelLayout[]} panels Panels in the grid
  * @cfg {number[]} [widths] Widths of columns as ratios
  * @cfg {number[]} [heights] Heights of rows as ratios
  */
-OO.ui.GridLayout = function OoUiGridLayout( panels, config ) {
+OO.ui.GridLayout = function OoUiGridLayout( config ) {
+       if ( !OO.isPlainObject( config ) ) {
+               config = arguments[ 1 ] || {};
+               config.panels = arguments[ 0 ];
+       }
+
        var i, len, widths;
 
        // Configuration initialization
@@ -27,9 +35,9 @@
 
        // Initialization
        this.$element.addClass( 'oo-ui-gridLayout' );
-       for ( i = 0, len = panels.length; i < len; i++ ) {
-               this.panels.push( panels[ i ] );
-               this.$element.append( panels[ i ].$element );
+       for ( i = 0, len = config.panels.length; i < len; i++ ) {
+               this.panels.push( config.panels[ i ] );
+               this.$element.append( config.panels[ i ].$element );
        }
        if ( config.widths || config.heights ) {
                this.layout( config.widths || [ 1 ], config.heights || [ 1 ] );
diff --git a/src/layouts/PageLayout.js b/src/layouts/PageLayout.js
index 67cd11b..721ed18 100644
--- a/src/layouts/PageLayout.js
+++ b/src/layouts/PageLayout.js
@@ -5,10 +5,18 @@
  * @extends OO.ui.PanelLayout
  *
  * @constructor
- * @param {string} name Unique symbolic name of page
+ * For backwards compatibility, the `name` configuration option can also be 
passed as the
+ * first positional parameter.
+ *
  * @param {Object} [config] Configuration options
+ * @cfg {string} name Unique symbolic name of page
  */
-OO.ui.PageLayout = function OoUiPageLayout( name, config ) {
+OO.ui.PageLayout = function OoUiPageLayout( config ) {
+       if ( !OO.isPlainObject( config ) ) {
+               config = arguments[ 1 ] || {};
+               config.name = arguments[ 0 ];
+       }
+
        // Configuration initialization
        config = $.extend( { scrollable: true }, config );
 
@@ -16,7 +24,7 @@
        OO.ui.PageLayout.super.call( this, config );
 
        // Properties
-       this.name = name;
+       this.name = config.name;
        this.outlineItem = null;
        this.active = false;
 
diff --git a/src/widgets/OutlineControlsWidget.js 
b/src/widgets/OutlineControlsWidget.js
index aa3939f..4428930 100644
--- a/src/widgets/OutlineControlsWidget.js
+++ b/src/widgets/OutlineControlsWidget.js
@@ -9,10 +9,18 @@
  * @mixins OO.ui.IconElement
  *
  * @constructor
- * @param {OO.ui.OutlineSelectWidget} outline Outline to control
+ * For backwards compatibility, the `outline` configuration option can also be 
passed as the
+ * first positional parameter.
+ *
  * @param {Object} [config] Configuration options
+ * @cfg {OO.ui.OutlineSelectWidget} outline Outline to control
  */
-OO.ui.OutlineControlsWidget = function OoUiOutlineControlsWidget( outline, 
config ) {
+OO.ui.OutlineControlsWidget = function OoUiOutlineControlsWidget( config ) {
+       if ( !OO.isPlainObject( config ) ) {
+               config = arguments[ 1 ] || {};
+               config.outline = arguments[ 0 ];
+       }
+
        // Configuration initialization
        config = $.extend( { icon: 'add' }, config );
 
@@ -24,7 +32,7 @@
        OO.ui.IconElement.call( this, config );
 
        // Properties
-       this.outline = outline;
+       this.outline = config.outline;
        this.$movers = $( '<div>' );
        this.upButton = new OO.ui.ButtonWidget( {
                framed: false,
@@ -43,7 +51,7 @@
        } );
 
        // Events
-       outline.connect( this, {
+       this.outline.connect( this, {
                select: 'onOutlineChange',
                add: 'onOutlineChange',
                remove: 'onOutlineChange'

-- 
To view, visit https://gerrit.wikimedia.org/r/191214
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ice897687ac02cd22a6e8c969b483667a4669a5c2
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to