Problem/Motivation
Enum values do not have (translatable) labels.

We need human-readable equivalents, and those equivalents must be translatable (using Drupal's interface translation mechanism).
Precedent:
- Core: #3471494: Add an icon management API
- The https://www.drupal.org/project/ui_patterns contrib module:
\Drupal\ui_patterns\EnumTrait::getEnumOptions()
Steps to reproduce
The core/modules/system/tests/modules/sdc_test/components/my-banner/my-banner.component.yml SDC contains:
ctaTarget:
title: CTA Target
type: string
enum:
- ''
- _blank
👆 Clearly the supported '' enum value is impossible to generate a sensible human-readable label for.
Proposed resolution
Change that example to:
ctaTarget:
title: CTA Target
type: string
enum:
- ''
- _blank
meta:enum:
'': 'Same window'
_blank: 'Open in new window'
x-translation-context: "Link target" (or omitted and use default empty context)
… and which if accessed through some TBD API should be passed through Drupal's UI translation mechanism, like so:
\Drupal\Core\StringTranslation\StringTranslationTrait::t(
string: 'Same window,
options: ['context' => 'Link target'],
)
Which would then result in Hetzelfde venster in Dutch (nl) or Même fenêtre in French (fr).
Naming things is hard context:
We took into account resources like https://github.com/adobe/jsonschema2md, and blogposts from the jsonchema team like https://json-schema.org/blog/posts/the-last-breaking-change (among other links) to decide:
- We use
meta:enum, because even not future-proof, it's an existing convention other projects are adopting. Devs might be already familiar with them. - We use
x-translation-context, which is future-proof as the x- namespace is protected. We didn't find any existing conventions or work around translating those enums.
Remaining tasks
- INFRA:
Updatemeta:enum will be optional, and default to the enum value if not present\Drupal\Core\Theme\Component\ComponentMetadata::parseSchemaInfo()to trigger a deprecation error when anenumis encountered without a correspondingmeta:enum - INFRA: Update
\Drupal\Core\Theme\Component\ComponentMetadata::parseSchemaInfo()to trigger a\LogicExceptionwhen ameta:enumis encountered whose keys do not match (i.e. are a subset or superset) the values listed in the correspondingenum. - Compliance: Update
sdc_test:my-bannerlike the above. Update all other core SDCs where it makes sense to provide ameta:enumandx-translation-context. - INFRA: add a
getEnumOptions(string $prop): array<string, TranslatableMarkup>method to\Drupal\Core\Theme\Component\ComponentMetadatainspired byui_patterns'\Drupal\ui_patterns\EnumTrait::getEnumOptions(). (The thing thatui_patternsdoes not yet do is pass it through Drupal'st().) - TEST: kernel test asserting that 2 identical
type: string, enum: […]props can have different translations for the same enum values if provided different x-translation-context. For example: a''enum value results inSame windowfor thetargetprop and inNonefor arelprop.
→ verifies it works end-to-end, and supports translation contexts - TEST: expand the previous point's test to test at least one language other than English.
- TEST: kernel test asserting that a
meta:enumnot matching theenumtriggers a\LogicException
User interface changes
None.
Introduced terminology
None.
API changes
- Every
enumSDC prop now supports ameta: enum - Every
enumSDC prop that has ameta: enumrequires them to be in sync, otherwise an exception is thrown.
Data model changes
None.
Release notes snippet
Single-Directory Components with props that have a restricted set of allowed values (using enum) now can specify human-readable labels for each of those allowed values. Those labels are also translatable using Drupal's user interface translation subsystem, and can provide specific translation context via x-translation-context property. This allows projects using Single-Directory Components (like Experience Builder) to better support translatability.
| Comment | File | Size | Author |
|---|---|---|---|
| #43 | 3493070-nr-bot.txt | 91 bytes | needs-review-queue-bot |
| #3 | Screenshot 4.png | 58.22 KB | griffynh |
Issue fork drupal-3493070
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:
- 3493070-sdc-metaenum
changes, plain diff MR !11791
Comments
Comment #2
griffynh commentedComment #3
griffynh commentedComment #4
pdureau commentedFor your information, in UI Patterns 2, we use "meta:enum" which is not an official standard but supported by some popular projects:
If an item is in
enumbut not inmeta:enum, its label will be the item stringIf an item is in
meta:enumbut not inenum, it is ignored.It would be great to stay compatible.
Comment #5
nagwani commentedComment #6
wim leersComment #7
wim leers@pdureau You're an SDC maintainer now, so … let's just get #4 implemented? 😄 Happy to provide reviews!
Comment #8
pdureau commentedSo, we have agreed on using
meta:enumand leveraging this information and the related translations (fromlocalemodule API) is up to the display building tools like UI Patterns 2 and Experience Builder.Also, adding
meta:enumto the documentation will be done in this issue #3484727: Complete and clarify SDC documentation.So, what can we do in Core?
meta:enumin ComponentMetadata? Ans some light logic?meta:enumto some of our test componentsmeta:enum? which one?Comment #9
mradcliffeI 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.
Comment #10
wim leers#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:
— https://github.com/adobe/jsonschema2md/blob/f3b5773eb610130891503c1cf71b...
So let's use that one (or a variation thereof).
By the way, #3471494: Add an icon management API also used
meta:enum:Comment #11
wim leersUpdated issue summary per @pdureau's #8.
Expanded it to a full implementation plan. Which is why it's clear this is definitely not .
Comment #12
wim leersComment #13
wim leersComment #16
penyaskitoComment #17
nagwani commentedComment #18
nagwani commentedComment #19
wim leersThis is looking GREAT! 🤩
Comment #20
wim leers@pdureau: as an SDC maintainer, what do you think about https://git.drupalcode.org/project/drupal/-/merge_requests/11791/diffs#n...?
Comment #21
penyaskitoAdded an end-to-end test (as a kernel test). This is ready from my POV.
The test failure is happening in HEAD too (edit: #3461309: Refactor FormTestClickedButtonForm::buildForm)Comment #22
wim leersFYI: Ran into @pdureau at DDD yesterday — he +1'd the implementation plan I added to the issue summary in #11 😊
Comment #23
wim leersI 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.
Comment #24
penyaskitoComment #25
penyaskitoFixed everything from Wim review, added change record draft.
Comment #26
larowlanComment #27
larowlanI think this is ready
Comment #29
longwaveComment #30
longwaveAdded some questions to the MR.
Comment #31
penyaskitoComment #32
longwaveDiscussed this with @penyaskito and @effulgentsia. We came up with two alternative options; we can still bikeshed over the exact names but the JSON Schema spec has no opinion on how to handle extensions, so we need to make our own decision here.
meta:enumfor human readable enum values, and some othermeta:prefix key for translation contexts.x-prefixes and where we addedx-formatting-contextto Experience Builder, we use something likex-enum-labelsfor human readable enum values andx-translation-contextsfor translation contexts.@pdureau (or anyone else!) do you have any strong preferences or guidance on this?
Comment #33
pdureau commentedI would prefer a policy where we avoid as much custom JSON schema annotation as possible:
x-formatting-contextis 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:x-annotationsComment #34
effulgentsia commentedBut 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:enumgiven 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 anx-prefix given JSON schema's move away from unknown keywords not prefixed with that. Perhapsx-translation-context?Comment #35
effulgentsia commentedMy reply to this is tangent to this issue, but I do want to note that I disagree with this statement. 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 dictatesx-formatting-context: inlinefor that prop.Comment #36
effulgentsia commentedMaybe 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?
Comment #37
pdureau commentedYou mean a slot? 😉
Comment #38
penyaskitoThanks everyone for the reviews and the conversations.
Given all the arguments, I'm gonna continue with the implementation with these in mind:
x-translation-contextat 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:
Comment #39
penyaskitoUpdate IS with latest discussions agreements.
Comment #40
penyaskitoComment #41
longwave+1 for the decision in #38, given we have seen prior art in using
meta:enumbut that also thex-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?
Comment #42
penyaskitoNeeds review again.
Comment #43
needs-review-queue-bot commentedThe 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.
Comment #44
penyaskitoDon't know why the bot complained, gitlab didn't, and rebase was automatic:
Comment #45
penyaskitoRebased again, guess too late for 11.2.0 now tho.
Comment #46
larowlanDiscussed 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
metavalues incorrectly.Comment #48
xjmSpecifically, 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!
Comment #50
xjmMeant to credit Lauri also for challenging whether it was still disruptive.
Comment #51
danielvezaLeft a review, mainly around the patterns for the meta:enum property and some small test questions
Comment #52
larowlanIssue credits
Comment #53
danielvezaThis 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.
Comment #56
larowlanCommitted to 11.x and backported to 11.2.x per #48
Published the change record. Thanks everyone 💙
Comment #58
xjmImproving the release note to link the CR and explain the importance of the change in the broader context.
Comment #59
pdureau commentedThere is something wrong with this addition mentioned in #46 (2 hours before the merge):
It forces component authors to add
meta:enumitems even for items which doesn't need a label distinct from the value.For example:
Or:
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:
enum,enum) tometa:enummeta:enumitems not found inenum(if not already the case) because the truth is inenumnotmeta:enumComment #60
pdureau commentedFollow-up issue created #3528998: Follow-up: SDC `enum` props should have translatable labels: use `meta:enum`
Comment #61
pdureau commentedThere may be something else worrying in the merged commit.
Instead of staying in the schema and just working on
metaandmeta:enumproperties in a prop definition by:metaandmeta:enumkeys (something not initially planned, but added to the MR and now the subject of #3528998: Follow-up: SDC `enum` props should have translatable labels: use `meta:enum`)meta:enum(the initial and main purpose of this ticket)The MR is creating a new
metakey in the return values ofComponentMetadata::normalize()which is partially reproducing the JSON schema (the$meta['properties'][$prop_name]part) without being JSON schema compliant and presenting a diverging state of the prop.Would it be way simpler and safer to work on the existing JSON schema data structure and just manipulating the
meta:enumvalues?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.
Comment #62
pdureau commentedComment #63
penyaskito#59.1: that was point 2 of the issue summary proposed resolution since comment #11, 2 months ago and pre-Atlanta.
Comment #64
pdureau commentedhi @penyakisto,
Indeed, comment #11 has expanded the consensus with an helpful implementation plan which is mentioning such Exception at step 2. Your implementation followed this plan carefully and that's great.
However, this specific step is causing troubles. Thankfully, removing this part 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.
Comment #65
pdureau commentedThe new proposal will be published in #3528998: Follow-up: SDC `enum` props should have translatable labels: use `meta:enum` and will have those differences with the Wim's implementation plan.
\Drupal\Core\Theme\Component\ComponentMetadata::parseSchemaInfo()to trigger a\LogicExceptionwhen ameta:enumis encountered whose keys do not match (i.e. are a subset or superset) the values listed in the correspondingenum.sdc_test:my-bannerlike the above. Update all other core SDCs where it makes sense to provide ameta:enumandx-translation-context.getEnumOptions(string $prop): array<string, TranslatableMarkup>method to\Drupal\Core\Theme\Component\ComponentMetadatainspired byui_patterns'\Drupal\ui_patterns\EnumTrait::getEnumOptions(). (The thing thatui_patternsdoes not yet do is pass it through Drupal'st().)type: string, enum: […]props can have different translations for the same enum values if provided different x-translation-context. For example: a''enum value results inSame windowfor thetargetprop and inNonefor arelprop.→ verifies it works end-to-end, and supports translation contexts
TEST: expand the previous point's test to test at least one language other than English.
metakey was removed from the return values ofComponentMetadata::normalize()to not be injected in the template because, as confessed by the current change notice, this must not be used in templates.meta:enumnot matching theenumtriggers a\LogicExceptionEverything else will be kept.
Comment #66
penyaskitoI 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.
Comment #67
pdureau commentedWe moved the discussion and the proposal to #3528998: Follow-up: SDC `enum` props should have translatable labels: use `meta:enum` to avoid messing with this already merged issue.
Comment #68
pdureau commentedJSON schema has 2 kinds of properties:
type,pattern,format,enum...title,description,examples...meta:enumis ignored by the JSON Schema validator for 2 reasons:So I believe we are good.
Comment #69
larowlanMoving status back
Comment #70
xjm