getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Extending core block blueprints #4560

Open ovenum opened 2 years ago

ovenum commented 2 years ago

Description

When extending Kirbys core image block it’s not possible to limit the ratio options.

testBlocks:
  type: blocks
  fieldsets:
    image:
      extends: blocks/image
      fields:
        ratio:
          options:
            1/1: "1:1"
            16/9: "16:9"
            7/5: "7:5"

Expected behavior

The only options for the ratio should be the ones passed from the blueprint. (1/1, 16/9, 7/5)

As described in the cook book recipe Modifying blocks types and custom blocks / Extending existing fields it should be possible to limit the options.

Your setup

Kirby Version
3.7.2.1

Additional context

The difference between the example given in cook book recipe and the one above is the use of an non-associative array for the level options in the recipe and a associative array for ratio options.

[…]
fieldsets: 
  heading:
    extends: blocks/heading
    fields:
      level:
        # non-associative array, does overwrite the core block options
        options:
          - h2
          - h3
  image:
    extends: blocks/image
    fields:
      ratio:
        # associative array, does not overwrite the core block options
        options:
          1/1: "1:1"
          16/9: "16:9"
          7/5: "7:5"   

This is probably the only possible behaviour? Kirby/Cms/Blueprint,php:210 is merging the core blocks blueprint settings with the extension by calling A::merge($mixin, $props, A::MERGE_REPLACE). With A::MERGE_REPLACE only non-associative arrays will be replaced, so it’s not possible to remove keys from an associative array and that’s what you want 99.9% of the time when using extends with blueprints.

Maybe the cook book recipe should make a note of this?

ovenum commented 2 years ago

If required i can create a PR to fix extending the core gallery block ratio options by supplying a non-associative array for the ratio options. This would allow extending the core gallery block with custom ratio values.

Not sure what the way forward is regarding #4555.

    options:
      - value: 1/1
        text: "1:1"
      - value: 16/9
        text: "16:9"
      - value: 10/8
        text: "10:8"
      - value: 21/9
        text: "21:9"
      - value: 7/5
        text: "7:5"
      - value: 4/3
        text: "4:3"
      - value: 5/3
        text: "5:3"
      - value: 3/2
        text: "3:2"
      - value: 3/1
        text: "3:1"
lukasbestle commented 2 years ago

This is probably the only possible behaviour? Kirby/Cms/Blueprint,php:210 is merging the core blocks blueprint settings with the extension by calling A::merge($mixin, $props, A::MERGE_REPLACE). With A::MERGE_REPLACE only non-associative arrays will be replaced, so it’s not possible to remove keys from an associative array and that’s what you want 99.9% of the time when using extends with blueprints.

That's exactly the issue. By merging arrays, we cannot control the merge or replace behavior for each array level. If we'd always replace nested associative arrays instead of merging them, you would have to basically define the whole block blueprint again.

Switching to an indexed array for the ratio options could indeed be an option. It's a bit verbose, but it would only be needed in the core block blueprint. However I don't know what the general plans for extending blueprints are with the upcoming blueprint refactoring.

A workaround can be to copy the core block blueprint and modify your copy instead of extending it.

ovenum commented 2 years ago

Thanks for looking at this @lukasbestle. Don’t think the merging behaviour should change–it works really well–except for this tiny case.

So this could be revisited after the blueprint rewrite is done and then core fields/ block could be adapted with this in mind, if it’s still an issue.