From 99804f40936f312b5d14f851ecd90809e263c29b Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Fri, 9 Feb 2024 22:30:28 +0000 Subject: [PATCH] Issue #3395555 by Eli-T, Akhil Babu, borisson_, Wim Leers, phenaproxima, penyaskito: Add validation constraints to olivero.settings --- core/config/schema/core.data_types.schema.yml | 12 ++++++ .../tests/src/Functional/System/ThemeTest.php | 42 +++++++++++++++++++ .../olivero/config/schema/olivero.schema.yml | 14 +++++++ core/themes/olivero/theme-settings.php | 17 ++------ 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/core/config/schema/core.data_types.schema.yml b/core/config/schema/core.data_types.schema.yml index 51b7a36f9cd8..390c0fd6bbbf 100644 --- a/core/config/schema/core.data_types.schema.yml +++ b/core/config/schema/core.data_types.schema.yml @@ -126,6 +126,18 @@ date_format: color_hex: type: string label: 'Color' + constraints: + # TRICKY: this cannot reuse the Color::validateHex() logic because: + # 1. Color::validateHex() would not allow NULL, but this constraint does. + # (Anything in config can be marked optional, so NULL must never trigger + # a validation error) Color::validateHex() does not allow this. + # 2. The Regex constraint is something that an external tool (no matter + # whether that's something generating/validating Drupal config or a + # JSON:API/REST/GraphQL/… client) to also correctly validate this. + Regex: + # Regex copied from Color::validateHex() + pattern: '/^[#]?([0-9a-fA-F]{3}){1,2}$/' + message: "%value is not a valid hexadecimal color." # Machine-readable identifier that can only contain certain characters. machine_name: diff --git a/core/modules/system/tests/src/Functional/System/ThemeTest.php b/core/modules/system/tests/src/Functional/System/ThemeTest.php index 2bec2adc5fc9..69d56c38d56b 100644 --- a/core/modules/system/tests/src/Functional/System/ThemeTest.php +++ b/core/modules/system/tests/src/Functional/System/ThemeTest.php @@ -261,6 +261,48 @@ public function testThemeSettingsLogo() { $this->assertSession()->pageTextNotContains('Shortcut icon settings'); } + /** + * Tests the theme settings color input. + */ + public function testThemeSettingsColorHexCode() : void { + // Install the Olivero theme. + $this->container->get('theme_installer')->install(['olivero']); + + // Define invalid and valid hex color codes. + $invalid_hex_codes = [ + 'xyz', + '#xyz', + '#ffff', + '#00000', + '#FFFFF ', + '00#000', + ]; + $valid_hex_codes = [ + '0F0', + '#F0F', + '#2ecc71', + '0074cc', + ]; + + // Visit Olivero's theme settings page. + $this->drupalGet('admin/appearance/settings/olivero'); + + // Test invalid hex color codes. + foreach ($invalid_hex_codes as $invalid_hex) { + $this->submitForm(['base_primary_color' => $invalid_hex], 'Save configuration'); + // Invalid hex codes should throw error. + $this->assertSession()->statusMessageContains('"' . $invalid_hex . '" is not a valid hexadecimal color.', 'error'); + $this->assertTrue($this->getSession()->getPage()->findField('base_primary_color')->hasClass('error')); + } + + // Test valid hex color codes. + foreach ($valid_hex_codes as $valid_hex) { + $this->submitForm(['base_primary_color' => $valid_hex], 'Save configuration'); + $this->assertSession()->statusMessageContains('The configuration options have been saved.', 'status'); + $this->assertSame($valid_hex, $this->config('olivero.settings')->get('base_primary_color')); + } + } + /** * Tests the 'rendered' cache tag is cleared when saving theme settings. */ diff --git a/core/themes/olivero/config/schema/olivero.schema.yml b/core/themes/olivero/config/schema/olivero.schema.yml index 7723ba3a464b..0bd946db2874 100644 --- a/core/themes/olivero/config/schema/olivero.schema.yml +++ b/core/themes/olivero/config/schema/olivero.schema.yml @@ -18,9 +18,23 @@ olivero.settings: mobile_menu_all_widths: type: integer label: 'Mobile menu all widths' + constraints: + Choice: + # @see olivero_preprocess_html() + # Set to 1 to enable the mobile menu toggle at all widths. + choices: + - 0 + - 1 site_branding_bg_color: type: string label: 'Site branding background color' + constraints: + Choice: + # @see olivero_form_system_theme_settings_alter() + choices: + - "default" + - "gray" + - "white" base_primary_color: type: color_hex label: 'Base Primary Color' diff --git a/core/themes/olivero/theme-settings.php b/core/themes/olivero/theme-settings.php index 7a5803754ece..849df4c11d5d 100644 --- a/core/themes/olivero/theme-settings.php +++ b/core/themes/olivero/theme-settings.php @@ -6,13 +6,11 @@ */ use Drupal\Core\Form\FormStateInterface; -use Drupal\Component\Utility\Color; /** * Implements hook_form_FORM_ID_alter() for system_theme_settings. */ function olivero_form_system_theme_settings_alter(&$form, FormStateInterface $form_state) { - $form['#validate'][] = 'olivero_theme_settings_validate'; $form['#attached']['library'][] = 'olivero/color-picker'; $color_config = [ @@ -62,7 +60,7 @@ function olivero_form_system_theme_settings_alter(&$form, FormStateInterface $fo $form['olivero_settings']['olivero_utilities']['mobile_menu_all_widths'] = [ '#type' => 'checkbox', '#title' => t('Enable mobile menu at all widths'), - '#default_value' => theme_get_setting('mobile_menu_all_widths'), + '#config_target' => 'olivero.settings:mobile_menu_all_widths', '#description' => t('Enables the mobile menu toggle at all widths.'), ]; $form['olivero_settings']['olivero_utilities']['site_branding_bg_color'] = [ @@ -73,7 +71,7 @@ function olivero_form_system_theme_settings_alter(&$form, FormStateInterface $fo 'gray' => t('Gray'), 'white' => t('White'), ], - '#default_value' => theme_get_setting('site_branding_bg_color'), + '#config_target' => 'olivero.settings:site_branding_bg_color', ]; $form['olivero_settings']['olivero_utilities']['olivero_color_scheme'] = [ '#type' => 'fieldset', @@ -109,7 +107,7 @@ function olivero_form_system_theme_settings_alter(&$form, FormStateInterface $fo '#size' => 10, '#title' => t($title), '#description' => t('Enter color in hexadecimal format (#abc123).') . '<br/>' . t('Derivatives will be formed from this color.'), - '#default_value' => theme_get_setting($key), + '#config_target' => "olivero.settings:$key", '#attributes' => [ // Regex copied from Color::validateHex() 'pattern' => '^[#]?([0-9a-fA-F]{3}){1,2}$', @@ -120,12 +118,3 @@ function olivero_form_system_theme_settings_alter(&$form, FormStateInterface $fo ]; } } - -/** - * Validation handler for the Olivero system_theme_settings form. - */ -function olivero_theme_settings_validate($form, FormStateInterface $form_state) { - if (!Color::validateHex($form_state->getValue('base_primary_color'))) { - $form_state->setErrorByName('base_primary_color', t('Colors must be 7-character string specifying a color hexadecimal format.')); - } -} -- GitLab