Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default alignment for blocks #185

Closed
wants to merge 1 commit into from
Closed

Fix default alignment for blocks #185

wants to merge 1 commit into from

Conversation

marcbelletre
Copy link
Contributor

@marcbelletre marcbelletre commented Dec 13, 2023

The default values for align_text and align_content don't work currently.

I figured out that alignText and alignContent attributes are automatically added to the block definition when enabling support for these properties. The values of both properties don't reflect the default values that are set in the Block definition.

Let's say we have a simple Block that have centered text and content as default values.

/**
 * The default block text alignment.
 *
 * @var string
 */
public $align_text = 'center';

/**
 * The default block content alignment.
 *
 * @var string
 */
public $align_content = 'center';

/**
 * The supported block features.
 *
 * @var array
 */
public $supports = [
    'align' => false,
    'align_text' => true,
    'align_content' => true,
    'full_height' => false,
    'anchor' => false,
    'mode' => false,
    'multiple' => true,
    'jsx' => true,
];

When adding a new block to the editor the defaults are not used.
If I dump the $block variable that is passed to the render function of src/Block.php I can see that alignText and alignContent are automatically set to left and top

array:29 [▼
  "name" => "acf/icon"
  ...
  "align" => ""
  "align_text" => "left"
  "align_content" => "top"
  ...
  "alignText" => "left"
  "alignContent" => "top"
  "_acf_context" => array:2 [▶]
  "id" => "block_79c01833-7af5-4fb7-8044-85de0dfb4006"
]

I think this bug has been introduced since ACF 6. According to this thread they have added the camel-case properties to match the block.json format.

We’ve fixed a few reported bugs with ACF Blocks in this build as well. For example, content after <InnerBlocks /> will now render correctly without the need to wrap it in another div, and alignText will now always default to the WordPress default of left rather than an empty string.

There is a comment referencing the same issue but no solution have been provided.

I found out that adding the alignText and alignContent properties to the block settings fixes the issue.

@Log1x
Copy link
Owner

Log1x commented Feb 26, 2024

Do we need both then?

@Log1x
Copy link
Owner

Log1x commented Mar 1, 2024

This was inadvertently handled in #224

@Log1x Log1x closed this Mar 1, 2024
@marcbelletre
Copy link
Contributor Author

The default alignment option still does not work with the latest version.

I have a simple Icon block with the following definition:

<?php

namespace App\Blocks;

use Log1x\AcfComposer\Block;
use StoutLogic\AcfBuilder\FieldsBuilder;

class Icon extends Block
{
    /**
     * The block name.
     *
     * @var string
     */
    public $name = 'Icon';

    /**
     * The default block text alignment.
     *
     * @var string
     */
    public $align_text = 'center';

    /**
     * The default block content alignment.
     *
     * @var string
     */
    public $align_content = 'center';

    // ...

When I insert a new Icon block in the editor, the alignment is set to left instead of being centered.

If I switch back to the patch I provided the block is centered as it should be.

@marcbelletre marcbelletre deleted the patch-1 branch March 5, 2024 11:30
@Log1x
Copy link
Owner

Log1x commented Mar 5, 2024

Sorry about that, I got confused and thought this change was inside of $supports which now automatically handles casing.

@marcbelletre
Copy link
Contributor Author

No problem, thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants