Issue with conditional logic in repeaters

[ghost]

Expected Behavior

Hello guys, You have the possibility, inside a repeater, to apply a condition to a field. This condition can come from another field which is at the same level as the repeater.

Actual Behavior

In fact, the condition inside the repeater not working if the condition come from to level above. My field '--title' or '--texte' are always present.

Steps to Reproduce the Problem

My code :

ButtonGroup::make('Choice ','sh-button')
   ->choices([
     'text' =>  'Text',
      'title' => 'Title',
      'nothing' => 'Nothing',
    ])
    ->defaultValue('title')
    ->returnFormat('value') 
    ->layout('horizontal'),
Repeater::make('Content','sh-repeater')
   ->buttonLabel('add slide')
   ->min(1)
    ->fields([
         Textarea::make('Titre de la section', '--title')
             ->conditionalLogic([
                   ConditionalLogic::if('sh-button')->equals('title')
               ]),
         Textarea::make('Titre de la section', '--texte')
             ->conditionalLogic([
              ConditionalLogic::if('sh-button')->equals('text'),
              ]),
      ])
      ->conditionalLogic([
    ConditionalLogic::if('sh-button')->notEquals('nothing'),
     ]),

Versions

• WordPlate: 8.4
• PHP: 7.4.2

[vinkla]

Do you know if this works when the fields are registered using the default ACF UI within the WP dashboard?

[ghost]

Yes it's works. I have attached, a PHP export with the fields are registered using the default ACF.

test.php.zip

[vinkla]

I see the problem now. This is a limitation of the current implementation since we always use the parent key when we construct new child keys. Thanks for pointing it out!

I've an idea though. Since we put all keys through the Key class we could probably create a registry of all available keys and their field names and save them in a static variable. We actually already do this to prevent duplicated keys:

https://github.com/wordplate/extended-acf/blob/1edce87ef2fe7d286526d0e63af1a42034910d32/src/Key.php#L46-L49

If this idea would work, it can probably be applied to the clone field as well. Which would be really cool. I'll look into this in the future.

[nateplusplus]

My team also ran into this issue. @vinkla let me know if there's anything I can do to help sort it out.

[vinkla]

I've been thinking about my idea and found one limitation. If we save the names and keys in an array globally they will most likely collide. Two fields in different field groups can share the same name as long as the field group name is different.

Since conditional logic only applies to the field group we could possibly save the keys on the field group. Not sure how yet.

@Cestfleurych @nateplusplus any ideas or suggestions to solve this issue would be most appreciated!

[nateplusplus]

@vinkla I think you're on to something. Forgive me as I am a bit new to your codebase but it seems like you could use the scope of your FieldGroup class to store those keys, as you've mentioned, and that way you have easier access without the risk of colliding with other field groups. Is that sort of what you have in mind?

I do wonder though: What happens if a field within a repeater and outside a repeater share the same name? Is this why you use a parent key to help generate field IDs? If so, in this scenario, I suppose you'll need to account for which field is closest or some other logic to determine the correct field ID?

[vinkla]

Is that sort of what you have in mind?

Almost, I believe we still need to save the keys globally since we can't have two field groups with the same name.

What happens if a field within a repeater and outside a repeater share the same name? Is this why you use a parent key to help generate field IDs?

Yes, ACF generates unique keys and save them in the database. Our implementation depends on the parent keys to generate unique field keys.

If so, in this scenario, I suppose you'll need to account for which field is closest or some other logic to determine the correct field ID?

I guess. Not really sure how ACF handles this internally. Is it possible to add conditional logic to sub fields in a repeater?

[nateplusplus]

Thanks for the clarification @vinkla, I think your approach makes sense as a pretty quick solution, but I do see what you mean about possible collisions.

So if I am understanding completely, your idea is to check a global array which maps field names to field keys, is that correct? In this case, if you do happen to come up with results from other field groups, maybe you could check those keys to see if they exist in the correct field group? Do the field keys get stored within the Config class? I do see a Config->has() method that may help?

Regarding my previous comment:

What happens if a field within a repeater and outside a repeater share the same name?

Thinking more about that, I have a feeling that is a very rare case. I did test it with ACF UI editor and confirmed it is possible, but probably not the best idea to setup fields with identical names in the same group anyway.

Is it possible to add conditional logic to sub fields in a repeater?

Yes, I think that is what this ticket is addressing right?

[vinkla]

Thanks for sharing your thoughts Nathan! Keep em coming 😃

So if I am understanding completely, your idea is to check a global array which maps field names to field keys, is that correct? In this case, if you do happen to come up with results from other field groups, maybe you could check those keys to see if they exist in the correct field group?

Yes, could probably group the keys by field group.

Thinking more about that, I have a feeling that is a very rare case. I did test it with ACF UI editor and confirmed it is possible, but probably not the best idea to setup fields with identical names in the same group anyway.

You're probably right.

Is it possible to add conditional logic to sub fields in a repeater?

Yes, I think that is what this ticket is addressing right?

What I meant (but left out 🤷) was if it is possible to to have conditional logic against other sub fields. Just tried it locally and it seems so. Would be great if we had a solution which worked as the UI.

[leandropapasidero]

My team also ran into this issue. :(

[jaydjohnson]

Any progress on this? I would love to see this working. Or has anyone found a work around?

[vinkla]

Any progress on this? I would love to see this working. Or has anyone found a work around?

We haven't had the time and resources to work on this yet.

---

We're thinking of setting up sponsorship for WordPlate to allocate more resources.

@Cestfleurych @nateplusplus @leandropapasidero @jaydjohnson If we were to add sponsorship, would any of you consider supporting the project? How much would you consider sponsoring the project with?

[loraxx753]

Hey all! I believe I resolved this issue. @jaydjohnson and @nateplusplus have already reviewed the code and can give more context if necessary.

Oh, here's the pull request!

[vinkla]

Thanks to @loraxx753 this will be shipped in version 10. This can be tested today with the branch alias for version 10. Please help us test this in your applications:

"require": {
    "wordplate/acf": "^10.0"
},
"minimum-stability: "dev",
"prefer-stable: true