🇺🇸United States @agentrickard

agentrickard → created an issue.

This is throwing fatal JS errors and breaking contrib modals:

Uncaught TypeError: event is undefined
    handle https://xxxx/core/misc/dialog/dialog-deprecation.js?v=10.3.0-dev:12
    jQuery 7
    openDialog https://xxxx/core/misc/dialog/dialog.js?v=10.3.0-dev:83
    showModal https://xxxx/core/misc/dialog/dialog.js?v=10.3.0-dev:101
    attach https://xxxx/modules/contrib/moderated_content_bulk_publish/js/moderated_content_bulk_publish.js?semf9b:120
    jQuery 2

smustgrave → credited agentrickard → .

Here's a quick patch for initial testing.

I don't think we have an issue here. I could not replicate the bug by clearing the cache.

That is likely because of the inheritance on the service:

domain_config.library.discovery.collector:
    decorates: library.discovery.collector
    class: \Drupal\domain_config\DomainConfigLibraryDiscoveryCollector
    arguments: ['@cache.discovery', '@lock', '@library.discovery.parser', '@theme.manager']
    tags:
      - { name: needs_destruction }
    calls:
      - [setDomainNegotiator, ['@domain.negotiator']]

class DomainConfigLibraryDiscoveryCollector extends LibraryDiscoveryCollector {}

class LibraryDiscoveryCollector extends CacheCollector {}

abstract class CacheCollector implements CacheCollectorInterface, DestructableInterface {}

CacheCollector::destruct() method is inherited by our class.

domain.route.provider seems to be fine.

domain_config.library.discovery.collector will hit this because it extends Drupal\Core\Cache\CacheCollector which calls the DestructableInterface

domain_config_ui.factory seems to be fine.

Quick patch which should be BC.

I am not going to have immediate time to pursue this, but here is a patch for testing. It does the following:

1. Moves the metadata update trait to a service
2. Introduces an event that fires after metadata has been mapped

I started to write default event handlers, but that gets tricky, so it seems "good enough" to allow developers to write their own Event handlers to respond to the metadata update.

8.x is closed.

Yes, the domain issue here is actually https://www.drupal.org/node/3425054 → -- though we still need to check these decorations:

  domain.route_provider:
    class: Drupal\domain\Routing\DomainRouteProvider
    decorates: router.route_provider
    decoration_priority: 10

  domain_config.library.discovery.collector:
    decorates: library.discovery.collector
    class: \Drupal\domain_config\DomainConfigLibraryDiscoveryCollector

  domain_config_ui.factory:
    class: Drupal\domain_config_ui\Config\ConfigFactory
    decorates: config.factory

Those should be two issues in the Domain queue.

Should we do the same for properties, in cases where there are only slots?

Not compatible:

1) SDC module is deprecated and should not be a dependency in the info file
2) The "'plugin.manager.sdc'" service has moved to a new class.

TypeError: Drupal\sdc_styleguide\Form\SDCExplorerForm::__construct(): Argument #1 ($componentPluginManager) must be of type Drupal\sdc\ComponentPluginManager, Drupal\Core\Theme\ComponentPluginManager

These are both fixed in the MR.

I think I get what this is trying to do.

It wants a `my_component.demo.NAME.yml` file for each item to display in the Styleguide.

Two immediate questions:

1. How does it deal with "slots"?
2. How does it deal with objects or complex arrays?

Consider the following component:

name: USWDS Accordion
status: experimental
group: USWDS
props:
  type: object
  properties:
    attributes:
      type: Drupal\Core\Template\Attribute
    title_suffix:
      type: array
      description: title_suffix is necessary if contextual links are included.
    modifier:
      title: Extra CSS classes
      type: string
      description: Extra CSS classes that get added to wrapper element.
    multiselect:
      title: Multiselect
      type: boolean
      description: Allows multiple accordion items to be open at the same time.
slots:
  accordion_items:
    title: Accordion Items

How do we represent that as a demo file?

agentrickard → created an issue.

TypeError: Drupal\sdc_styleguide\Form\SDCExplorerForm::__construct(): Argument #1 ($componentPluginManager) must be of type Drupal\sdc\ComponentPluginManager, Drupal\Core\Theme\ComponentPluginManager

agentrickard → created an issue.

agentrickard → created an issue.

agentrickard → created an issue.

Maybe -- though I haven't run into that yet.

