wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.34k stars 191 forks source link

Trigger events field with ^ parent selector does not work (nestedform) #716

Closed AIC-BV closed 6 months ago

AIC-BV commented 1 year ago

Winter CMS Build

1.2

PHP Version

8.1

Database engine

MySQL/MariaDB

Plugins installed

Winter.Sitemap, Winter.Pages, Winter.TinyPNG, Winter.Translate, Winter.Redirects, Aic.Globals, Aic.ExtendWinterPages, Aic.Blog, Aic.Team, Aic.Account, Aic.Contact, Aic.Invoice, Aic.Discount, Aic.Aftersales, Aic.Reviews, Aic.Faq, StudioBosco.BackendNotifications, StudioBosco.BackendComments, ...

Issue description

Adding a trigger to your form with the fields parent selector does not work

block_gallery:
    name: Gallery
    icon: icon-camera-retro
    fields:
        type:
            label: Type
            type: balloon-selector
            default: grid
            options:
                grid: Grid
                slider: Slider
        data:
            type: nestedform
            usePanelStyles: false
            cssClass: custom-nested-form
            form:
                tabs:
                        images:
                            tab: Images
                            type: repeater
                            form:
                                fields:
                                    caption:
                                        label: Caption
                                        type: richeditor
                                        size: small
                                        trigger:
                                            action: hide
                                            field: ^^type
                                            condition: value[grid]

No matter how many thingies ^^^ I add, the trigger never happens

Steps to replicate

  1. Create a plugin
  2. Use the code above
  3. Check the trigger and notice it won't trigger

Workaround

I added a comment, which is an -ok acceptable- workaround for my case commentAbove: Not for type - GRID

github-actions[bot] commented 1 year ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.

github-actions[bot] commented 11 months ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.

LukeTowers commented 11 months ago

@AIC-BV is this still an issue? Did you end up using a workaround?

AIC-BV commented 11 months ago

@LukeTowers I still have this issue... Used the workaround but its not what it is supposed to be I have it in a new project so it must be there still

fields.yaml

# ===================================
#  Field Definitions
# ===================================

tabs:

    icons:
        Blocks: icon-hammer

    fields:

        blocks:
            tab: Blocks
            type: repeater
            groups: ~/plugins/aic/demo/models/issues/blocks.yaml

blocks.yaml

block_gallery:
    name: Gallery
    icon: icon-camera-retro
    fields:
        type:
            label: Type
            type: balloon-selector
            default: grid
            options:
                grid: Grid
                slider: Slider
        data:
            type: nestedform
            usePanelStyles: false
            cssClass: custom-nested-form
            form:
                tabs:
                        images:
                            tab: Images
                            type: repeater
                            form:
                                fields:
                                    caption:
                                        label: Caption
                                        type: richeditor
                                        size: small
                                        trigger:
                                            action: hide
                                            field: ^^type # No matter the amount of ^, its not triggering
                                            condition: value[grid]
AIC-BV commented 11 months ago

But it must be very specific to my setup because when I adjust wn-blocks-plugin it is working!

image

I guess it does work then

AIC-BV commented 11 months ago

I managed to break it here. Test3 (label: Test 2 😅) never gets hidden, no matter the amount of ^. Its hard to say this is my specific case because it works differently in my plugin but it might be the issue I have

image

button.block

name: winter.blocks::lang.blocks.button.name
description: winter.blocks::lang.blocks.button.description
icon: icon-caret-square-o-right
tags: ["pages"]
fields:
    config:
        type: nestedform
        usePanelStyles: false
        form:
            fields:
                is_delayed:
                    label: Send later
                    comment: Place a tick in this box if you want to send this message at a later time.
                    type: checkbox
                trigger:
                    type: repeater
                    form:
                        fields:
                            test:
                                label: Test
                                span: auto
                                type: text
                                trigger:
                                    action: hide
                                    field: ^is_delayed
                                    condition: checked
                            test2:
                                type: nestedform
                                usePanelStyles: false
                                form:
                                    fields:
                                        test3:
                                            label: Test 2
                                            span: auto
                                            type: text
                                            trigger:
                                                action: hide
                                                field: ^^is_delayed
                                                condition: checked

            tabs:
                icons:
                    winter.blocks::lang.fields.actions: 'icon-arrow-pointer'
                    winter.blocks::lang.tabs.display: 'icon-brush'

                fields:
                    actions:
                        type: repeater
                        tab: winter.blocks::lang.fields.actions
                        prompt: winter.blocks::lang.fields.actions_prompt
                        groups: $/winter/blocks/meta/actions.yaml
                    color:
                        label: winter.blocks::lang.fields.color
                        tab: winter.blocks::lang.tabs.display
                        span: auto
                        type: colorpicker
                    icon:
                        label: winter.blocks::lang.fields.icon
                        tab: winter.blocks::lang.tabs.display
                        span: auto
                        type: iconpicker
==
LukeTowers commented 11 months ago

@AIC-BV would you be able to add a test case to the Record model in the Winter.Test plugin replicating the issue and then we can come up with a fix from there and have a persistent test case for it in the future?

AIC-BV commented 11 months ago
image

I made a plugin, let me see if I can put it as PR

AIC-BV commented 11 months ago

@LukeTowers https://github.com/wintercms/wn-test-plugin/pull/18

LukeTowers commented 6 months ago

@mjauvin is this something you can take a look at?

mjauvin commented 6 months ago

Ok, I found the issue.

In order to find the trigger field, the field's arrayName is reduced by a number of levels to find the actual parent form to look for the trigger field. This happens here:

https://github.com/wintercms/winter/blob/develop/modules/backend/classes/FormField.php#L508

Now, the HtmlHelper::reduceNameHierarchy() method was poorly designed and will not properly reduce the arrayName hierarchy when nested forms are involved, it was designed to reduce it for repeater fields. As explained in the reduceNameHierarchy() method's comments, it removes $level*2 at a time (repeaters have the fieldName + index). But nested forms only have the nestedform fieldName.

ref. https://github.com/wintercms/storm/blob/develop/src/Html/Helper.php#L53-L66

So to fix the issue, we would need to rewrite HtmlHelper::reduceNameHierarchy() to remove $levels*2 when there's an index, and $level*1 otherwise.