area17 / twill

Twill is an open source CMS toolkit for Laravel that helps developers rapidly create a custom admin console that is intuitive, powerful and flexible. Chat with us on Discord at https://discord.gg/cnWk7EFv8R.
https://twillcms.com
Apache License 2.0
3.72k stars 568 forks source link

Custom blocks: Block title is being displayed twice #2485

Open Tofandel opened 6 months ago

Tofandel commented 6 months ago

The block title is being displayed both in the repeater header and in the block content

image

image

This is wasting precious screen space

The default blocks don't seem to have this, but when you create you own blocks they all have this, is there any way to remove it?

ifox commented 6 months ago

If you have more than one field in a block, you do want a label on each field for clarity and accessibility purposes. In the case of blocks with a single field, I would agree with you and I believe you can do that by using an empty label (TBC in Twill 3 with the form builder, but I know for sure it was possible in Twill 2). However, the label on the block wrapper itself is something you can actually configure to use the content of one of the fields in the block itself, and you can decide to keep the prefix or not in that case. I know that's not exactly what you are talking about but I want to make sure you are aware of the full behavior. The default blocks are also not a great reference, see the discussion in #2410.

Tofandel commented 6 months ago

I realised that yes it's the label of the field and not the block which could be different

But it's not possible to set an empty label to a field, maybe we should allow it?

public function getForm(): Form
    {
        return Form::make([
            Wysiwyg::make()->name('text')->label('')->translatable()
        ]);
    }

image

All the fields have mandatoryProperties: ['label'] it seems

Tofandel commented 6 months ago

If I change

+ if ($this->{$property} === null) {
- if (! $this->{$property}) {
  throw new \InvalidArgumentException(
      "Missing required field property '$property' on " . $this::class
  );
}

Then it displays without a label and doesn't complain about it, but I don't know if the better solution would be to remove label from the mandatoryFields

ifox commented 6 months ago

Right, that's what I was suspecting would be blocking it on Twill 3. I think we could totally remove it from mandatoryFields to allow this use case.