Problem/Motivation

#3379725: Make Block config entities fully validatable accidentally made the label_display configuration value of blocks translatable. It is not a user-facing value, however, it should always be the string "visible" regardless of the configuration language.

It is currently just configured via a checkbox, so there is an attempt to change this into a boolean type over at #2544708: The `label_display` setting in `type: block_settings` has the wrong config schema type, but that requires updating all exported block configuration so is a complex issue.

Also, as was brought up there, there is an attempt to add a second possible value (or counting the empty value "0" a third value) over at #2614950: Add option for visually-hidden block titles which would make having it as a string actually make sense.

So this issue is just about fixing the regression introduced by #3379725: Make Block config entities fully validatable so that the value does not get incorrectly translated.

Steps to reproduce

  1. Install a site in German
  2. Export configuration

Observe that the exported blocks will have a "label_display" value of "sichtbar" instead of "visible".

Proposed resolution

  1. Change the schema type from "label" back to "string"
  2. To not lose the intent of what #3379725: Make Block config entities fully validatable was trying to achieve add a validation constraint that makes sure that only "0" and "visible" are allowed as values

Remaining tasks

User interface changes

-

Introduced terminology

-

API changes

-

Data model changes

The label_display configuration is now (again) of type "string" with a proper constraint. (Since #3379725: Make Block config entities fully validatable did not include a change notice for the change from "string" to "label" I assume this issue also does not need a change notice.)

Release notes snippet

Issue fork drupal-3547808

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

tstoeckler created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Title: label_display configuration should not be translatable » label_display block configuration value should not be translatable
Issue summary: View changes

tstoeckler opened merge request !13287

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Configuration schema, +validation

I would very much like to see the other issue go in instead, this should be a boolean.
It also indicates that we're missing some kind of testcoverage, since the steps to reproduce are very easy.

However, to me it seems like this is a regression that we can easily fix in this issue, because the other issue has been going for a very long time and I don't see that landing quickly.

Adding the right tags to this issue to categorize it in config schema validation part.

astonvictor’s picture

astonvictor commented

thanks for the MR. works for me
+1 RTBC

  • longwave committed 871d7f1c on 11.3.x 
    fix: #3547808 label_display block configuration value should not be...

  • longwave committed b59cf672 on 11.x 
    fix: #3547808 label_display block configuration value should not be...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

I checked some sites that I work on and they have either "visible" or "0" as the label_display value so while that's kinda odd, it's better to be consistent as done in this MR. I agree that fixing the actual value can be done in a followup and this just solves the regression in multilingual sites.

Committed and pushed b59cf67292f to 11.x and 871d7f1c070 to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !13287

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

This change caused test failures in Canvas. Arguably, this should have had a change record? Should we still add one to help the next contrib module that is impacted by this?