[ticket/17100] Refactor code to be more reusable

PHPBB3-17100
2025-06-07 20:08:53 +00:00 · 2022-04-18 20:10:02 +02:00 · 2022-04-18 20:10:02 +02:00 · 2dfe5ebe6d
commit 2dfe5ebe6d
parent 516d5313ad
16 changed files with 189 additions and 101 deletions
--- a/phpBB/adm/style/acp_attachments.html
+++ b/phpBB/adm/style/acp_attachments.html
@ -51,15 +51,8 @@
 			<dl>
 				<dt><label for="{options.KEY}">{options.TITLE}{L_COLON}</label><!-- IF options.S_EXPLAIN --><br /><span>{options.TITLE_EXPLAIN}</span><!-- ENDIF --></dt>
 				<dd>
-					{% if options.CONTENT.tag %}
-						{% if options.CONTENT.tag == 'input' %}
-							{{ form_macros.input(options.CONTENT) }}
-						{% elseif options.CONTENT.tag == 'dimension' %}
-							{{ form_macros.dimension(options.CONTENT) }}
-						{% elseif options.CONTENT.tag == 'radio' %}
-							{{ form_macros.radio_buttons(options.CONTENT) }}
-						{% endif %}
-						{% if options.CONTENT.append %}{{ options.CONTENT.append }}{% endif %}
+					{% if options.CONTENT is iterable %}
+						{{ form_macros.build_template(options.CONTENT)}}
 					{% else %}
 						{options.CONTENT}
 					{% endif %}
--- a/phpBB/adm/style/acp_board.html
+++ b/phpBB/adm/style/acp_board.html
@ -29,15 +29,8 @@
 		<dl>
 			<dt><label for="{options.KEY}">{options.TITLE}{L_COLON}</label><!-- IF options.S_EXPLAIN --><br /><span>{options.TITLE_EXPLAIN}</span><!-- ENDIF --></dt>
 			<dd>
-				{% if options.CONTENT.tag %}
-					{% if options.CONTENT.tag == 'input' %}
-						{{ form_macros.input(options.CONTENT) }}
-					{% elseif options.CONTENT.tag == 'dimension' %}
-						{{ form_macros.dimension(options.CONTENT) }}
-					{% elseif options.CONTENT.tag == 'radio' %}
-						{{ form_macros.radio_buttons(options.CONTENT) }}
-					{% endif %}
-					{% if options.CONTENT.append %}{{ options.CONTENT.append }}{% endif %}
+				{% if options.CONTENT is iterable %}
+					{{ form_macros.build_template(options.CONTENT)}}
 				{% else %}
 					{{ options.CONTENT }}
 				{% endif %}
--- a/phpBB/adm/style/acp_bots.html
+++ b/phpBB/adm/style/acp_bots.html
@ -30,13 +30,10 @@
 		<dd><select id="bot_style" name="bot_style">{S_STYLE_OPTIONS}</select></dd>
 	</dl>
 	<dl>
-		<dt><label for="bot_lang">{L_BOT_LANG}{L_COLON}</label><br /><span>{L_BOT_LANG_EXPLAIN}</span></dt>
+		<dt><label for="{{ LANG_OPTIONS.id }}">{L_BOT_LANG}{L_COLON}</label><br /><span>{L_BOT_LANG_EXPLAIN}</span></dt>
 		<dd>
-			<select id="bot_lang" name="bot_lang">
-				{% for option in lang_options %}
-					<option value="{{ option.LANG_ISO }}"{% if option.SELECTED %} selected="selected"{% endif %}>{{ option.LANG_LOCAL_NAME }}</option>
-				{% endfor %}
-			</select>
+			{% import "form_macros.twig" as form_macros %}
+			{{ form_macros.select(LANG_OPTIONS) }}
 		</dd>
 	</dl>
 	<dl>
--- a/phpBB/adm/style/acp_users_prefs.html
+++ b/phpBB/adm/style/acp_users_prefs.html
@ -41,13 +41,10 @@
 			<label><input type="radio" class="radio" name="notifypm" value="0"<!-- IF not NOTIFY_PM --> id="notifypm" checked="checked"<!-- ENDIF --> /> {L_NO}</label></dd>
 	</dl>
 	<dl>
