Enum vales do not have translatable labels

For your information, in UI Patterns 2 → , we use "meta:enum" which is not an official standard but supported by some popular projects:

• https://github.com/adobe/jsonschema2md
• https://github.com/coveooss/json-schema-for-human

props:
  type: object
  properties:
    position:
      type: string
      enum:
        - top
        - bottom
      "meta:enum":
        top: Top
        bottom: Bottom

If an item is in enum but not in meta:enum, its label will be the item string
If an item is in meta:enum but not in enum, it is ignored.

It would be great to stay compatible.

@pdureau You're an SDC maintainer now, so … let's just get #4 implemented? 😄 Happy to provide reviews!

So, we have agreed on using meta:enum and leveraging this information and the related translations (from locale module API) is up to the display building tools like UI Patterns 2 and Experience Builder.

Also, adding meta:enum to the documentation will be done in this issue 🌱 Clarify SDC documentation by toning down Twig blocks promotion Active .

So, what can we do in Core?

• Add a mention of meta:enum in ComponentMetadata? Ans some light logic?
• Add meta:enum to some of our test components
• Add a specific test about meta:enum? which one?

I performed Novice Triage on this issue. I added the Novice issue tag because we can update the issue summary and potentially start. We need to come up with a good test for the change.

#4 referenced https://github.com/adobe/jsonschema2md, so I went to look for an example there, and found one that's sufficiently silly to be fun:

    "string_pattern": {
      "type": "string",
      "description": "A string following a regular expression",
      "pattern": "^ba.$",
      "examples": ["bar", "baz", "bat"],
      "meta:enum": {
        "baa": "the sounds of sheeps",
        "bad": "German bathroom",
        "bag": "holding device",
        "bah": "humbug!",
        "bam": "a loud sound",
        "ban": "don't do this",
        "bap": "a British soft bread roll",
        "bas": "from ancient Egyptian religion, an aspect of the soul",
        "bat": "…out of hell",
        "bay": ", sitting by the dock of the"
      },

— https://github.com/adobe/jsonschema2md/blob/f3b5773eb610130891503c1cf71b...

So let's use that one (or a variation thereof).

By the way, ✨ Add an icon management API Active also used meta:enum:

 * If an `enum` is set, then the select is used:
 * - enum => #type = select and #options
 * The key `meta:enum` is used to support description for each enum.
 *
 * @internal
 *   This API is experimental.
 */
class IconExtractorSettingsForm {

Updated issue summary per @pdureau's #8.

Expanded it to a full implementation plan. Which is why it's clear this is definitely not .

This is looking GREAT! 🤩

@pdureau: as an SDC maintainer, what do you think about https://git.drupalcode.org/project/drupal/-/merge_requests/11791/diffs#n...?

Added an end-to-end test (as a kernel test). This is ready from my POV.
The test failure is happening in HEAD too.

FYI: Ran into @pdureau at DDD yesterday — he +1'd the implementation plan I added to the issue summary in #11 😊

I think this is in the final stretch now! This needs a change record, plus 2 clarifications, plus 1 code clarity nit by Lee that I +1'd.

Fixed everything from Wim review, added change record draft.

I think this is ready

longwave → credited phenaproxima → .

Added some questions to the MR.

Discussed this with @penyaskito and @effulgentsia. We came up with two alternative options; we can still bikeshed over the exact names but JSON Schema has no opinion on how to handle extensions, so we need to make our own decision here.

1. As proposed by @pdureau in #4 we use meta:enum for human readable enum values, and some other meta: prefix key for translation contexts.
2. Or, as per this JSON Schema blog post where we added x-formatting-context to Experience Builder 📌 `StringSemanticsConstraint::MARKUP`: agree how SDC prop JSON schema can convey it should be markup, and Needs review we use something like x-enum-labels for human readable enum values and x-translation-contexts

for translation contexts.

@pdureau (or anyone else!) do you have any strong preferences or guidance on this?

I would prefer a policy where we avoid as much custom JSON schema annotation as possible:

• They are creating our own "island" in the JSON schema world
• They are tempting to be used as convenient workarounds on the SDC side instead of fixing the root cause in the application side.
  x-formatting-context is an example of such workaround, because we are asking the component author to add an annotation related to the UX of a specific display building tool he doesn't have to care about, instead of being focused only on its component's own UI model and logic.

That's why I like meta:enum. Yes, it is not part of the JSON schema specs, however:

• it is not something we have invented, it is already used in opens source tools
• it is still describing the component and not some unrelated applicative stuff
• it looks like something special, we are not opening a Pandora box by normalizing x- annotations
• it looks like it can be included in a future version of JSON schema

That's why I like meta:enum. Yes, it is not part of the JSON schema specs, however...it is not something we have invented, it is already used in opens source tools

But is that future-compatible considering https://json-schema.org/blog/posts/the-last-breaking-change? When a stable json schema version is released and tools adopt it, are we expecting https://github.com/adobe/jsonschema2md to provide the vocabulary/schema for it, and then we'll change all of our SDCs to reference it?

If we do stick with meta:enum given its prior art, then what about translation context? If that's one that we are inventing here and not copying from other OSS tools, then I think that one needs an x- prefix given JSON schema's move away from unknown keywords not prefixed with that. Perhaps x-translation-context?

x-formatting-context is an example of such workaround, because we are asking the component author to add an annotation related to the UX of a specific display building tool he doesn't have to care about, instead of being focused only on its component's own UI model and logic

My reply to this is tangent to this issue, but I do want to note that I disagree with this characterization. If an SDC defines an HTML-containing prop, and then has Twig code that renders that prop inside a <p> tag or has CSS that assumes that what's in that prop is only inline formatted content, then it's the component's own UI logic that dictates x-formatting-context: inline.

and then we'll change all of our SDCs to reference it?

Maybe that won't be necessary since SDCs reference the schema in Drupal core, so we'll be able to update just that central one when the time comes?

an HTML-containing prop

You mean a slot? 😉

Thanks everyone for the reviews and the conversations.

Given all the arguments, I'm gonna continue with the implementation with these in mind:

• We want to use "meta:enum", even if not very future-proof, for consistency with other frameworks frontend devs might be familiar with.
• "meta:enum" shouldn't be required. We default to the enum keys if not provided.
• We will add x-translation-context at the prop level. We will take into account this might get promoted to the component level, so we want to "cascade". If no translation context found, we use an empty string.

So an example of an enum prop would be:

      enum:
        - info
        - success
      meta:enum:
        info: Information
        success: Success
      x-translation-context: "Status icon"

Update IS with latest discussions agreements.

+1 for the decision in #38, given we have seen prior art in using meta:enum but that also the x- prefix is seemingly preferred by the JSON Schema team for extensions.

I think cascading the context makes sense too. It seems likely that you would want the same context to be applied across all translatable strings in the prop (and certainly all cases in the enum), but you might also want it to cover the whole component without having to repeat yourself.

@pdureau are you OK with this?

Needs review again.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Don't know why the bot complained, gitlab didn't, and rebase was automatic:

$ git rebase 11.x
Successfully rebased and updated refs/heads/3493070-sdc-metaenum.

Rebased again, guess too late for 11.2.0 now tho.

Discussed with release manager if this could still make it into 11.2 because of the strategic importance.

There are some new exceptions thrown but the change is not disruptive - it only occurs if you opt in to the new feature but declare your meta values incorrectly.

        if (isset($prop_schema['enum'], $prop_schema['meta:enum'])) {
          $enum_keys_diff = array_diff($prop_schema['enum'], array_keys($prop_schema['meta:enum']));
          if (!empty($enum_keys_diff)) {
            throw new InvalidComponentException(sprintf('The values for the %s prop enum in component %s must be defined in meta:enum.', $name, $this->id));
          }
        }

xjm → credited catch → .

Specifically, while a change like this would normally need to be committed before 11.2.0-beta1, we're willing to allow it up until 11.2.0-rc1 given the impact on the XB release cycle and the fact that this was made into a minimally disruptive API addition per #46.

However, if this isn't fixed before RC1, it will still have to wait for 11.3 unfortunately, because it is still a minor-only feature and API addition. RC1 is scheduled for this week. :) @larowlan can hopefully keep the RMs updated on where we're at with this issue up until the RC is tagged. Thanks all!

xjm → credited lauriii → .

Meant to credit Lauri also for challenging whether it was still disruptive.

Left a review, mainly around the patterns for the meta:enum property and some small test questions

Issue credits

This issue has gone through a number of comprehensive reviews, the tests are green and all comments from my most recent review have been fixed or commented on.

I think this is ready to be in RTBC.

Committed to 11.x and backported to 11.2.x per #48

Published the change record. Thanks everyone 💙

larowlan → closed merge request !11791

Improving the release note to link the CR and explain the importance of the change in the broader context.

There is something wrong with this addition mentioned in #46 ✨ Enum vales do not have translatable labels Active (2 hours before the merge):

        if (isset($prop_schema['enum'], $prop_schema['meta:enum'])) {
          $enum_keys_diff = array_diff($prop_schema['enum'], array_keys($prop_schema['meta:enum']));
          if (!empty($enum_keys_diff)) {
            throw new InvalidComponentException(sprintf('The values for the %s prop enum in component %s must be defined in meta:enum.', $name, $this->id));
          }
        }

It forces component authors to add meta:enum items even for items which doesn't need a label distinct from the value.

For example:

 type: string
 enum: ["auto", "1", "2"]
  meta:enum:
    "auto": "Automatic"

Or:

  type: string
  enum: ["ul", "ol"]
  "meta:enum":
    ul: "ul (Default)"

There are some new exceptions thrown but the change is not disruptive - it only occurs if you opt in to the new feature but declare your meta values incorrectly.

I am afraid this decision is both making the component author life more difficult and breaking the projects already using this feature in a legit way.

So, I will create a follow-up issue for 11.2, proposing:

• the removal of this exception
• the merge of array_combine(enum, enum) to meta:enum to be sure all
• the removal of meta:enum items not found in enum (if not already the case)

Follow-up issue created 🐛 Don't raise exception when an enum value is missing from meta:enum Active

There may be something else worrying in the MR.

Instead of staying in the schema and just working on meta and meta:enum properties in a prop definition by:

• checking and aligning meta values with meta:enum keys (something not initially planned, but added to the MR and now the subject of 🐛 Don't raise exception when an enum value is missing from meta:enum Active )
• translating the values of meta:enum (the initial and main purpose of this ticket)

The MR is creating a new and confusing meta key in the return values of ComponentMetadata::normalize() which is partially reproducing the JSON schema (the$mata ['properties'][$prop_name] part) without being JSON schema compliant and presenting a diverging state of the prop.

Would it be simpler and safer to work on the existing JSON schema data structure and just manipulating the meta:enum values?

It will also prevent the "not good practice" use case mentioned in the change notice: https://www.drupal.org/node/3519574 →

So, I am not sure we can keep this commit (and the related change notice) in the 11.x and 11.2.x branches in this current state.

#59.1: that was point 2 of the issue summary proposed resolution since comment #11, 2 months ago and pre-Atlanta.

hi @penyakisto,

Indeed, comment #11 has expanded the consensus with an helpful implementation plan with mention of such Exception at step 2. Your implementation followed this plan carefully and that's great.

However, this step is causing troubles. Thankfully, removing its implementation is a simple and targeted alteration of the work which has been merged. No big deal.

However, the change proposed in comment #61 may have more impact.

The new proposal will be published in 🐛 Don't raise exception when an enum value is missing from meta:enum Active and will have those differences with the Wim's implementation plan.

Everything else will be kept.

I don't think altering the actual metadata is the right thing to do. Specially since json schema can't validate a TranslatableMarkup object.
If we are introducing a Drupalism, as we can't avoid for making them translatable, better to isolate it as the merged MR was doing.

We moved the discussion and the proposal to 🐛 Don't raise exception when an enum value is missing from meta:enum Active to avoid reopening this issue.

I don't think altering the actual metadata is the right thing to do. Specially since json schema can't validate a TranslatableMarkup object.

JSON schema ahs 2 kind of properties

the ones for validation. Example: type, pattern, format, enum...
the ones only for documentation. Example: title, description, examples...

meta:enum is ignored by the JSON Schema validator for 2 reasons:

• it is not an official property
• it is a documentation only property

So I guess we are good.

Moving status back