ECA Form: add NOT value condition to "Form field: add state" action

MR has some PHPStan failures but they don't appear to be caused by my changes.

The code works. However, it would be more user friendly if adding a #state to only show (and require/make mandatory) the value form field if "value" or "NOT value" are selected in the condition form field (this may need corresponding form validation). Therefore, I set this issue to "Needs work".

It's a nice feature, but is it really necessary? Can the same not be achieved by using the opposite state for the same value? E.g. visible if value = "something" equals to invisible if value = anything else. Or is there a use case that can not be handled this way?

However, instead of only adding to the value form field description, it would be more user friendly adding a #state to only show (and require/make mandatory) the value form field if "value" or "NOT value" are selected in the condition form field (this may need corresponding form validation).

I can do that, but it's worth noting that the original action did not implement this either; the main use case of ECA is with BPMN, which isn't compatible with #states and only works with a small handful of Drupal form elements.

It's a nice feature, but is it really necessary? Can the same not be achieved by using the opposite state for the same value? E.g. visible if value = "something" equals to invisible if value = anything else. Or is there a use case that can not be handled this way?

I am not sure if it is an issue related to the theme I was using, but when I used "visible if value = something", it did not make the fields invisible when a different value was selected. It only seemed to apply the "display: none;" style if I inverted the condition ("invisible if !value = something").

I can do that, but it's worth noting that the original action did not implement this either

That's correct. I brought it up because you have changed the value form field description accordingly. Therefore I think it would make sense to also add a state condition within this issue.

According to https://www.lakedrops.com/en/blog/drupals-modeler-api-released-learn-abo... version 3 of the BPMN modeler now supports Drupal form states:

This property panel is now replaced by the off-canvas container from Drupal core, and the configuration forms are native Drupal Form API forms. That allows for tailored form field widgets as well as states, so that individual fields can be hidden depending on context.

Applied suggestion.

This looks nice. I've rebased the MR so that tests should now be green. If no objections come up, this could be merged and then back ported to 2.1.x

If the value form field is visible, shouldn't it also be required (which would also require a server side validation)? Drupal usually stores the form field value if set, although the form field is invisible. Should the value be removed in such cases?

If the value form field is visible, shouldn't it also be required (which would also require a server side validation)?

That may be great, I guess it should also be possible. At least worth a try. I'll set this back to NW for this.

Drupal usually stores the form field value if set, although the form field is invisible. Should the value be removed in such cases?

I think that's not common practice, and it's not necessary if the rest of it is implemented correctly, i.e. the value will only be used when needed, i.e. it the type is either value or not value.

Also, it seems to be a benefit to keep the value, especially when you play around with different setting. In that case, it's rather positive if the value is kept should you later come back to that setting.

If the value form field is visible, shouldn't it also be required (which would also require a server side validation)?

Applied suggestion.

The code looks good and works including validation of the value configuration form field.

Although the same might be possible inverting the existing conditions, it does make sense especially in cases where you would have to check for a lot of values instead of excluding 1 value.

The label seems to be technical, maybe it would make sense to use "value NOT" instead of "NOT value". Anyway, setting this to RTBC.
@starlight-sparkle Thanks for that feature!

Merged and back ported to 2.1

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.