Problem/Motivation
Part of the plan in #3565209: [Meta] Drop support for Drupal 10.2 and below: Use config_target for forms.
The settings forms of Admin Toolbar, Admin Toolbar Tools and Admin Toolbar Search still use the legacy pattern of ConfigFormBase: each form reads its values with $this->config() to set #default_value, declares getEditableConfigNames() and copies each submitted value to the configuration by hand in submitForm().
Since Drupal 10.2, the #config_target form element key makes Drupal load the default values, save the submitted values and validate them against the configuration schema. The configuration schemas of the three modules are already fully validatable (#3570717: Make configuration schema fully validatable), but the forms do not benefit from it.
Steps to reproduce
Review the form classes:
src/Form/AdminToolbarSettingsForm.phpadmin_toolbar_tools/src/Form/AdminToolbarToolsSettingsForm.phpadmin_toolbar_search/src/Form/AdminToolbarSearchSettingsForm.php
Proposed resolution
- Add
#config_targetto each form element and remove the#default_valueproperties and the manual saving insubmitForm(). - Use core's
RedundantEditableConfigNamesTraitinstead ofgetEditableConfigNames(). - Use the
Rangeconstraints of the schema for the limits of the Menu depth and Maximum number of bundle sub-menus fields, so the form and the schema have a single source of truth. - Cast the hoverIntent timeout to an integer with a
ConfigTarget(toConfig), since the select element returns a string and the schema expects an integer. - Keep the labels and descriptions in
$this->t()in the forms, so the existing translations still apply. - Add an update hook setting
sticky_behaviortoenabledif it does not exist: it was added without an update hook, and the setting is now required by the schema and the form. - Add and update tests to check the saved values and their types.
Remaining tasks
- Review.
- Once the Drupal 12 issue (#3594696) is merged, add the PHPUnit attributes to the new test (
#[Group],#[RunTestsInSeparateProcesses]) if needed.
User interface changes
None, the forms look the same. Submitted values are now validated with the configuration schema, so an invalid value is reported as a form error.
API changes
None. The form classes no longer define getEditableConfigNames(), which is provided by RedundantEditableConfigNamesTrait.
Data model changes
None. The update hook admin_toolbar_update_8007() only adds the sticky_behavior: enabled setting, if it is missing.
Issue fork admin_toolbar-3638122
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
dydave commentedComment #5
dydave commentedMerged, thanks! 🥳
The settings forms of Admin Toolbar, Admin Toolbar Tools and Admin Toolbar Search now use
#config_target, so Drupal loads, validates and saves the settings from the configuration schema:#default_valueproperties, the manual saving insubmitForm()andgetEditableConfigNames()(replaced by core'sRedundantEditableConfigNamesTrait).Rangeconstraints of the schema.ConfigTarget, as expected by the schema.$this->t(), so the existing translations still apply.admin_toolbar_update_8007(), settingsticky_behaviortoenabledif it does not exist, since the schema requires it.There are no visible changes to the forms, except invalid values are now reported as form errors.
The change will be included in the next release.