Support `{type: array, …}` prop shapes

#2: yes.

(Which is what you referred to in #3454125-70: Implement temporary design system for the DrupalCon Barcelona demo, point 3.

While this does need design, I think we can lay the shape matching foundations. A conversation with @catch about #3440578: [PP-2] JSON-based data storage proposal for component-based page building yesterday made me realize that:

1. this is more within reach since #3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings than I thought
2. having this working for at least type: array, items: integer and type: array, items: string, maxItems: 2 would actually help the discussion over at #3440578

I've made progress today on updating FieldTypePropExpression, StaticPropSource and Evaluator to support type: array.

How?

By changing StaticPropSource to not be required to contain a FieldItemInterface (i.e. a single field delta), but instead allowing a FieldItemListInterface too (i.e. multiple field deltas).

It's too unorganized right now to push; expect an update tomorrow 😊

Thanks @catch for nudging me in this direction 😄

Just pushed initial test coverage in PropSourceTest::testStaticPropSource(), to test it with multiple cardinality:

    // A simple (expression targeting a simple prop) array example (with
    // cardinality specified, rather than the default of `cardinality=1`).
    $simple_array_example = StaticPropSource::parse([
      'sourceType' => 'static:field_item:integer',
      'sourceTypeSettings' => [
        'storage' => [
          'cardinality' => 5,
        ],
      ],
      'value' => [
        20,
        06,
        1,
        88,
        92,
      ],
      'expression' => 'ℹ︎integer␟value',
    ]);

It should now fail like this:

1) Drupal\Tests\experience_builder\Kernel\PropSourceTest::testStaticPropSource
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'{"sourceType":"static:field_item:integer","value":[20,6,1,88,92],"expression":"ℹ︎integer␟value","sourceTypeSettings":{"storage":{"cardinality":5}}}'
+'{"sourceType":"static:field_item:integer","value":null,"expression":"ℹ︎integer␟value","sourceTypeSettings":{"storage":{"cardinality":5}}}'

… because StaticPropSource currently ignores cardinality — it always just constructs a single FieldItemInterface object rather than a list when cardinality >=1.

Yay, from:

1) Drupal\Tests\experience_builder\Kernel\HookStoragePropAlterTest::testPropShapesYieldWorkingStaticPropSources
Sample value 88 generated by field type integer for type=array&items[type]=integer is invalid!
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 ()
+Array &0 (
+    0 => Array &1 (
+        'property' => 'ds032jpb'
+        'pointer' => '/ds032jpb'
+        'message' => 'Integer value found, but an array is required'
+        'constraint' => 'type'
+        'context' => 1
+    )
+)

(IOW: a single integer is passed, when an array of integers is needed)

to … that test passing 🤓

More coming.

I was chasing a heisenbug for a few hours today. I then eventually found the root cause.

It happens only for a list_string field type with one of its allowed values the empty string:

        [
          'allowed_values' => [
            ['value' => '', 'label' => ''],
            ['value' => '_blank', 'label' => '_blank'],
          ],
        ]

… because somewhere '' gets cast to NULL 😅

XB's tests generate a random value to verify the static prop source works end-to-end, and due to the changes in this MR (introducing the use of FieldItemList and hence calling FieldItemList::isEmpty(), which we didn't call before), this:

now resulted in

which is obviously wrong, but count() works fine:

… this is valid configuration for the list_string field type: its config schema type field.storage_settings.list_string does not forbid this.

But the UI does forbid it:

which causes:

So probably a fixed ListStringItem::isEmpty() that is more strict than the inherited \Drupal\options\Plugin\Field\FieldType\ListItemBase::isEmpty()'s implementation:

  public function isEmpty() {
    return empty($this->value) && (string) $this->value !== '0';
  }

… would not even be accepted by Drupal core maintainers! 😬

Unfortunately, I lost hours today because of this. If there was config schema validation in place, this would've been sorted out in minutes. 😭

So: not filing a Drupal core issue, but I am creating one for Experience Builder to disallow '' as an enum value, so that'd be a reason to refuse to support such an SDC ← I can't justify that, because it's a valid use case for SDC, and we can make it work.

To make #13 discoverable.

I actually think that matching field instances should be possible too, so broadening the issue title/scope 🤓

type: array breakthrough — it’s starting to work end-to-end! 😄

