BUG: NotEmptyValidator blocks saving Inspector changes, even if the field is hidden

[pKallert]

Description

Some fields have a hidden attribute that depends on other fields being checked or chosen. If those fields are declared mandatory with the NotEmptyValidator the whole node cannot be saved even if there is no property shown.

Example: The Neos shortcut has a field 'target' that is only shown when the target mode is 'selectedTarget'. Right now it is possible to create a shortcut without setting the target.

Steps to reproduce

1. Edit the Neos.Neos:Shortcut in Packages/Neos/Neos.Neos/Configuration/NodeTypes.Shortcut.yaml
2. Make the target field required so that Editors cannot create an empty shortcut

   target:
      type: string
      ui:
        label: i18n
        reloadPageIfChanged: true
        inspector:
          hidden: 'ClientEval:node.properties.targetMode === "selectedTarget" ? false : true'
          group: 'document'
          editor: 'Neos.Neos/Inspector/Editors/LinkEditor'
          editorListeners:
            setTargetModeIfNotEmpty:
              property: 'targetMode'
              handler: 'Neos.Neos/Inspector/Handlers/ShortcutHandler'
      validation:
        'Neos.Neos/Validation/NotEmptyValidator': [ ]

3. Create a new Shortcut in the Neos UI.

Expected Result

The 'target' should only be required if it is shown. Editors should be able to make changes without filling in the hidden required field.

Actual Result

The field is required even if it is not shown. Changes to the node cannot be applied:

Suggestions

I can think of three solutions we could apply here.

• Change the NotEmptyValidator to only apply to visible fields in the UI.
• Make it configurable if the NotEmptyValidator should only apply to visible fields
• Create a new validator that considers if a field is visible

Affected Versions

Neos: 8.3

UI: 8.3

[mhsdesign]

Hi, I just want to add some informations ;)

there is actually a plan to introduce the concept of required properties like

type: string
required: true

https://github.com/neos/neos-development-collection/issues/4631

this was intended as replacement for NotEmptyValidator and would be enforced directly by the content repository. That would also mean, that your proposed hidden logic could not be accounted for that low level.

My point is I actually want to move away from this super soft validaton (which is currently only triggered by the ui, and invisible to the cr) to a much stricter enforcement. And I’m not sure that these validators are the future and if we should introduce concepts like this to make nodes less strict.

(Our conditional hidden properties are a thorn in my eyes in general- even if I very much like them and use them - imagine your php model which is called ‚Thing‘ which has a lot of properties and it’s implicitly known that when x is set to bar that y must be also set … such models are not fun to work with and don’t tell from the best architecture, but in nodes we might not always have better options)

Edit: for now you might be able to implement a custom ValidateIfNotHidden validator or something;)

[grebaldi]

I'm changing this into a BUG issue, because the inspector actually isn't supposed to behave like it does:

I believe that validation should be entirely disabled (all of it, not just NotEmpty) if a field is hidden.

When a field is hidden, it is effectively inaccessible to the user. Therefore, any validation error that occurs cannot be acted upon, leaving the Inspector in an irreconcilably locked state and rendering all other pending changes lost.

[mhsdesign]

I think there is no good fix for this except a custom hacky validator which considers the hidden state. My reasoning is that validation depending on the ui.hidden state has no future in the new cr where we like to have real validation on pho land. Additionally as the property validation is also run on the server https://github.com/neos/neos-ui/pull/2351 the ui.hidden client eval cannot be taken into account and thus even if the ui allows this the server would deny the change.