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

feat: Make wrapNode protected #116

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

erkenes
Copy link

@erkenes erkenes commented Sep 7, 2022

Make the function wrapNode as protected to allow overwriting

Make the function `wrapNode` as protected to allow overwriting
@bwaidelich
Copy link
Member

Thanks for the contribution but could you please share your usecase? This method is private intentionally because this is not part of the public API.

@erkenes
Copy link
Author

erkenes commented Sep 7, 2022

@bwaidelich In some cases the wrapper breaks the layout in the backend.
For example if I have a Grid-Node with Colum-Nodes in a collection.

image

The current behavior destroys the layout by putting a second div between elements where none should be existing.

<div data-_neos-form-builder-type="Foo.FormBuilder:Grid" >
    <div class="row">
        <div data-_neos-form-builder-type="Foo.FormBuilder:Column">
            <div class="col col-md-6">
                // Content
            </div>
        </div>
    </div>
</div>

How it is possible after the change:

<div data-_neos-form-builder-type="Foo.FormBuilder:Grid" >
    <div class="row">
        <div data-_neos-form-builder-type="Foo.FormBuilder:Column" class="col col-md-6">
            <div>
                // Content, the upper div doesn't have the classes in the backend
            </div>
        </div>
    </div>
</div>
<?php
namespace Foo\BaseFormBuilder\Fusion;

class FormElementWrappingImplementation extends \Neos\Form\Builder\Fusion\FormElementWrappingImplementation
{
    protected function wrapNode(NodeInterface $node, string $output, string $fusionPath): string
    {
        // [...]
    
        if ($node->getNodeType()->getName() === 'Foo.FormBuilder:Column') {
            $columnWidths = '';
    
            if ($node->hasProperty('widthXs')) {
                $columnWidthXs = $node->getProperty('widthSm');
                $columnWidths .= ($columnWidthXs != 'unset' && $columnWidthXs)
                    ? $node->getProperty('widthXs')
                    : '';
            }
            if ($node->hasProperty('widthSm')) {
                $columnWidthSm = $node->getProperty('widthSm');
                $columnWidths .= ($columnWidthSm != 'unset' && $columnWidthSm)
                    ? ' ' . $node->getProperty('widthSm')
                    : '';
            }
            
            $additionalAttributes['class'] = $columnWidths;
        }
        
        return $this->contentElementWrappingService->wrapContentObject($node, $output, $fusionPath, $additionalAttributes);
    }
}

@bwaidelich
Copy link
Member

bwaidelich commented Sep 7, 2022

Thanks for your thorough explanation. I'm reluctant to make this part of an extensible API because it will most likely break compatibility if the implementation is going to change.
If the backend layout is broken due to some invalid markup, we should report a bug and fix the root cause (similar to #27).

If this is just an issue with your specific markup, you could maybe adjust your CSS and/or the rendering of your custom form elements.
You could do that only for rendering in the Neos backend.
For example, to add some custom wrapping markup:

prototype(Your.Package:YourElement) {
    @process.wrap = ${'<div>' + value + '</div>'}
    @[email protected] = ${node.context.inBackend}
}

or, to add custom attributes:

prototype(Your.Package:YourElement) {
    @process.augment = Neos.Fusion:Augmenter {
        'custom-attribute' = 'custom-value'
    }
    @[email protected] = ${node.context.inBackend}
}

does that help?

@crydotsnake
Copy link
Member

Ping @erkenes

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.

3 participants