Soon: 🖼️🖼️🖼️🖼️ aka image galleries!

(Note that this currently requires removing the logic in \Drupal\experience_builder\Controller\SdcController::getComponentsList() to determine the dynamic_prop_source_candidates for each SDC.)

99% of remaining test failures are due to not yet supporting finding DynamicPropSources (aka existing entity field instances) for the type: array prop shape.

That's why I widened the scope in #15 — because I first thought it'd be an order of magnitude harder than supporting StaticPropSources (which is the only thing the XB UI currently supports). But it'd be a shame to let that part "fall behind", so trying to tackle this while my head is in this space 😊

1% of remaining test failures are in the Cypress E2E tests, which makes sense: the Redux integration has only been built with single-cardinality props in mind, that's changing here.

IOW: having this in also empowers #3463842: [META] Redux sync on ALL prop types, not just ones with a single [value] property to design a more complete solution.

Merged in all upstream changes.

We may also need to consider this in #3467954: META: Evolve XB UI's data model to allow non-SDC components' inputs, DynamicPropSource support, etc..

https://nerdy.dev/carousel-adaptive-anchor-positioning-with-calc-in-a-co... would be a fantastic way to demonstrate this!

~90% done now. To be continued tomorrow.

Good news — previews are now working! For example, for the Sparkline test SDC:

powered by:

props:
  type: object
  required:
    - data
  properties:
    data:
      type: array
      title: Sparkline data
      maxItems: 100
      items:
        type: integer
        minimum: -100
        maximum: 100
      examples:
        - [0, 10, 20, 30, -40, -50, 5, 7, 9]

And, the one everyone has been waiting for: image galleries … found and fixed a pre-existing bug thanks to this work: #3522673: Previews of SDCs with default image props broken. With it, I get:

powered by

props:
  type: object
  required:
    - images
  properties:
    caption:
      type: string
      title: Caption
      description: The image gallery caption
    images:
      title: 'Images for gallery'
      type: array
      items:
        $ref: json-schema-definitions://experience_builder.module/image
        # @todo `type: object` should not be necessary, it's because \Drupal\sdc\Component\ComponentValidator::getClassProps() does not yet support $ref
        type: object
      examples:
        - - src: gracie.jpg
            alt: 'A good dog'
            width: 601
            height: 402
          - src: gracie.jpg
            alt: 'Still a good dog'
            width: 601
            height: 402
          - src: gracie.jpg
            alt: 'The BEST dog!'
            width: 601
            height: 402

Soon: 🖼️🖼️🖼️🖼️ aka image galleries!

— me in #16, ~7 months ago

Here we go! 🚀

(It doesn't work completely yet — only the first selected image media item is rendered due to the mediaSelection client-side transform, and removing the last selected media item also doesn't work yet. But those are out-of-scope to tackle here.)

Removing the Needs design tag, because extracted the relevant part into a new issue: #3522718: [later phase] [needs design] UX for associating mismatched cardinality field instance (too little or too much) with a higher cardinality SDC prop (e.g. `type: array, maxItems: 5`) or lower cardinality (e.g. `type: string`).

This MR removes 3 @todos from the codebase, all new @todos are for existing issues.

Self-RTBC'ing because @tedbow already approved the direction in #21. I've applied thorough self-review. And I desperately need this to make progress on #3520449: [META] Production-ready data storage! 😅 Even if imperfect (which it surely will prove to be), this represents a big step forward, one we've been postponing for far too long.

1. FYI this no longer works anymore.
   There was no e2e tests added here and in manual testing:

   e2e tests were intentionally out of scope, this was purely about shape matching infra. The field widget/editing part of this never worked, see #16.

2. Testing with a simple multi-value string shows the same result, only ever the first value is saved.

   Correct, that's why I wrote this in #28:

   (It doesn't work completely yet — only the first selected image media item is rendered due to the mediaSelection client-side transform, and removing the last selected media item also doesn't work yet. But those are out-of-scope to tackle here.)

That's why I posted a comment on #3463842 — see #3463842-12: [META] Redux sync on ALL prop types, not just ones with a single [value] property. But thanks for creating #3546869: Editing support for `type: array`: support multiple-cardinality Field Widgets (`StaticPropSource`s) to get that done sooner! 🤩