-		<dt><label for="lang">{L_BOARD_LANGUAGE}{L_COLON}</label></dt>
+		<dt><label for="{{ LANG_OPTIONS.id }}">{L_BOARD_LANGUAGE}{L_COLON}</label></dt>
 		<dd>
-			<select id="lang" name="lang">
-				{% for option in lang_options %}
-					<option value="{{ option.LANG_ISO }}"{% if option.SELECTED %} selected="selected"{% endif %}>{{ option.LANG_LOCAL_NAME }}</option>
-				{% endfor %}
-			</select>
+			{% import "form_macros.twig" as form_macros %}
+			{{ form_macros.select(LANG_OPTIONS) }}
 		</dd>
 	</dl>
 	<dl>
--- a/phpBB/adm/style/form_macros.twig
+++ b/phpBB/adm/style/form_macros.twig
@ -33,3 +33,26 @@
 	<label>{{ _self.input(form_data.buttons[0]) ~ form_data.buttons[0].label }}</label>
 	<label>{{ _self.input(form_data.buttons[1]) ~ form_data.buttons[1].label }}</label>
 {% endmacro %}
+
+{% macro select(form_data) %}
+	<select id="{{ form_data.id }}" name="{{ form_data.name }}">
+		{% for option in form_data.options %}
+			<option value="{{ option.value }}"{% if option.selected %} selected="selected"{% endif %}>{{ option.label }}</option>
+		{% endfor %}
+	</select>
+{% endmacro %}
+
+{% macro build_template(form_data) %}
+	{% if form_data.tag == 'input' %}
+		{{ _self.input(form_data) }}
+	{% elseif form_data.tag == 'dimension' %}
+		{{ _self.dimension(form_data) }}
+	{% elseif form_data.tag == 'radio' %}
+		{{ _self.radio_buttons(form_data) }}
+	{% elseif form_data[0] %}
+		{% for element in form_data %}
+			{{ _self.build_template(element) }}
+		{% endfor %}
+	{% endif %}
+	{% if form_data.append %}{{ form_data.append }}{% endif %}
+{% endmacro %}
--- a/phpBB/includes/acp/acp_bots.php
+++ b/phpBB/includes/acp/acp_bots.php
@ -330,7 +330,7 @@ class acp_bots
 				}

 				$style_select = style_select($bot_row['bot_style'], true);
-				phpbb_language_select($db, $template, $bot_row['bot_lang']);
+				$lang_options = phpbb_language_select($db, $bot_row['bot_lang']);

 				$l_title = ($action == 'edit') ? 'EDIT' : 'ADD';

@ -347,9 +347,13 @@ class acp_bots
 					'S_EDIT_BOT'		=> true,
 					'S_ACTIVE_OPTIONS'	=> $s_active_options,
 					'S_STYLE_OPTIONS'	=> $style_select,
+					'LANG_OPTIONS'		=> [
+						'id'		=> 'bot_lang',
+						'name'		=> 'bot_lang',
+						'options'	=> $lang_options,
+					],
 					'S_ERROR'			=> (count($error)) ? true : false,
-					)
-				);
+				));

 				return;

--- a/phpBB/includes/acp/acp_users.php
+++ b/phpBB/includes/acp/acp_users.php
@ -1796,7 +1796,7 @@ class acp_users
 				}

 				phpbb_timezone_select($template, $user, $data['tz'], true);
-				phpbb_language_select($db, $template, $data['lang']);
+				$lang_options = phpbb_language_select($db, $data['lang']);

 				$user_prefs_data = array(
 					'S_PREFS'			=> true,
@ -1833,6 +1833,11 @@ class acp_users
 					'DEFAULT_DATEFORMAT'	=> $config['default_dateformat'],
 					'A_DEFAULT_DATEFORMAT'	=> addslashes($config['default_dateformat']),

+					'LANG_OPTIONS'		=> [
+						'id'		=> 'lang',
+						'name'		=> 'lang',
+						'options'	=> $lang_options,
+					],
 					'S_STYLE_OPTIONS'	=> style_select($data['style']),
 				);