Related to @japerry's comment in #139, I would note that this approach only handles base field values (string, integer, boolean), and there are cases where it would be important to have source data mapped to, say, a taxonomy term reference field.

The current code can't do that, but I would propose a follow-up issue to make it possible using an event handler.

I can work on that over in ✨ Mapping metadata to reference field Active as a proof-of-concept. Posting here to see if there is an existing issue or indeed, a desire for that kind of feature.

So the current metadata refresh looks pretty simple. See MetadataRefreshTrait::forceMappedFieldRefresh.

We should be able to adapt that method a bit to account for entity reference fields. The problem here is that we have to register field lookups.

Consider this case:

• We have a Drupal taxonomy - "File type" that we want to sync from the DAM.
• File type contains "pdf", "doc", "image"
• In Drupal, these are referenced by entity ID
• Acquia DAM doesn't know the entity ID, it probably stores a string
• So we would need a means to lookup the proper entity reference by string

This is all pretty simple if we assume that the Entity Reference is a taxonomy term, but I am not sure that is a safe assumption.

What I would propose is adding an event to the trait which allows for a data lookup on the Drupal side before we resolve the call to $translation->set($entity_field_name, $media_source->getMetadata($translation, $metadata_attribute_name));

Something like this:

  protected function forceMappedFieldRefresh(MediaInterface $media): void {
    $media_source = $media->getSource();
    $translations = $media->getTranslationLanguages();
    foreach ($translations as $langcode => $data) {
      if ($media->hasTranslation($langcode)) {
        $translation = $media->getTranslation($langcode);
        // Try to set fields provided by the media source and mapped in
        // media type config.
        foreach ($translation->bundle->entity->getFieldMap() as $metadata_attribute_name => $entity_field_name) {
          if ($translation->hasField($entity_field_name)) {
            // New pseudo-code here.
            $metadata = $media_source->getMetadata($translation, $metadata_attribute_name);
            // We have to write this event handler, which can use $translation->getFieldDefinition($entity_field_name) to get the target type
            $metadata = $this->eventDispatcher->dispatch(new MappedFieldEvent($translation, $metadata, $entity_field_name));
            $translation->set($entity_field_name, $metadata);
          }
        }
      }
    }
  }

That should provide enough metadata to derive the entity lookup.

I would propose that we write the Event handler and a default implementation, allowing other modules / sites to also respond.

We are also looking at this and it does not appear to be supported, so marking as a Feature Request.

Has this been solved?

Closing in favor of #3300685: Unclear replacement of getMockBuilder() with createMock(). →

We do not use the drupal.org repository, since Rector is not a module and cannot be maintained on the infrastructure.

That is an automated link that cannot be changed.

See the project page for details.

https://www.drupal.org/project/rector →

I do!

Fixed.

This has been fixed and merged.

This isn't really a bug. It's a feature request.

We have to add each deprecation rule as it comes.

Can you be more specific about the change record that should be addressed here? Is it https://www.drupal.org/node/3207439 →

This has been merged.

Merged.

The patch/MR is malformed and has non utf-8 characters, likely "smart" quotes.

+      'children' => [
+        '#markup' => t('Displaying media items of %bundle_name type by referencing their asset via an embed code is not supported by the remote DAM system. Therefore the “Managed File ID” field has been already set to use instead.', [
+          '%bundle_name' => \Drupal::entityTypeManager()->getStorage('media_type')->load($form['#bundle'])->label(),
+        ]),
+      ],

Notes that the SDC module is required for versions lower than 10.3.

Comment 20 -- https://www.drupal.org/project/drupal/issues/3365367#comment-15557100 📌 Convert Olivero's teaser into a single directory component Postponed says that this was merged to 11.x but the issue is now assigned to 10.3.

Is the intent to cherry-pick that commit to 10.3, or is the change accidental?

agentrickard → created an issue.

We're going to close this as part of the development plan to "catch errors early". See 🌱 [Policy] SDC backwards compatibility policy Active . specifically the note "To determine if this policy is feasible, it would be worth SDC-ifiying some core components that would benefit from better default styles."

New ticket for the breadcrumb issue: 📌 SDC: Breadcrumb handling consistency Active

agentrickard → created an issue.

Adding a note about schema validation rules.

Well, except we are trying to use a theme that does have schema validation. The theory makes sense but I'd still love to see a setting that would not trigger a fatal render error.

I see those as breaking changes (deprecations) that have no advance warning, since they can come from the fact that current theming doesn't enforce strict typing and SDC does.

