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

FEATURE: Placeholder-Insert: Exclude specific NodeTypes from the drop… #100

Closed
wants to merge 3 commits into from

Conversation

erkenes
Copy link

@erkenes erkenes commented Sep 6, 2021

Load all form-elements recursively. Check if the elements are allowed to shown in the
Placeholder-Insert-Dropdown.
Disallow Neos.Form.Builder:ElementCollection and Neos.Form.Builder:ValidatorCollection by default. (The section is excluded in the yaml file)

Resolves: #99

…down

Load all form-elements recursively. Check if the elements are allowed to shown in the
Placeholder-Insert-Dropdown.
Disallow `Neos.Form.Builder:ElementCollection` and `Neos.Form.Builder:ValidatorCollection` by default. (The section is excluded in the yaml file)

Resolves: neos#99
# excludeNodeTypes:
# 'Neos.Form.Builder:ElementCollection':
# exclude: true # excludes only the element
# excludeChildren: false # includes all child-elements
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"children" is the wrong notion here. A node has child nodes, a node type has super types.
I would suggest to simplify the syntax to a single list of true/false values like we do it in other places (e.g. node type constraints):

excludeNodeTypes:
  'Neos.Form.Builder:ElementCollection': true
  'Some.Specific:ElementCollection': false

And, to make it even more readable, we could swap the "deny list" to an "allow list", as in:

nodeTypes:
  'Neos.Form.Builder:ElementCollection': false
  'Some.Specific:ElementCollection': false

Copy link
Author

@erkenes erkenes Sep 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think only to have the simple syntax is not a good solution because my solution allows your objection and an easier configuration.

A boolean value allows to hide or show the entire element and its child nodes.

nodeTypes:
  'Neos.Form.Builder:ElementCollection': false

(based on your objection this would exclude a node of this node type)

Or with the more specific syntax, the entire element can be hidden, but not his child nodes or just the child nodes.

nodeTypes:
  # hides the node type, shows the child nodes
  'Neos.Form.Builder:ElementCollection':
    excludeNodeType: true
    excludeChildNodes: false

  # shows the node type, hides the child nodes
  'Neos.Form.Builder:ElementCollection':
    excludeNodeType: false
    excludeChildNodes: show

The complex syntax is helpful for the validators or the SelectOptionCollection, for example, since only the validator collection has to be hidden here and not every single validator.

Thats the configuration I'm using currently in my project (maybe that helps more to understand why I solved it that way):

excludeNodeTypes:
  'Foo.Bar.FormBuilder:Grid': &includeOnlyChildren
    exclude: true
    excludeChildren: false
  'Foo.Bar.FormBuilder:Column': *includeOnlyChildren
  'Foo.Bar.FormBuilder:NavigationNext': true
  'Foo.Bar.FormBuilder:NavigationPrevious': true
  'Foo.Bar.FormBuilder:Image': true
  'DL.HoneypotFormField:HoneypotField': true
  'Neos.Form.Builder:StaticText': true
  'Neos.Form.Builder:ValidatorCollection': true
  'Neos.Form.Builder:ElementCollection': *includeOnlyChildren
  'Neos.Form.Builder:SelectOptionCollection': *includeOnlyChildren

In the new syntax it would look like this:

nodeTypes:
  'Foo.Bar.FormBuilder:Grid': &includeOnlyChildNodes
    includeNodeType: false
    includeChildNodes: true
  'Foo.Bar.FormBuilder:Column': *includeOnlyChildNodes
  'Foo.Bar.FormBuilder:NavigationNext': false
  'Foo.Bar.FormBuilder:NavigationPrevious': false
  'Foo.Bar.FormBuilder:Image': true
  'DL.HoneypotFormField:HoneypotField': false
  'Neos.Form.Builder:StaticText': false
  'Neos.Form.Builder:ValidatorCollection': false
  'Neos.Form.Builder:ElementCollection': *includeOnlyChildNodes
  'Neos.Form.Builder:SelectOptionCollection': *includeOnlyChildNodes

Adjust the syntax to the basic syntax of neos
@erkenes
Copy link
Author

erkenes commented Sep 6, 2021

I've updated the PR. Now I'm using the basic syntax of neos.

The settings to hide or show a NodeType are now set in editorOptions.nodeTypes.
'Neos.Form.Builder:StaticText': false: hides this NodeType
'Foo.Bar.FormBuilder:Image': true: shows the NodeType

With the complex synax:

nodeTypes:
  'Foo.Bar.FormBuilder:Grid':
    includeNodeType: false # hides this NodeType
    includeChildNodes: true # shows the child-nodes

  'Neos.Form.Builder:SelectOptionCollection':
    includeNodeType: true # show this NodeType
    includeChildNodes: false # hides all child-nodes

…tions

Adjusts the comments to the new syntax
@erkenes erkenes requested a review from bwaidelich September 6, 2021 15:19
@erkenes
Copy link
Author

erkenes commented Nov 17, 2021

@bwaidelich Can you check the PR again please? :)

@bwaidelich
Copy link
Member

Hey,
thanks for the reminder.
I'm still a bit puzzled by the syntax, but that's probably mainly due to the fact that I have never used the insert placeholder feature (so far) and because I wasn't involved in its implementation. @dimaip is probably a much better person to judge

@bwaidelich bwaidelich requested review from dimaip and removed request for bwaidelich November 17, 2021 10:41
@erkenes
Copy link
Author

erkenes commented Mar 2, 2023

revoke this pr in favor of #128

@erkenes erkenes closed this Mar 2, 2023
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.

Placeholder-Insert: Only elements of the first level are displayed
2 participants