From bc0db4dcc35aa848b8bdf7d0144d867950ebff25 Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Mon, 6 Jun 2022 15:30:53 +0200 Subject: Apply ID and class more consistently in HTMLSelect(Or|And)OtherField There were several inconsistencies in how ID vs cssclass vs own classes were applied, both within the same widget + mode, between different modes for the same widget, and different widgets. So uniform all of them according to the plan detailed on phabricator. Include HTMLSelectAndOtherField as well (that wasn't mentioned on phab) for consistency. Also remove the select-or-other class from HTMLAutoCompleteSelectField (which was only applied in HTML mode), since the widget is not technically a SelectOrOther. Adjust the logic in selectorother.js so that it works fine in HTML mode, and disable it for OOUI mode since SelectWithInputWidget does that already (and the field is autoinfused). Also, in HTMLAutoCompleteSelectField, don't assign an array to $this->mClass. This happens to be working right now, but it violates the type definition of the property. Luckily, according to codesearch this change seems to be backwards-compatible because these fields are not commonly used, and nobody seems to be passing cssclass to them. Bug: T309883 Change-Id: If950f131d1c4bc1645de6021781045dccfc61966 --- .../fields/HTMLAutoCompleteSelectField.php | 8 ++--- .../htmlform/fields/HTMLSelectAndOtherField.php | 36 +++++++++++----------- .../htmlform/fields/HTMLSelectOrOtherField.php | 33 ++++++++++++-------- 3 files changed, 42 insertions(+), 35 deletions(-) (limited to 'includes/htmlform') diff --git a/includes/htmlform/fields/HTMLAutoCompleteSelectField.php b/includes/htmlform/fields/HTMLAutoCompleteSelectField.php index 57a06f021414..a2ac801cd0a3 100644 --- a/includes/htmlform/fields/HTMLAutoCompleteSelectField.php +++ b/includes/htmlform/fields/HTMLAutoCompleteSelectField.php @@ -125,7 +125,7 @@ class HTMLAutoCompleteSelectField extends HTMLTextField { public function getInputHTML( $value ) { $oldClass = $this->mClass; - $this->mClass = (array)$this->mClass; + $classes = (array)$this->mClass; $valInSelect = false; $ret = ''; @@ -141,7 +141,6 @@ class HTMLAutoCompleteSelectField extends HTMLTextField { $selected = $valInSelect ? $value : 'other'; $select = new XmlSelect( $this->mName . '-select', $this->mID . '-select', $selected ); $select->addOptions( $this->getOptions() ); - $select->setAttribute( 'class', 'mw-htmlform-select-or-other' ); if ( !empty( $this->mParams['disabled'] ) ) { $select->setAttribute( 'disabled', 'disabled' ); @@ -153,7 +152,7 @@ class HTMLAutoCompleteSelectField extends HTMLTextField { $ret = $select->getHTML() . "
\n"; - $this->mClass[] = 'mw-htmlform-hide-if'; + $classes[] = 'mw-htmlform-hide-if'; } if ( $valInSelect ) { @@ -165,7 +164,8 @@ class HTMLAutoCompleteSelectField extends HTMLTextField { } } - $this->mClass[] = 'mw-htmlform-autocomplete'; + $classes[] = 'mw-htmlform-autocomplete'; + $this->mClass = implode( ' ', $classes ); $ret .= parent::getInputHTML( $valInSelect ? '' : $value ); $this->mClass = $oldClass; diff --git a/includes/htmlform/fields/HTMLSelectAndOtherField.php b/includes/htmlform/fields/HTMLSelectAndOtherField.php index 39a05e1c721c..11b0d3e82120 100644 --- a/includes/htmlform/fields/HTMLSelectAndOtherField.php +++ b/includes/htmlform/fields/HTMLSelectAndOtherField.php @@ -13,6 +13,7 @@ * @todo FIXME: If made 'required', only the text field should be compulsory. */ class HTMLSelectAndOtherField extends HTMLSelectField { + private const FIELD_CLASS = 'mw-htmlform-select-and-other-field'; /** @var string[] */ private $mFlatOptions; @@ -46,16 +47,9 @@ class HTMLSelectAndOtherField extends HTMLSelectField { $select = parent::getInputHTML( $value[1] ); $textAttribs = [ - 'id' => $this->mID . '-other', 'size' => $this->getSize(), - 'class' => [ 'mw-htmlform-select-and-other-field' ], - 'data-id-select' => $this->mID, ]; - if ( $this->mClass !== '' ) { - $textAttribs['class'][] = $this->mClass; - } - if ( isset( $this->mParams['maxlength-unit'] ) ) { $textAttribs['data-mw-maxlength-unit'] = $this->mParams['maxlength-unit']; } @@ -74,7 +68,18 @@ class HTMLSelectAndOtherField extends HTMLSelectField { $textbox = Html::input( $this->mName . '-other', $value[2], 'text', $textAttribs ); - return "$select
\n$textbox"; + $wrapperAttribs = [ + 'id' => $this->mID, + 'class' => self::FIELD_CLASS + ]; + if ( $this->mClass !== '' ) { + $wrapperAttribs['class'] .= ' ' . $this->mClass; + } + return Html::rawElement( + 'div', + $wrapperAttribs, + "$select
\n$textbox" + ); } protected function getOOUIModules() { @@ -103,14 +108,9 @@ class HTMLSelectAndOtherField extends HTMLSelectField { $this->getAttributes( $allowedParams ) ); - if ( $this->mClass !== '' ) { - $textAttribs['classes'] = [ $this->mClass ]; - } - # DropdownInput $dropdownInputAttribs = [ 'name' => $this->mName, - 'id' => $this->mID . '-select', 'options' => $this->getOptionsOOUI(), 'value' => $value[1], ]; @@ -124,15 +124,15 @@ class HTMLSelectAndOtherField extends HTMLSelectField { $this->getAttributes( $allowedParams ) ); - if ( $this->mClass !== '' ) { - $dropdownInputAttribs['classes'] = [ $this->mClass ]; - } - $disabled = false; if ( isset( $this->mParams[ 'disabled' ] ) && $this->mParams[ 'disabled' ] ) { $disabled = true; } + $inputClasses = [ self::FIELD_CLASS ]; + if ( $this->mClass !== '' ) { + $inputClasses = array_merge( $inputClasses, explode( ' ', $this->mClass ) ); + } return $this->getInputWidget( [ 'id' => $this->mID, 'disabled' => $disabled, @@ -140,7 +140,7 @@ class HTMLSelectAndOtherField extends HTMLSelectField { 'dropdowninput' => $dropdownInputAttribs, 'or' => false, 'required' => $this->mParams[ 'required' ] ?? false, - 'classes' => [ 'mw-htmlform-select-and-other-field' ], + 'classes' => $inputClasses, 'data' => [ 'maxlengthUnit' => $this->mParams['maxlength-unit'] ?? 'bytes' ], diff --git a/includes/htmlform/fields/HTMLSelectOrOtherField.php b/includes/htmlform/fields/HTMLSelectOrOtherField.php index d2bb8ea1a9ad..1f1bb4d99a47 100644 --- a/includes/htmlform/fields/HTMLSelectOrOtherField.php +++ b/includes/htmlform/fields/HTMLSelectOrOtherField.php @@ -9,6 +9,7 @@ * @stable to extend */ class HTMLSelectOrOtherField extends HTMLTextField { + private const FIELD_CLASS = 'mw-htmlform-select-or-other'; /** * @stable to call @@ -37,12 +38,10 @@ class HTMLSelectOrOtherField extends HTMLTextField { $selected = $valInSelect ? $value : 'other'; - $select = new XmlSelect( $this->mName, $this->mID, $selected ); + $select = new XmlSelect( $this->mName, false, $selected ); $select->addOptions( $this->getOptions() ); - $select->setAttribute( 'class', 'mw-htmlform-select-or-other' ); - - $tbAttribs = [ 'id' => $this->mID . '-other', 'size' => $this->getSize() ]; + $tbAttribs = [ 'size' => $this->getSize() ]; if ( !empty( $this->mParams['disabled'] ) ) { $select->setAttribute( 'disabled', 'disabled' ); @@ -60,13 +59,20 @@ class HTMLSelectOrOtherField extends HTMLTextField { $tbAttribs['maxlength'] = $this->mParams['maxlength']; } - if ( $this->mClass !== '' ) { - $tbAttribs['class'] = $this->mClass; - } - $textbox = Html::input( $this->mName . '-other', $valInSelect ? '' : $value, 'text', $tbAttribs ); - return "$select
\n$textbox"; + $wrapperAttribs = [ + 'id' => $this->mID, + 'class' => self::FIELD_CLASS + ]; + if ( $this->mClass !== '' ) { + $wrapperAttribs['class'] .= ' ' . $this->mClass; + } + return Html::rawElement( + 'div', + $wrapperAttribs, + "$select
\n$textbox" + ); } protected function shouldInfuseOOUI() { @@ -93,7 +99,6 @@ class HTMLSelectOrOtherField extends HTMLTextField { 'name' => $this->mName, 'options' => $this->getOptionsOOUI(), 'value' => $valInSelect ? $value : 'other', - 'class' => [ 'mw-htmlform-select-or-other' ], ]; $allowedParams = [ @@ -125,9 +130,6 @@ class HTMLSelectOrOtherField extends HTMLTextField { $this->getAttributes( $allowedParams ) ); - if ( $this->mClass !== '' ) { - $textAttribs['classes'] = [ $this->mClass ]; - } if ( $this->mPlaceholder !== '' ) { $textAttribs['placeholder'] = $this->mPlaceholder; } @@ -137,8 +139,13 @@ class HTMLSelectOrOtherField extends HTMLTextField { $disabled = true; } + $inputClasses = [ self::FIELD_CLASS ]; + if ( $this->mClass !== '' ) { + $inputClasses = array_merge( $inputClasses, explode( ' ', $this->mClass ) ); + } return $this->getInputWidget( [ 'id' => $this->mID, + 'classes' => $inputClasses, 'disabled' => $disabled, 'textinput' => $textAttribs, 'dropdowninput' => $dropdownAttribs, -- cgit v1.2.3