Every XB `Component` config entity should have a `category` property

What is this postponed on? On design? On an outcome of the discussion at #3455036: Clarify "components" vs "elements" vs "patterns"?

This affects #3444424: [META] Configuration management: define needed config entity types and would be one of the logical next things to work on now that #3452397: Allow specifying default props values when opting an SDC in for XB landed.

This came up in a meeting with @larowlan — see the parenthetical at the bottom of #3454519-22: [META] Support component types other than SDC, block, and code components, quoted here for simplicity:

(For example, we also discussed the need for per-slot restrictions on which components would be allowed, which is possible in Layout Builder using https://www.drupal.org/project/layout_builder_restrictions, but that module has scalability issues — through no fault of its own — because the "tags" are not built in, resulting in hundreds of lines of YAML in third_party_settings 😳. That validates @lauriii's thinking to make that an inherent part — see #21.4. Related: #3459088: Every XB `Component` config entity should have a `category` property.)

Lee confirms the need for this based on real-world experience with Layout Builder 👍

Matching the issue title prefix pattern I used for #3455036: Clarify "components" vs "elements" vs "patterns".

IIRC @lauriii said that user testing around this has happened? Can we get the results posted here? 🙏

AFAICT this is necessary for #3455753: Milestone 0.2.0: Early preview because it's implied to be necessary for Place blocks as components?

I thought we had an issue for how the design system can specify tags/categories (in addition to group) so that the design system components are organized in a particular way.

I struggled to find such an issue and so maybe it was this issue I was remembering. But, I don't think this issue is focused on this topic, but rather a "level up" in terms of categorizing.

I know we had some discussions in Slack and in other issues about grouping/tagging/categorizing components... do we need a new issue for it? Or should it be part of this issue or part of another existing issue?

This is in progress, I am getting this issue ready for handoff for next weeks call!

🥳

FYI: you can get preliminary feedback from people interested in this area by sharing Figma links ahead of that meeting. That'd make that meeting more productive too! 😄

(This information needs to be consumable by the community at large anyway, i.e. also be understandable by people outside that meeting.)

Very excited to see the designs!

Based on this, the key separation is the source of the components; module vs theme. The module provided components are listed under "Extensions" and theme provided components are listed in the sidebar "Library".

This will be implemented in #3482393: HTTP API: update /xb-components to specify each `Component`'s `source`.

Inside "Extensions" and "Library", the categories are defined by components and there are no built-in categories there. These can ideally leverage #3474533: ComponentPluginManager must implement CategorizingPluginManagerInterface.

So that's the categories for SDCs — and each SDC author can freely choose these. That does not yet exist. It will need to be implemented upstream (in Drupal core) in #3474533: ComponentPluginManager must implement CategorizingPluginManagerInterface (as identified in #3462705: [META] Missing features in SDC needed for XB).

There's also no built-in components.

This is a big pivot/conclusion!

There are Blocks which are exposed under "Dynamic Components".

Not stated by @lauriii but equally important: "Dynamic Components" aka Blocks also have categories, and block plugins already implement that very same interface: CategorizingPluginManagerInterface — because Block plugin authors have been able to freely choose a category for a decade now!

There is also a special category "Elements" in future which is reserved for Experience Builder. It allows XB to provide SDCs that are more close to web standards / HTML tags or design system atoms.

So the Elements category is going to be reserved, and any SDC (or block) that uses that category will not get an XB Component config entity, and will hence not be available in XB.

Rather than closing this (given: no built-in components, and no built-in categories, so in theory: nothing to do), I'd like to rescope this, to: Every XB Component config entity should have a category property.

Did you link to the wrong issue?

https://www.drupal.org/project/experience_builder/issues/3482393

Is in XB, but your post suggests this is being solved upstream in core, but I couldn’t find any issue.

Edit: Nor in the meta, unless I’m just missing Things

Discussed with @lauriii and @jessebaker, they +1'd #18.

This should happen after #3475584: Add support for Blocks as Components.

Attached is a patch that provides a rough outline of how to implement this. I think @f.mazeikis is the right person to implement this after he wraps up #3475584.

@reneelund: It'd be great if you could update the issue summary to link to the findings of the various rounds of user testing 🙏

This is an outline of what I expect this MR to look like, but given there's 2 blockers, working on an actual MR right now is premature.

#3475584: Add support for Blocks as Components is in.

Added some comments and an alternative approach to #3474533: ComponentPluginManager must implement CategorizingPluginManagerInterface

The related #3482393: HTTP API: update /xb-components to specify each `Component`'s `source` is in — this is next. But we're still blocked on #3474533: ComponentPluginManager must implement CategorizingPluginManagerInterface. We need to help land that core issue first.

We are already extending ComponentPluginManager so if we do that in an identical way we don't need to wait for upstream.

First pass seems to work, no test coverage though.

LGTM except for the config schema not matching the PHP definition of the Component config entity.

That is where test coverage is missing: a ComponentTest::testCategory() with a @testWith to test 3 possible categories should surface the bug I identified:

• ''
• NULL
• 'foo'

Thanks, I'm learning more about config schema from you in every MR!

This was AFAICT only failing due to upstream changes, not due to this MR itself. #3480805: CI: update to >=1.6.3 of the GitLab CI Template is in now, merged in origin/0.x, let's find out :)

I wanted to write the missing docs (updating docs/components.md) for you and merge this. But then I spotted something that doesn't quite add up 🙈

Actually, \Drupal\Core\Plugin\CategorizingPluginManagerTrait::processDefinitionCategory() does this:

  protected function processDefinitionCategory(&$definition) {
    // Ensure that every plugin has a category.
    if (empty($definition['category'])) {
      // Default to the human readable module name if the provider is a module;
      // otherwise the provider machine name is used.
      $definition['category'] = $this->getProviderName($definition['provider']);
    }
  }

… so category: null essentially never happens. Not for Block-sourced Components (since BlockManager uses the above trait) nor for SDC-sourced Components (since this MR is updating ComponentPluginManager to use the above trait, but using $this->t('Other') as the fallback value instead), so only for theoretical/future other component types (see #3454519: [META] Support component types other than SDC, block, and code components).

So … why exactly are we trying to support category: null in the XB Component config entity? 🤔

I don't understand how to work around

1) Drupal\Tests\experience_builder\Kernel\Entity\ComponentValidationTest::testRequiredPropertyValuesMissing
TypeError: Cannot assign null to property Drupal\experience_builder\Entity\Component::$category of type Drupal\Core\StringTranslation\TranslatableMarkup|string

Do we just need to override the entire method here? There doesn't seem to be a technique available in the parent testRequiredPropertyValuesMissing() to solve this; because core doesn't use types on config entity properties this doesn't seem to affect any core tests.

Addressed #35 by making the property nullable but the config schema still disallows it.

@Wim up to you whether to do it this way or revert and fix the test instead.

Arghhhhhhh that dreaded flaw in all config entities keeps causing pain 😬

Last time I ran into this: \Drupal\experience_builder\Entity\Pattern::$component_tree — Pattern config entities MUST have a component tree, but due to the way config entities work, the only choice we have is: make a required config entity property nullable at the PHP class level, while making it required at the config schema level. 😬

So I'm 90% happy with what you did here, the only change I want to make is that the ::getCategory() method cannot ever return NULL — because all of XB's config entities are validated, and validation would have caught the absence of a category thanks to the config schema!

Tada: what we want and need, but in a less-than-ideal (yet pragmatic) fashion.

Did that.

Also added the missing docs.