Enum vales do not have translatable labels

Created on 11 December 2024, 5 months ago

Problem/Motivation

'enums values do not have (translatable) labels'

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Feature request
Status

Active

Version

11.0 🔥

Component

single-directory components

Created by

🇦🇺Australia griffynh Sydney

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @griffynh
  • 🇦🇺Australia griffynh Sydney
  • 🇦🇺Australia griffynh Sydney
  • 🇫🇷France pdureau Paris

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

    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.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

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

  • 🇫🇷France pdureau Paris

    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?
  • 🇺🇸United States mradcliffe USA

    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.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #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 {
    
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Updated issue summary per @pdureau's #8.

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

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • First commit to issue fork.
  • Pipeline finished with Failed
    about 1 month ago
    Total: 99s
    #470051
  • Pipeline finished with Failed
    about 1 month ago
    Total: 162s
    #470126
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
  • Pipeline finished with Failed
    about 1 month ago
    Total: 100s
    #473005
  • Pipeline finished with Failed
    about 1 month ago
    Total: 114s
    #473018
  • Pipeline finished with Failed
    about 1 month ago
    Total: 100s
    #473043
  • Pipeline finished with Failed
    about 1 month ago
    Total: 569s
    #473046
  • Pipeline finished with Failed
    about 1 month ago
    Total: 633s
    #473063
  • Pipeline finished with Failed
    about 1 month ago
    Total: 115s
    #473093
  • Pipeline finished with Success
    about 1 month ago
    Total: 1216s
    #473104
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This is looking GREAT! 🤩

  • Pipeline finished with Canceled
    about 1 month ago
    Total: 541s
    #473184
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 224s
    #473195
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 137s
    #473200
  • Pipeline finished with Success
    about 1 month ago
    Total: 608s
    #473201
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

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

  • Pipeline finished with Success
    30 days ago
    Total: 339s
    #473757
  • Pipeline finished with Failed
    30 days ago
    Total: 194s
    #473935
  • Pipeline finished with Failed
    30 days ago
    Total: 118s
    #473943
  • Pipeline finished with Canceled
    30 days ago
    Total: 149s
    #473948
  • Pipeline finished with Failed
    30 days ago
    Total: 795s
    #473954
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

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

  • Pipeline finished with Success
    30 days ago
    Total: 1730s
    #474036
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

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

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    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.

  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

    Fixed everything from Wim review, added change record draft.

  • Pipeline finished with Failed
    28 days ago
    Total: 1061s
    #475304
  • Pipeline finished with Success
    28 days ago
    Total: 363s
    #475461
  • Pipeline finished with Canceled
    22 days ago
    Total: 93s
    #479633
  • Pipeline finished with Success
    22 days ago
    Total: 2258s
    #479634
  • Pipeline finished with Canceled
    22 days ago
    Total: 410s
    #479694
  • Pipeline finished with Canceled
    22 days ago
    Total: 82s
    #479696
  • Pipeline finished with Canceled
    22 days ago
    Total: 72s
    #479697
  • Pipeline finished with Failed
    22 days ago
    Total: 163s
    #479698
  • Pipeline finished with Success
    22 days ago
    Total: 1731s
    #479699
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    I think this is ready

  • 🇺🇸United States phenaproxima Massachusetts
  • 🇬🇧United Kingdom longwave UK
  • 🇬🇧United Kingdom longwave UK

    Added some questions to the MR.

  • Status changed to Needs work 9 days ago
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
  • 🇬🇧United Kingdom longwave UK

    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
    3. for translation contexts.

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

  • 🇫🇷France pdureau Paris

    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
  • 🇺🇸United States effulgentsia

    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?

  • 🇺🇸United States effulgentsia

    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.

  • 🇺🇸United States effulgentsia

    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?

  • 🇫🇷France pdureau Paris

    an HTML-containing prop

    You mean a slot? 😉

  • Pipeline finished with Failed
    8 days ago
    Total: 503s
    #491367
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

    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"
    
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

    Update IS with latest discussions agreements.

  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
  • 🇬🇧United Kingdom longwave UK

    +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?

  • Pipeline finished with Failed
    7 days ago
    Total: 121s
    #491572
  • Pipeline finished with Canceled
    7 days ago
    Total: 139s
    #491601
  • Pipeline finished with Failed
    7 days ago
    Total: 253s
    #491603
  • Pipeline finished with Canceled
    7 days ago
    Total: 638s
    #491620
  • Pipeline finished with Canceled
    7 days ago
    Total: 233s
    #491625
  • Pipeline finished with Canceled
    7 days ago
    Total: 190s
    #491629
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

    Needs review again.

  • Pipeline finished with Failed
    7 days ago
    #491633
  • Pipeline finished with Success
    7 days ago
    Total: 2542s
    #491653
  • 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.

  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

    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.
    
  • Pipeline finished with Success
    2 days ago
    Total: 693s
    #495084
  • Pipeline finished with Success
    1 day ago
    Total: 786s
    #496431
Production build 0.71.5 2024