It may be that the plan is to correct all that for Drupal 11, but as it stands, SDC may break randomly when modules are installed.

Note: I will update the docs, too.

Updated title.

This issue has been fixed -- sort of -- in 10.3. However, the enforce_prop_schemas value in a theme.info.yml file only applies if NO schema is present for the component.

Individual validation errors are still thrown if one part of the schema is incorrect.

    $schema = $component->metadata->schema;
    if (!$schema) {
      if ($component->metadata->mandatorySchemas) {
        throw new InvalidComponentException(sprintf('The component "%s" does not provide schema information. Schema definitions are mandatory for components declared in modules. For components declared in themes, schema definitions are only mandatory if the "enforce_prop_schemas" key is set to "true" in the theme info file.', $component_id));
      }
      return TRUE;
    }

As a developer, I would expect the final exception to be wrapped in a similar statement.

      if ($component->metadata->mandatorySchemas) {
        throw new InvalidComponentException($message);
      }

I tend to agree with the assessment that this is a documentation/schema feature and not a means to inject data into Twig, which would make this a documentation issue.

I wonder if this part: generating synthetic example renderings of a component is something that might be implemented by modules like Storybook. If so, we could open a similar issue there.

This may have been corrected when SDC merged to core, since this seems to be new:

    // Dismiss type errors if the prop received a render array.
    $errors = array_filter(
      $validator->getErrors(),
      function (array $error) use ($context): bool {
        if (($error['constraint'] ?? '') !== 'type') {
          return TRUE;
        }
        return !Element::isRenderArray($context[$error['property']] ?? NULL);
      }
    );
    if (empty($errors)) {
      return TRUE;
    }

agentrickard → changed the visibility of the branch 11.x to hidden.

agentrickard → changed the visibility of the branch 11.x to hidden.

agentrickard → created an issue.

I think the temporary solution for that render problem may be to use Twig blocks and embed to pass changes from the story file, but I am not thrilled by that option.

e.g -- twig render template (template.twig)

<% block image %>
  {{ image }}
<% endblock %>

-- story implementation