--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@ -270,7 +270,7 @@ function phpbb_version_compare(string $version1, string $version2, string $opera
 * @param string $default	Language ISO code to be selected by default in the dropdown list
 * @param array $langdata	Language data in format of array(array('lang_iso' => string, lang_local_name => string), ...)
 */
-function phpbb_language_select(\phpbb\db\driver\driver_interface $db, \phpbb\template\template $template, string $default = '', array $langdata = []): void
+function phpbb_language_select(\phpbb\db\driver\driver_interface $db, string $default = '', array $langdata = []): array
 {
 	if (empty($langdata))
 	{
@ -282,14 +282,17 @@ function phpbb_language_select(\phpbb\db\driver\driver_interface $db, \phpbb\tem
 		$db->sql_freeresult($result);
 	}

+	$lang_options = [];
 	foreach ($langdata as $row)
 	{
-		$template->assign_block_vars('lang_options', [
-			'SELECTED'			=> $row['lang_iso'] == $default,
-			'LANG_ISO'			=> $row['lang_iso'],
-			'LANG_LOCAL_NAME'	=> $row['lang_local_name']
-		]);
+		$lang_options[] = [
+			'value'		=> $row['lang_iso'],
+			'label'		=> $row['lang_local_name'],
+			'selected'	=> $row['lang_iso'] === $default,
+		];
 	}
+
+	return $lang_options;
 }

 /**
--- a/phpBB/includes/ucp/ucp_prefs.php
+++ b/phpBB/includes/ucp/ucp_prefs.php
@ -177,7 +177,7 @@ class ucp_prefs
 				$db->sql_freeresult($result);
 				$s_more_styles = count($styles_row) > 1;

-				phpbb_language_select($db, $template, $data['lang'], $lang_row);
+				$lang_options = phpbb_language_select($db, $data['lang'], $lang_row);

 				$template->assign_vars(array(
 					'ERROR'				=> (count($error)) ? implode('<br />', $error) : '',
@ -200,6 +200,11 @@ class ucp_prefs
 					'S_MORE_LANGUAGES'		=> $s_more_languages,
 					'S_MORE_STYLES'			=> $s_more_styles,

+					'LANG_OPTIONS'			=> [
+						'id'		=> 'lang',
+						'name'		=> 'lang',
+						'options'	=> $lang_options,
+					],
 					'S_STYLE_OPTIONS'		=> ($config['override_user_style']) ? '' : style_select($data['user_style'], false, $styles_row),
 					'S_CAN_HIDE_ONLINE'		=> ($auth->acl_get('u_hideonline')) ? true : false,
 					'S_SELECT_NOTIFY'		=> ($config['jab_enable'] && $user->data['user_jabber'] && @extension_loaded('xml')) ? true : false)
--- a/phpBB/includes/ucp/ucp_register.php
+++ b/phpBB/includes/ucp/ucp_register.php
@ -157,7 +157,7 @@ class ucp_register
 			$lang_row = (array) $db->sql_fetchrowset($result);
 			$db->sql_freeresult($result);

-			phpbb_language_select($db, $template, $user_lang, $lang_row);
+			$lang_options = phpbb_language_select($db, $user_lang, $lang_row);

 			if ($coppa === false && $config['coppa_enable'])
 			{
@ -169,6 +169,11 @@ class ucp_register
 				unset($now);

 				$template_vars = array(
+					'LANG_OPTIONS'		=> [
+						'id'		=> 'lang',
+						'name'		=> 'lang',
+						'options'	=> $lang_options,
+					],
 					'L_COPPA_NO'		=> $user->lang('UCP_COPPA_BEFORE', $coppa_birthday),
 					'L_COPPA_YES'		=> $user->lang('UCP_COPPA_ON_AFTER', $coppa_birthday),

@ -183,6 +188,11 @@ class ucp_register
 			else
 			{
 				$template_vars = array(
+					'LANG_OPTIONS'		=> [
+						'id'		=> 'lang',
+						'name'		=> 'lang',
+						'options'	=> $lang_options,
+					],
 					'L_TERMS_OF_USE'	=> sprintf($user->lang['TERMS_OF_USE_CONTENT'], $config['sitename'], generate_board_url()),

 					'S_SHOW_COPPA'		=> false,
@ -626,7 +636,7 @@ class ucp_register
 		$lang_row = (array) $db->sql_fetchrowset($result);
 		$db->sql_freeresult($result);

-		phpbb_language_select($db, $template, $data['lang'], $lang_row);
+		$lang_options = phpbb_language_select($db, $data['lang'], $lang_row);

 		$template_vars = array(
 			'USERNAME'			=> $data['username'],
@ -638,6 +648,11 @@ class ucp_register
 			'L_USERNAME_EXPLAIN'		=> $user->lang($config['allow_name_chars'] . '_EXPLAIN', $user->lang('CHARACTERS', (int) $config['min_name_chars']), $user->lang('CHARACTERS', (int) $config['max_name_chars'])),
 			'L_PASSWORD_EXPLAIN'		=> $user->lang($config['pass_complex'] . '_EXPLAIN', $user->lang('CHARACTERS', (int) $config['min_pass_chars'])),

+			'LANG_OPTIONS'		=> [
+				'id'		=> 'lang',
+				'name'		=> 'lang',
+				'options'	=> $lang_options,
+			],
 			'S_TZ_PRESELECT'	=> !$submit,
 			'S_CONFIRM_REFRESH'	=> ($config['enable_confirm'] && $config['confirm_refresh']) ? true : false,
 			'S_REGISTRATION'	=> true,
--- a/phpBB/phpbb/message/topic_form.php
+++ b/phpBB/phpbb/message/topic_form.php
@ -154,11 +154,16 @@ class topic_form extends form

 		$this->user->add_lang('viewtopic');

-		phpbb_language_select($this->db, $template, $this->recipient_lang);
+		$lang_options = phpbb_language_select($this->db, $this->recipient_lang);

 		$template->assign_vars(array(
 			'EMAIL'				=> $this->recipient_address,
 			'NAME'				=> $this->recipient_name,
+			'LANG_OPTIONS'		=> [
+				'id'		=> 'lang',
+				'name'		=> 'lang',
+				'options'	=> $lang_options,
+			],
 			'MESSAGE'			=> $this->body,

 			'L_EMAIL_BODY_EXPLAIN'	=> $this->user->lang['EMAIL_TOPIC_EXPLAIN'],
--- a/phpBB/styles/all/template/macros/form_macros.twig
+++ b/phpBB/styles/all/template/macros/form_macros.twig
@ -0,0 +1,58 @@
+{% macro input(form_data) %}
+	<input
+		{% if form_data.id %}id="{{ form_data.id }}"{% endif %}
+		type="{{ form_data.type }}"
+		name="{{ form_data.name }}"
+		{% if form_data.size %}size="{{ form_data.size }}"{% endif %}
+		{% if form_data.maxlength %}maxlength="{{ form_data.maxlength }}"{% endif %}
+		{% if form_data.min %}min="{{ form_data.min }}"{% endif %}
+		{% if form_data.max %}max="{{ form_data.max }}"{% endif %}
+		{% if form_data.type == 'password' %}autocomplete="off"{% endif %}
+		{% if form_data.checked %}checked="checked"{% endif %}
+		{% if form_data.class %}class="{{ form_data.class }}"{% endif %}
+		value="{{ form_data.value }}"
+	>
+{% endmacro %}
+
+{% macro dimension(form_data) %}
+	{{ _self.input(form_data.width) }} x {{ _self.input(form_data.height) }}
+{% endmacro %}
+
+{% macro textarea(form_data) %}
+	<textarea
+		id="{{ form_data.id }}"
+		name="{{ form_data.name }}"
+		rows="{{ form_data.rows }}"
+		cols="{{ form_data.cols }}"
+	>
+		{{ form_data.content }}
+	</textarea>
+{% endmacro %}
+
+{% macro radio_buttons(form_data) %}
+	<label>{{ _self.input(form_data.buttons[0]) ~ form_data.buttons[0].label }}</label>
+	<label>{{ _self.input(form_data.buttons[1]) ~ form_data.buttons[1].label }}</label>
+{% endmacro %}
+
+{% macro select(form_data) %}
+	<select id="{{ form_data.id }}" name="{{ form_data.name }}">
+		{% for option in form_data.options %}
+			<option value="{{ option.value }}"{% if option.selected %} selected="selected"{% endif %}>{{ option.label }}</option>
+		{% endfor %}
+	</select>
+{% endmacro %}
+
+{% macro build_template(form_data) %}
+	{% if form_data.tag == 'input' %}
+		{{ _self.input(form_data) }}
+	{% elseif form_data.tag == 'dimension' %}
+		{{ _self.dimension(form_data) }}
+	{% elseif form_data.tag == 'radio' %}
+		{{ _self.radio_buttons(form_data) }}
+	{% elseif form_data[0] %}
+		{% for element in form_data %}
+			{{ _self.build_template(element) }}
+		{% endfor %}
+	{% endif %}
+	{% if form_data.append %}{{ form_data.append }}{% endif %}
+{% endmacro %}
--- a/phpBB/styles/prosilver/template/ucp_agreement.html
+++ b/phpBB/styles/prosilver/template/ucp_agreement.html
@ -2,15 +2,12 @@

 <!-- IF S_SHOW_COPPA or S_REGISTRATION -->

-{% if lang_options|length > 0 %}
+{% if LANG_OPTIONS %}
 	<form method="post" action="{S_UCP_ACTION}" id="register">
 		<p class="rightside">
-			<label for="lang">{{ lang('LANGUAGE') ~ lang('COLON') }}</label>
-			<select name="lang" id="lang" title="{{ lang('LANGUAGE') }}">
-				{% for option in lang_options %}
-					<option value="{{ option.LANG_ISO }}"{% if option.SELECTED %} selected="selected"{% endif %}>{{ option.LANG_LOCAL_NAME }}</option>
-				{% endfor %}
-			</select>
+			<label for="{{ LANG_OPTIONS.id }}">{{ lang('LANGUAGE') ~ lang('COLON') }}</label>
+			{% import "macros/form_macros.twig" as form_macros %}
+			{{ form_macros.select(LANG_OPTIONS) }}
 			{S_HIDDEN_FIELDS}
 		</p>
 	</form>
@ -21,7 +18,7 @@
 		/**
 		 * Change language on change
 		 */
-		document.querySelector('#lang').addEventListener('change', (event) => {
+		document.querySelector("{{ '#' ~ LANG_OPTIONS.id }}").addEventListener('change', (event) => {
 			const langIso = event.target.value;
 			document.cookie = '{{ COOKIE_NAME }}_lang=' + langIso + '; path={{ COOKIE_PATH }}';
 			document.forms['register'].change_lang.value = langIso;
--- a/phpBB/styles/prosilver/template/ucp_prefs_personal.html
+++ b/phpBB/styles/prosilver/template/ucp_prefs_personal.html
@ -52,13 +52,10 @@
 	<!-- ENDIF -->
 	<!-- IF S_MORE_LANGUAGES -->
 		<dl>
-			<dt><label for="lang">{L_BOARD_LANGUAGE}{L_COLON}</label></dt>
+			<dt><label for="{{ LANG_OPTIONS.id }}">{L_BOARD_LANGUAGE}{L_COLON}</label></dt>
 			<dd>
-				<select name="lang" id="lang">
-					{% for option in lang_options %}
-						<option value="{{ option.LANG_ISO }}"{% if option.SELECTED %} selected="selected"{% endif %}>{{ option.LANG_LOCAL_NAME }}</option>
-					{% endfor %}
-				</select>
+				{% import "macros/form_macros.twig" as form_macros %}
+				{{ form_macros.select(LANG_OPTIONS) }}
 			</dd>
 		</dl>
 	<!-- ENDIF -->
--- a/phpBB/styles/prosilver/template/ucp_register.html
+++ b/phpBB/styles/prosilver/template/ucp_register.html
@ -57,17 +57,23 @@
 	<hr />

 	<!-- EVENT ucp_register_options_before -->
-	{% if lang_options|length > 0 %}
+	{% if LANG_OPTIONS %}
 	<dl>
-		<dt><label for="lang">{{ lang('LANGUAGE') ~ lang('COLON') }}</label></dt>
+		<dt><label for="{{ LANG_OPTIONS.id }}">{{ lang('LANGUAGE') ~ lang('COLON') }}</label></dt>
 		<dd>
-			<select name="lang" id="lang" onchange="change_language(this.value); return false;" tabindex="6" title="{{ lang('LANGUAGE') }}">
-				{% for option in lang_options %}
-					<option value="{{ option.LANG_ISO }}"{% if option.SELECTED %} selected="selected"{% endif %}>{{ option.LANG_LOCAL_NAME }}</option>
-				{% endfor %}
-			</select>
+			{% import "macros/form_macros.twig" as form_macros %}
+			{{ form_macros.select(LANG_OPTIONS) }}
 		</dd>
 	</dl>
+	<script>
+		/**
+		 * Change language on change
+		 */
+		document.querySelector("{{ '#' ~ LANG_OPTIONS.id }}").addEventListener('change', (event) => {
+			change_language(event.target.value);
+			return false;
+		});
+	</script>
 	{% endif %}

 	<!-- INCLUDE timezone_option.html -->
--- a/tests/functions/language_select_test.php
+++ b/tests/functions/language_select_test.php
@ -25,14 +25,14 @@ class phpbb_functions_language_select_test extends phpbb_database_test_case
 				'',
 				[
 					[
-						'SELECTED'			=> false,
-						'LANG_ISO'			=> 'cs',
-						'LANG_LOCAL_NAME'	=> 'Čeština',
+						'selected'			=> false,
+						'value'			=> 'cs',
+						'label'	=> 'Čeština',
 					],
 					[
-						'SELECTED'			=> false,
-						'LANG_ISO'			=> 'en',
-						'LANG_LOCAL_NAME'	=> 'English',
+						'selected'			=> false,
+						'value'			=> 'en',
+						'label'	=> 'English',
 					],
 				]
 			],
@ -40,14 +40,14 @@ class phpbb_functions_language_select_test extends phpbb_database_test_case
 				'en',
 				[
 					[
-						'SELECTED'			=> false,
-						'LANG_ISO'			=> 'cs',
-						'LANG_LOCAL_NAME'	=> 'Čeština',
+						'selected'			=> false,
+						'value'			=> 'cs',
+						'label'	=> 'Čeština',
 					],
 					[
-						'SELECTED'			=> true,
-						'LANG_ISO'			=> 'en',
-						'LANG_LOCAL_NAME'	=> 'English',
+						'selected'			=> true,
+						'value'			=> 'en',
+						'label'	=> 'English',
 					],
 				]
 			],
@ -55,14 +55,14 @@ class phpbb_functions_language_select_test extends phpbb_database_test_case
 				'cs',
 				[
 					[
-						'SELECTED'			=> true,
-						'LANG_ISO'			=> 'cs',
-						'LANG_LOCAL_NAME'	=> 'Čeština',
+						'selected'			=> true,
+						'value'			=> 'cs',
+						'label'	=> 'Čeština',
 					],
 					[
-						'SELECTED'			=> false,
-						'LANG_ISO'			=> 'en',
-						'LANG_LOCAL_NAME'	=> 'English',
+						'selected'			=> false,
+						'value'			=> 'en',
+						'label'	=> 'English',
 					],
 				]
 			],
@ -70,14 +70,14 @@ class phpbb_functions_language_select_test extends phpbb_database_test_case
 				'de',
 				[
 					[
-						'SELECTED'			=> false,
-						'LANG_ISO'			=> 'cs',
-						'LANG_LOCAL_NAME'	=> 'Čeština',
+						'selected'			=> false,
+						'value'			=> 'cs',
+						'label'	=> 'Čeština',
 					],
 					[
-						'SELECTED'			=> false,
-						'LANG_ISO'			=> 'en',
-						'LANG_LOCAL_NAME'	=> 'English',
+						'selected'			=> false,
+						'value'			=> 'en',
+						'label'	=> 'English',
 					],
 				]
 			],
@ -91,19 +91,9 @@ class phpbb_functions_language_select_test extends phpbb_database_test_case
 	{
 		global $db;
 		$db = $this->new_dbal();
-		$template	= $this->getMockBuilder('\phpbb\template\base')
-			->disableOriginalConstructor()
-			->getMock();
-		$template_data = [];
-		$template->expects($this->any())
-			->method('assign_block_vars')
-			->willReturnCallback(function ($blockname, array $vararray) use (&$template_data) {
-				$template_data[$blockname][] = $vararray;
-				return null;
-			});

-		phpbb_language_select($db, $template, $default);
+		$lang_options = phpbb_language_select($db, $default);

-		$this->assertEquals($expected, $template_data['lang_options']);
+		$this->assertEquals($expected, $lang_options);
 	}
 }