Problem/Motivation

https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/Element/... has this method:

  /**
   * Process stories slots.
   *
   * Stories slots have no "#" prefix in render arrays. Let's add them.
   * A bit like UI Patterns 1.x's PatternPreview::getPreviewMarkup()
   * This method belongs here because used by both ui_patterns_library and
   * ui_patterns_legacy.
   */
  public function processStoriesSlots(array $slots): array {
    foreach ($slots as $slot_id => $slot) {
      if (!is_array($slot)) {
        continue;
      }
      if (array_is_list($slot)) {
        $slots[$slot_id] = $this->processStoriesSlots($slot);
      }
      $slot_keys = array_keys($slot);
      $render_keys = ["theme", "type", "markup", "plain_text"];
      if (count(array_intersect($slot_keys, $render_keys)) > 0) {
        foreach ($slot as $key => $value) {
          if (is_array($value)) {
            $value = $this->processStoriesSlots($value);
          }
          if (str_starts_with($key, "#")) {
            continue;
          }
          $slots[$slot_id]["#" . $key] = $value;
          unset($slots[$slot_id][$key]);
        }
      }
    }
    return $slots;
  }

However, some recent examples showed this rule is too naive and simple. For example, with a html_tag render element, we don't want the children to get the # prefix

Proposed resolution

  1. Move the logic to its own service under ui_patterns_library (so, ui_patterns_library is becoming a dependency of ui_patterns_legacy)
  2. Fix the logic

Issue fork ui_patterns-3477573

Command icon 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:

About issue forks

Comments

pdureau created an issue. See original summary.

pdureau’s picture

pdureau commented
Title: [2.0.0-beta4] Performance of SourcePluginManager » [2.0.0-beta4] Better stories slots processing
Issue summary: View changes

pdureau opened merge request !229

pdureau’s picture

pdureau commented
Assigned: pdureau » grimreaper
Issue summary: View changes
Status: Active » Needs review
pdureau’s picture

pdureau commented
Component: Code » UI Patterns Library
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Needs review » Needs work
Issue tags: +Needs tests

Minor code adjustments to discuss.

And needs tests.

pdureau’s picture

pdureau commented

Let's put this list in a constant or an attribute?

OK, I will move $render_keys to a class CONST.

Not sure about $html_tag_allowed_render_keys because it is very specific to the internal logic of the condition.

As this class has no requirement and just manipulate the provided data, should it stay as a service or be converted into a trait?

I personally don't like traits so much in PHP. But I will follow your recommendation if you tell me to use them.

When looking at ui_suite_bootstrap examples, I know it is ui_patterns 1 currently, but is the following code be handled too? Or I misunderstood the goal of the issue?

I will do the test now

pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau
pdureau’s picture

pdureau commented

Tested with ui_suite_bootstrap examples. 2 issues.

--- a/src/Definition/ExampleDefinition.php
+++ b/src/Definition/ExampleDefinition.php
@@ -108,7 +108,9 @@ class ExampleDefinition extends PluginDefinition {
    *   Render array.
    */
   public function getRender(): array {
-    return $this->definition['render'];
+    $render = $this->definition['render'];
+    $converter = \Drupal::service("ui_patterns_library.stories_syntax_converter");
+    return $converter->convertSlots(["example" => $render])["example"];
   }

html_tag: order of keys

Having the html_tag workaround children at the beginning doesn't work:

"0": {}
type: "html_tag"
tag: "div"

We need to have it at the end:

type: "html_tag"
tag: "div"
"0": {}

Proposal:

@@ -80,7 +83,8 @@ class StoriesSyntaxConverter {
       return FALSE;
     }
     // This property has to be a string value.
-    if (!is_string($array[$intersect[0]])) {
+    $property = $intersect[array_key_first($intersect)];
+    if (!is_string($array[$property])) {
       return FALSE;
     }
     return TRUE;

Skip integers children

+++ b/modules/ui_patterns_library/src/StoriesSyntaxConverter.php
@@ -56,6 +56,9 @@ class StoriesSyntaxConverter {
       if ($in_html_tag && !in_array($property, $html_tag_allowed_render_keys)) {
         continue;
       }
+      if (!is_string($property)) {
+        continue;
+      }
       if (str_starts_with($property, "#")) {
         continue;
       }
pdureau’s picture

pdureau commented
Assigned: pdureau » grimreaper
Status: Needs work » Needs review
pdureau’s picture

pdureau commented
Assigned: grimreaper » pdureau
Status: Needs review » Needs work
pdureau’s picture

pdureau commented
Assigned: pdureau » grimreaper
Status: Needs work » Needs review

I have added unit tests.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » pdureau
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

  • pdureau committed bd0ffbd2 on 2.0.x 
    Issue #3477573 by pdureau, grimreaper: Better stories slots processing
pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.