From: Bartosz Dziewoński <matma.rex@gmail.com>
Date: Wed, 12 Jul 2017 19:31:33 +0000 (+0200)
Subject: Reduce code duplication for parsing messages into dropdown menus
X-Git-Tag: 1.31.0-rc.0~2137^2
X-Git-Url: https://git.heureux-cyclage.org/?a=commitdiff_plain;h=a5c3d029c5d0f5765827d10d3fc34942a4d16853;p=lhc%2Fweb%2Fwiklou.git

Reduce code duplication for parsing messages into dropdown menus

The same behavior was implemented by Xml::listDropDown(),
Article::confirmDelete() and HTMLFormField::getOptions().
A new function Xml::listDropDownOptions() is added and
all three are changed to use it.

Additionally:
* Xml::listDropDown() now uses XmlSelect internally to generate the
  dropdown HTML.
* Xml::listDropDownOptionsOoui() is introduced to handle converting
  the existing format to the OOUI format, which was previously
  duplicated in Article::confirmDelete() and HTMLFormField::getOptionsOOUI().
* This change allows HTMLForm 'select' fields (HTMLSelectField) to
  support nested options (optgroups) in OOUI mode. This is a
  prerequisite for T117781.

Bug: T117781
Change-Id: I0a088f61eb32ec59677113583c7ecdcbc3fd2af0
---