<code>
{% story my_story with {
    name: 'My story,
    image: '<img src="this.png"/>`
  } %}
  {% embed template.twig %}
    {% block image %}
      {{ image|raw ||
    {% endblock %}
  {% endembed %}
{% endstory %}

The problem here, I suspect, goes much deeper. The current implementation of SDC doesn't do any testing against core render components, which can be wildly inconsistent depending on how they are passed to the template.

So the component assumes that breadcrumb.[0].title is a string, but that is not a safe assumption. How do we make it so?

Or is the intent of SDC not to render core templates but only custom data that the theme/module completely controls?

We were using this patch with CL_SERVER and really like it because it doesn't require config_split to separate permissions on the local or development server, which increases module security.

With this patch, you can still choose to use the permission, but do not need to configure it for local development.

@Igor Mashevskyi The MR will need to be redone against 2.0.x.

This needs to be against the 2.0 branch.

Patch looks good and there is a test! Re-queuing the test bot.

It's a shame there is no way to alter the theme operations except through the page preprocess, but that's a core issue.

See https://github.com/palantirnet/drupal-rector/pull/300

agentrickard → created an issue.

I was just able to do this in a story file, which solves a similar issue where drupal render attributes are not present.

  first: {
    visible: false,
    attributes: create_attribute(),
  },

I wonder if we can make a simple: create_markup() Twig extension and a create_render_array() for the complex case in ✨ Story representation of Drupal render arrays Active

agentrickard → created an issue.

Drush 12 requires PHP Attributes instead of Annotations. So this module only works with Drush 12.

agentrickard → created an issue. See original summary → .

Adds an "enum" element to the "color" example component, since we explicitly only allow two values.

Looks good. Clean and logical.

agentrickard → created an issue.

Sorry. I am bouncing between two docroots to test this. I simply forgot to assign the propery value.

Fixed again, and required `drush cr`

Turns out that you cannot inject the extension list without throwing "Typed property Drupal\cl_server\Theme\ClServerThemeNegotiator::$extensionList must not be accessed before initialization"

Pushed up code style changes after running phpstan / phpcbf etc.

agentrickard → made their first commit to this issue’s fork.

This may be a duplicate of 🐛 String value found, but an array or an object is required RTBC but it is very difficult to see where the issue should be fixed.

Just taking notes at this point. This almost works -- the schema now validates, but the $image element no longer renders.

    // Validator::arrayToObjectRecursive stringifies the props using the JSON
    // encoder. Before that happens, we want to validate classes. Once the
    // classes are validated, we remove them as potential problems for the JSON
    // Schema validation.
    [
      $schema,
      $props_raw,
    ] = $this->validateClassProps($schema, $props_raw, $component_id);

    foreach ($props_raw as $id => $prop) {
      if (is_string($prop)) {
        $decoded = json_decode($prop);
        if ($decoded !== NULL && $decoded !== $prop) {
          $props_raw[$id] = $decoded;
        }
      }
    }

    $schema = Validator::arrayToObjectRecursive($schema);
    $props = Validator::arrayToObjectRecursive($props_raw);

Here's what is being returned from core:

    // Validator::arrayToObjectRecursive stringifies the props using the JSON
    // encoder. Before that happens, we want to validate classes. Once the
    // classes are validated, we remove them as potential problems for the JSON
    // Schema validation.
    [
      $schema,
      $props_raw,
    ] = $this->validateClassProps($schema, $props_raw, $component_id);
    $schema = Validator::arrayToObjectRecursive($schema);
    $props = Validator::arrayToObjectRecursive($props_raw);

$props_raw:

array(6) {
  ["title"]=>
  string(24) "Item with link and image"
  ["title_suffix"]=>
  string(13) "["foo","bar"]"
  ["image"]=>
  string(163) "{"type":"url","url":"https://designsystem.digital.gov/img/introducing-uswds-2-0/built-to-grow--alt.jpg","alt":"USWDS Card alt text","title":"USWDS Card alt title"}"
  ["url"]=>
  string(11) "my-test-url"
  ["date"]=>
  string(24) "2024-02-04T15:00:00.000Z"
  ["attributes"]=>
  NULL
}

$props:

object(stdClass)#1762 (6) {
  ["title"]=>
  string(24) "Item with link and image"
  ["title_suffix"]=>
  string(13) "["foo","bar"]"
  ["image"]=>
  string(163) "{"type":"url","url":"https://designsystem.digital.gov/img/introducing-uswds-2-0/built-to-grow--alt.jpg","alt":"USWDS Card alt text","title":"USWDS Card alt title"}"
  ["url"]=>
  string(11) "my-test-url"
  ["date"]=>
  string(24) "2024-02-04T15:00:00.000Z"
  ["attributes"]=>
  NULL
}

So it appears that we may need to transform the string to JSON prior to that call.

I suspect this is a core issue, since the JsonSchema/Validator component says:

Validates the given data against the schema and returns an object containing the results Both the php object and the schema are supposed to be a result of a json_decode call. The validation works as defined by the schema proposal in http://json-schema.org.

Fix typo in title.

agentrickard → created an issue.

Update:

* This merge shows the issue (and the test fails) https://git.drupalcode.org/project/drupal/-/merge_requests/5505/diffs?co...
* Re-added the original patch / change
* Updated all affected views

Here's an updated MR that does the following:

* Removes the new test file
* Adds the `description` element to `views.view.content.yml`

Well, I suspect this minor issue may be covering up a more major problem (which we should file separately).

This is the YML from the test file:

            time_diff:
              enabled: false
              future_format: '@interval hence'
              past_format: '@interval ago'
              granularity: 2
              refresh: 60
              description: ''

This is the YML from core views.view_content

            time_diff:
              enabled: false
              future_format: '@interval hence'
              past_format: '@interval ago'
              granularity: 2
              refresh: 60

When the schema change was made in core, the affected core views were not updated. The following core views are potentially affected:

• views.view.media.yml
• views.view.test_content_ajax.yml
• views.view.test_field_created.yml
• views.view.glossary.yml
• views.view.content.yml
• views.view.media_library.yml
• views.view.media.yml
• views.view.files.yml
• views.view.moderated_content.yml
• views.view.comment.yml
• views.view.block_content.yml
• core.entity_view_display.media.type_five.default.yml
• core.entity_view_display.media.type_two.default.yml
• core.entity_view_display.media.type_three.default.yml
• core.entity_view_display.media.type_one.default.yml

What does this potentially mean?

1. Core is not updating its views consistently -- for both distribution and tests -- this seems like a process issue
2. Views only updates yml elements when the plugin usage is added (not edited, so far as I can tell)

If the affected core views had been updated with the original change was made, tests would have caught the issue.