Problem/Motivation

A setting's label and description are often written twice: as label in its config schema, and as #title and #description on its settings form element.

Example: for the page cache's maximum age system.schema.yml reads:

max_age:
  type: integer
  label: 'Max age'

And PerformanceForm reads:

$form['caching']['page_cache_maximum_age'] = [
  '#type' => 'select',
  '#title' => $this->t('Browser and proxy cache maximum age'),
  '#config_target' => 'system.performance:cache.page.max_age',
  '#options' => $period,
  '#description' => $this->t('This is used as the value for max-age in Cache-Control headers.'),
];

There a number of challenges with the code above. They often drift (e.g. 'Max age' vs 'Browser and proxy cache maximum age') but perhaps more importantly, the form fields can't be accessed by other users inside or outside of Drupal. This issue was split out of #3635315: [Prototype] Write a setting's rules once, in config schema, and use them for validation, forms and the published JSON Schema where I go into a lot more detail about these challenges.

With the MR, the schema looks as follows:

max_age:
  type: integer
  label: 'Browser and proxy cache maximum age'
  description: 'This is used as the value for max-age in Cache-Control headers.'

And the form element can leave out the title and description:

$form['caching']['page_cache_maximum_age'] = [
  '#type' => 'select',
  '#config_target' => 'system.performance:cache.page.max_age',
  '#options' => $period,
];

The text now lives in one place, so the form and the schema can no longer drift apart. And because the label and description moved out of the form's PHP code and into its schema, other code (e.g.ECA, FlowDrop, Maestro, Tool API, etc) can describe a setting the same way a Drupal form does.

#3477363: Add a way to automatically generate basic config forms goes further: it generates whole form elements from config schema. This issue helps us get a step closer to making #3477363 reality.

In #3477363, joachim and james.williams agreed that a description belongs in config schema (https://www.drupal.org/project/drupal/issues/3477363#comment-16098165).

Issue fork drupal-3636978

Command icon 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:

About issue forks

Comments

dries created an issue. See original summary.

dries changed the visibility of the branch 3636978-titles-from-shemas to hidden.

dries opened merge request !17779

dries’s picture

dries commented
Status: Active » Needs review

I took the code from the prototype in #3635315 and evolved it.

When a form element in a ConfigFormBase form has a #config_target and no #title or #description, we now use the label and description specified in the config schema.

If a form element already has a #title or #description, the existing ones continue to be used. That way existing forms remain unchanged, and we can convert forms one at a time.

The MR adds TypedConfigManagerInterface::getDefinitionForKey(), which reads the definition straight from the schema. It's faster and more reusable than the prototype I had in #3635315.

As a demo, max_age's title and description moved from PerformanceForm into system.schema.yml. The form renders the same as before, except that we still have to add translation support for descriptions.

I'd love feedback on the direction before going further. If people like it, the same pattern can be applied to many other settings forms in follow-ups.

Disclaimer: I built this with the help of an LLM but reviewed everything extensively.

dries’s picture

dries commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

We need an issue open in potx to extract these strings from schema.

dries’s picture

dries commented

Agreed, we need a potx issue, and I'm happy to open it. I wanted to know if we were aligned on the idea and approach first.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I think the direction here looks good, this seems simpler than the min/max issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I think direction is fine and actually we don't need a potx issue we just need to add description to core.data_types.schema.yml

description:
  type: label
  label: 'Description'

The one thing we have to work out is about the regex and whether the description regex (inherited above from label) should be more permissive.

FWIW; in Config Translation we're already doing

        // Labels originate from configuration schema and are translatable.
        // phpcs:ignore Drupal.Semantics.FunctionT.NotLiteralString
        '@label' => $this->t($this->definition->getLabel()),

So we don't have to worry about translation context because we've already accepted we don't have a way to mark that up in schema.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Another thing - are we sure that the new API on TypeConfigManager is needed? We do something very similar in the config translation module. I also wonder about performance - if we're building data structure multiple times for the same config file whether it'd be better to build one data definition per config target and use that.

dries’s picture

dries commented

Thanks for the reviews, @alexpott and @borisson_, and for the +1 on the direction.

On the regex (#9): I don't think it needs loosening right now. I believe the Regex constraint checks config values, not schema definitions.

On the new API (#10): a new API isn't strictly needed. However, I first implemented what config_translation does, building typed config from the saved values, but building AccountSettingsForm then took 4.4 ms, or 1.3 ms with a cache, against 0.75 ms reading only the schema.

I ultimately felt that finding a key in config schema is typed config manager's job, not a form's. We don't need the saved values here, just the schema definition. I also found contributed modules that do this lookup by hand, such as layout_paragraphs_theme_extension and views_autocomplete_filters. They would benefit from a getDefinitionForKey().

If you'd rather not add a method to TypedConfigManagerInterface, I can move the loop into ConfigFormBase. Personally, I don't think that is the better trade-off, but I'm happy to follow your call.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Hmmm... you're right about the regex applying to config values... I misunderstood how potx discovers the label in schema files... we do need an issue for potx. We need to update the yaml_translation_patterns.yml to include description in the translatable keys.

  - matches: 'config/schema/*.yml'
    translatable_keys:
      - label
+      - description

Re getDefinitionForKey() - could config_translation use it? And I wonder if this could be generalised for all of type data?

fago’s picture

fago
Primary language German
Location Vienna
commented

This looks like great DX improvement, and improves the schema at the same time!

For typed data, I think the schema 'description' of config should be mapped to the typed-data definition description entry, what afaics already works in the MR. Still, I'd suggest to cover this in tests also.

One thought though: Often a description of a property/data-value is not necessarily the same value you might want to have in the form. The person describing a property in the schema, might describe in from a data perspective, not from a UX/form perspective. Anyway, I think this is a good default + can still be overridden on the form level if it really shall differ. So this is no blocker here imo.