diff --git a/includes/Xml.php b/includes/Xml.php
index 16a5a9ddec..0091513125 100644
--- a/includes/Xml.php
+++ b/includes/Xml.php
@@ -493,7 +493,8 @@ class Xml {
 	}
 
 	/**
-	 * Build a drop-down box from a textual list.
+	 * Build a drop-down box from a textual list. This is a wrapper
+	 * for Xml::listDropDownOptions() plus the XmlSelect class.
 	 *
 	 * @param string $name Name and id for the drop-down
 	 * @param string $list Correctly formatted text (newline delimited) to be
@@ -507,60 +508,91 @@ class Xml {
 	public static function listDropDown( $name = '', $list = '', $other = '',
 		$selected = '', $class = '', $tabindex = null
 	) {
-		$optgroup = false;
+		$options = self::listDropDownOptions( $list, [ 'other' => $other ] );
+
+		$xmlSelect = new XmlSelect( $name, $name, $selected );
+		$xmlSelect->addOptions( $options );
+
+		if ( $class ) {
+			$xmlSelect->setAttribute( 'class', $class );
+		}
+		if ( $tabindex ) {
+			$xmlSelect->setAttribute( 'tabindex', $tabindex );
+		}
 
-		$options = self::option( $other, 'other', $selected === 'other' );
+		return $xmlSelect->getHTML();
+	}
 
+	/**
+	 * Build options for a drop-down box from a textual list.
+	 *
+	 * The result of this function can be passed to XmlSelect::addOptions()
+	 * (to render a plain `<select>` dropdown box) or to Xml::listDropDownOptionsOoui()
+	 * and then OOUI\DropdownInputWidget() (to render a pretty one).
+	 *
+	 * @param string $list Correctly formatted text (newline delimited) to be
+	 *   used to generate the options.
+	 * @param array $params Extra parameters
+	 * @param string $params['other'] If set, add an option with this as text and a value of 'other'
+	 * @return array Array keys are textual labels, values are internal values
+	 */
+	public static function listDropDownOptions( $list, $params = [] ) {
+		$options = [];
+
+		if ( isset( $params['other'] ) ) {
+			$options[ $params['other'] ] = 'other';
+		}
+
+		$optgroup = false;
 		foreach ( explode( "\n", $list ) as $option ) {
 			$value = trim( $option );
 			if ( $value == '' ) {
 				continue;
 			} elseif ( substr( $value, 0, 1 ) == '*' && substr( $value, 1, 1 ) != '*' ) {
-				// A new group is starting ...
+				# A new group is starting...
 				$value = trim( substr( $value, 1 ) );
-				if ( $optgroup ) {
-					$options .= self::closeElement( 'optgroup' );
-				}
-				$options .= self::openElement( 'optgroup', [ 'label' => $value ] );
-				$optgroup = true;
+				$optgroup = $value;
 			} elseif ( substr( $value, 0, 2 ) == '**' ) {
-				// groupmember
-				$value = trim( substr( $value, 2 ) );
-				$options .= self::option( $value, $value, $selected === $value );
-			} else {
-				// groupless reason list
-				if ( $optgroup ) {
-					$options .= self::closeElement( 'optgroup' );
+				# groupmember
+				$opt = trim( substr( $value, 2 ) );
+				if ( $optgroup === false ) {
+					$options[$opt] = $opt;
+				} else {
+					$options[$optgroup][$opt] = $opt;
 				}
-				$options .= self::option( $value, $value, $selected === $value );
+			} else {
+				# groupless reason list
 				$optgroup = false;
+				$options[$option] = $option;
 			}
 		}
 
-		if ( $optgroup ) {
-			$options .= self::closeElement( 'optgroup' );
-		}
-
-		$attribs = [];
-
-		if ( $name ) {
-			$attribs['id'] = $name;
-			$attribs['name'] = $name;
-		}
+		return $options;
+	}
 
-		if ( $class ) {
-			$attribs['class'] = $class;
-		}
+	/**
+	 * Convert options for a drop-down box into a format accepted by OOUI\DropdownInputWidget etc.
+	 *
+	 * TODO Find a better home for this function.
+	 *
+	 * @param array $options Options, as returned e.g. by Xml::listDropDownOptions()
+	 * @return array
+	 */
+	public static function listDropDownOptionsOoui( $options ) {
+		$optionsOoui = [];
 
-		if ( $tabindex ) {
-			$attribs['tabindex'] = $tabindex;
+		foreach ( $options as $text => $value ) {
+			if ( is_array( $value ) ) {
+				$optionsOoui[] = [ 'optgroup' => (string)$text ];
+				foreach ( $value as $text2 => $value2 ) {
+					$optionsOoui[] = [ 'data' => (string)$value2, 'label' => (string)$text2 ];
+				}
+			} else {
+				$optionsOoui[] = [ 'data' => (string)$value, 'label' => (string)$text ];
+			}
 		}
 
-		return self::openElement( 'select', $attribs )
-			. "\n"
-			. $options
-			. "\n"
-			. self::closeElement( 'select' );
+		return $optionsOoui;
 	}
 
 	/**
diff --git a/includes/htmlform/HTMLFormField.php b/includes/htmlform/HTMLFormField.php
index dfd18ba4c9..e642c2cdd0 100644
--- a/includes/htmlform/HTMLFormField.php
+++ b/includes/htmlform/HTMLFormField.php
@@ -1076,33 +1076,8 @@ abstract class HTMLFormField {
 				$this->mOptionsLabelsNotFromMessage = true;
 				$this->mOptions = self::forceToStringRecursive( $this->mParams['options'] );
 			} elseif ( array_key_exists( 'options-message', $this->mParams ) ) {
-				/** @todo This is copied from Xml::listDropDown(), deprecate/avoid duplication? */
 				$message = $this->getMessage( $this->mParams['options-message'] )->inContentLanguage()->plain();
-
-				$optgroup = false;
-				$this->mOptions = [];
-				foreach ( explode( "\n", $message ) as $option ) {
-					$value = trim( $option );
-					if ( $value == '' ) {
-						continue;
-					} elseif ( substr( $value, 0, 1 ) == '*' && substr( $value, 1, 1 ) != '*' ) {
-						# A new group is starting...
-						$value = trim( substr( $value, 1 ) );
-						$optgroup = $value;
-					} elseif ( substr( $value, 0, 2 ) == '**' ) {
-						# groupmember
-						$opt = trim( substr( $value, 2 ) );
-						if ( $optgroup === false ) {
-							$this->mOptions[$opt] = $opt;
-						} else {
-							$this->mOptions[$optgroup][$opt] = $opt;
-						}
-					} else {
-						# groupless reason list
-						$optgroup = false;
-						$this->mOptions[$option] = $option;
-					}
-				}
+				$this->mOptions = Xml::listDropDownOptions( $message );
 			} else {
 				$this->mOptions = null;
 			}
@@ -1122,16 +1097,7 @@ abstract class HTMLFormField {
 			return null;
 		}
 
-		$options = [];
-
-		foreach ( $oldoptions as $text => $data ) {
-			$options[] = [
-				'data' => (string)$data,
-				'label' => (string)$text,
-			];
-		}
-
-		return $options;
+		return Xml::listDropDownOptionsOoui( $oldoptions );
 	}
 
 	/**
diff --git a/includes/page/Article.php b/includes/page/Article.php
index 5b03a22a2f..b91bd9a88a 100644
--- a/includes/page/Article.php
+++ b/includes/page/Article.php
@@ -1695,24 +1695,11 @@ class Article implements Page {
 
 		$outputPage->enableOOUI();
 
-		$options = [];
-		$options[] = [
-			'data' => 'other',
-			'label' => $ctx->msg( 'deletereasonotherlist' )->inContentLanguage()->text(),
-		];
-		$list = $ctx->msg( 'deletereason-dropdown' )->inContentLanguage()->text();
-		foreach ( explode( "\n", $list ) as $option ) {
-			$value = trim( $option );
-			if ( $value == '' ) {
-				continue;
-			} elseif ( substr( $value, 0, 1 ) == '*' && substr( $value, 1, 1 ) != '*' ) {
-				$options[] = [ 'optgroup' => trim( substr( $value, 1 ) ) ];
-			} elseif ( substr( $value, 0, 2 ) == '**' ) {
-				$options[] = [ 'data' => trim( substr( $value, 2 ) ) ];
-			} else {
-				$options[] = [ 'data' => trim( $value ) ];
-			}
-		}
+		$options = Xml::listDropDownOptions(
+			$ctx->msg( 'deletereason-dropdown' )->inContentLanguage()->text(),
+			[ 'other' => $ctx->msg( 'deletereasonotherlist' )->inContentLanguage()->text() ]
+		);
+		$options = Xml::listDropDownOptionsOoui( $options );
 
 		$fields[] = new OOUI\FieldLayout(
 			new OOUI\DropdownInputWidget( [
diff --git a/tests/phpunit/includes/XmlTest.php b/tests/phpunit/includes/XmlTest.php
index 6c059ec5fd..25b754d2bc 100644
--- a/tests/phpunit/includes/XmlTest.php
+++ b/tests/phpunit/includes/XmlTest.php
@@ -403,12 +403,15 @@ class XmlTest extends MediaWikiTestCase {
 	 */
 	public function testListDropDown() {
 		$this->assertEquals(
-			'<select id="test-name" name="test-name" class="test-css" tabindex="2">' . "\n" .
-				'<option value="other">other reasons</option>' .
-				'<optgroup label="Foo"><option value="Foo 1">Foo 1</option>' .
-				'<option value="Example" selected="">Example</option>' .
-				'</optgroup><optgroup label="Bar">' .
-				'<option value="Bar 1">Bar 1</option></optgroup>' . "\n" .
+			'<select name="test-name" id="test-name" class="test-css" tabindex="2">' .
+				'<option value="other">other reasons</option>' . "\n" .
+				'<optgroup label="Foo">' .
+				'<option value="Foo 1">Foo 1</option>' . "\n" .
+				'<option value="Example" selected="">Example</option>' . "\n" .
+				'</optgroup>' . "\n" .
+				'<optgroup label="Bar">' .
+				'<option value="Bar 1">Bar 1</option>' . "\n" .
+				'</optgroup>' .
 				'</select>',
 			Xml::listDropDown(
 				// name
@@ -426,4 +429,52 @@ class XmlTest extends MediaWikiTestCase {
 			)
 		);
 	}
+
+	/**
+	 * @covers Xml::listDropDownOptions
+	 */
+	public function testListDropDownOptions() {
+		$this->assertEquals(
+			[
+				'other reasons' => 'other',
+				'Foo' => [
+					'Foo 1' => 'Foo 1',
+					'Example' => 'Example',
+				],
+				'Bar' => [
+					'Bar 1' => 'Bar 1',
+				],
+			],
+			Xml::listDropDownOptions(
+				"* Foo\n** Foo 1\n** Example\n* Bar\n** Bar 1",
+				[ 'other' => 'other reasons' ]
+			)
+		);
+	}
+
+	/**
+	 * @covers Xml::listDropDownOptionsOoui
+	 */
+	public function testListDropDownOptionsOoui() {
+		$this->assertEquals(
+			[
+				[ 'data' => 'other', 'label' => 'other reasons' ],
+				[ 'optgroup' => 'Foo' ],
+				[ 'data' => 'Foo 1', 'label' => 'Foo 1' ],
+				[ 'data' => 'Example', 'label' => 'Example' ],
+				[ 'optgroup' => 'Bar' ],
+				[ 'data' => 'Bar 1', 'label' => 'Bar 1' ],
+			],
+			Xml::listDropDownOptionsOoui( [
+				'other reasons' => 'other',
+				'Foo' => [
+					'Foo 1' => 'Foo 1',
+					'Example' => 'Example',
+				],
+				'Bar' => [
+					'Bar 1' => 'Bar 1',
+				],
+			] )
+		);
+	}
 }