getkirby / kirby

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

[v4] Programmable Blueprints: blocks not shown on empty fieldsets array #5794

Closed HYR closed 10 months ago

HYR commented 11 months ago

This approach came from the Programmable Blueprints cookbook article.

Summary

I've been using the programmable blueprints approach for quite some time. It continued to work in the alphas but now in the betas, no blocks in the block chooser show. Same for page types that are loaded by default via this method. Now all page types are show by default.

Longer Description

I'm using the system.loadPlugins:after hook in the index of my custom plugin and that part still seems to be working. I'm pulling in options via the shortcut options('...') and merging that array with a default list of blocks.

image

Registering them like this:

image

Been using this approach for the last year or two. This worked in the Alphas, but now in the betas, no blocks show up.

CleanShot 2023-10-13 at 11 32 47@2x

Same issue with page types (you can see them added in the ->extend() above):

CleanShot 2023-10-13 at 11 34 35@2x

Expected behavior
Expected blocks and page types to continue to work like they did in the v4 alphas.

To reproduce

Not sure how to reproduce. :( Not sure what changed.

Your setup

Kirby Version
Kirby 4.0.0-beta.2

Console output
No errors in the browser console

Your system (please complete the following information)

Additional context
Attached is a zip with 2 key files.

  1. plugin index
  2. layouts.php which is pulled in first index-and-layout.zip
distantnative commented 11 months ago

@HYR any chance you could trim the files down to a reproducing test that's easier to grasp? Right now the plugin index.php is very overwhelming to start tackling debugging. If not, we'll give our best, but it would be very helpful.

HYR commented 11 months ago

I agree. I'll try to do that tomorrow.

HYR commented 10 months ago

@distantnative I think I got it narrowed down to the open: option on block groups. Did something change between the alphas and betas regarding that option?

image

If I comment out the open line on each and the blocks work again. Doesn't seem to like empty arrays anymore. I think before it must've defaulted to false?

Here's the variable definition they're referencing:$open = option('wsp.site-panel.page.blocks.open') ?? [];.

HYR commented 10 months ago

Also seems like another issue where it doest link fieldsets being an empty array anymore.

CleanShot 2023-11-13 at 17 20 01@2x

In this case $customBlocks was an empty array, which worked in v3. But no longer in v4.

lukasbestle commented 10 months ago

@HYR Could you please post example code as code, not as screenshots? That way we can reproduce it as well without having to type it manually or use OCR. Thanks.

HYR commented 10 months ago

@lukasbestle here is some code to reproduce. In the alphas (and before) it seems that the empty array was acceptable, but starting in the betas an empty array would cause the blocks to not display at all. We have a lot of site relying on the previous way it worked.

Using the code below you can see it cause the empty block chooser. Add any block into the $testBlocks array and it displays the blocks again.

Bare bones plugin index (index.php):

<?php
use Kirby\Cms\App as Kirby;

Kirby::plugin('test/dynamic-blocks', [
    'blueprints'    => [
        'shared/layout' => function ($kirby) {
            return include __DIR__ . '/layout.php';
        },
    ],
    'hooks' => [
        'system.loadPlugins:after' => function () {
            $commonBlocks = ['heading','text','image','video'];
            $testBlocks = []; // empty

            kirby()->extend([
                'blueprints' => [
                    'fieldset/common' => [
                        'fieldsets'=> $commonBlocks
                    ],
                    'fieldset/test' => [
                        'fieldsets'=> $testBlocks
                    ],
                ],
            ], kirby()->plugin('test/dynamic-blocks'));
        }
    ]
]);

Bare bones dynamic blueprint (layout.php):

<?php
use Kirby\Data\Yaml;
$yamlFilePath = __DIR__ . '/layout.yml'; // Path to YAML file
$yamlContent = file_get_contents($yamlFilePath); // Read the file contents
$parsedContent = Yaml::decode($yamlContent); // Parse the YAML content
$defaultLayouts = ["1/1"];
$layouts = ['layouts' => $defaultLayouts];

$fieldsets = [
    'fieldsets' => [
        'common' => [
            'label' => 'Common',
            'type' => 'group',
            'extends' => 'fieldset/common',
        ],
        'test' => [
            'label' => 'Test',
            'type' => 'group',
            'extends' => 'fieldset/test',
        ],
    ]
];

$data = array_merge($parsedContent, $fieldsets, $layouts);
return $data;

Bare bones yaml to import for dynamic blueprint (layout.yml):

type: layout
label: Page Content
distantnative commented 10 months ago

But wouldn't you consider it to be correct that if you pass an empty array as a fieldsets, that those show up as zero/none? An empty array doesn't quite read as "no value, please ignore and use default" like null would do

HYR commented 10 months ago

I would expect the one that receives an empty array to not work but I wouldn't expect it to break the other fieldsets. And correct me if I'm wrong, it seemed be how it worked before.

distantnative commented 10 months ago

I didn't get that it also breaks other field sets - I haven't been able to reproduce it, your examples still include more files that you haven't shared. And yes it very well can be that v4 in some areas does stricter value checks. But can't say for sure until we understand where it goes wrong/differently.

HYR commented 10 months ago

Sorry, the only page I didn't include was the about.yml in the starterkit. And the only change was to update the layout field to use the new dynamically created fieldset definition, the rest was default code from the starterkit's about page.

tabs:
  content:
    icon: text
    label: content
    fields:
      layout:
        extends: shared/layout

I ran through it from scratch again to make sure. Here are the steps I took:

  1. Install kirby starter kit
  2. Update kirby installtion with Kirby4 RC1
  3. Create plugin with the 3 files from previous comment (index.php, layout.php, layout.yml)
  4. Update about.yml blueprint to use the new shared/layout fieldset (above in this comment)

At this point the block chooser should be empty because of the array $testBlocks is empty. Which looks like this:

CleanShot 2023-11-21 at 21 02 59@2x

Once you put any valid block name (ex: 'heading') into the array, the chooser works again. Which looks like this:

CleanShot 2023-11-21 at 21 01 41@2x
distantnative commented 10 months ago

But what about the layout.yml?

HYR commented 10 months ago

It's super short at the bottom of this comment: https://github.com/getkirby/kirby/issues/5794#issuecomment-1820069306

type: layout
label: Page Content

In the actual sites it has all of the tabs with extra fields I use but in this example wanted it to be bare bones.

distantnative commented 10 months ago

@HYR are you getting any errors in the console?

distantnative commented 10 months ago

Ahhh think I got it