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

Neos.Neos:Site NodeType should not show uriPathSegment property #4621

Closed
mhsdesign opened this issue Oct 15, 2023 · 7 comments · Fixed by #4826
Closed

Neos.Neos:Site NodeType should not show uriPathSegment property #4621

mhsdesign opened this issue Oct 15, 2023 · 7 comments · Fixed by #4826
Labels

Comments

@mhsdesign
Copy link
Member

We briefly discussed this in our weekly:
Actually the uriPathSegment property should not be part of the homepage.
It would be rather nasty to remove the property from this type even though it is declared in the super type.. But I would say that this is the most pragmatic solution in this case. If the following works:

Neos.Neos:Site:
  'Neos.Neos:Document': true
  properties:
    'uriPathSegment': ~

(untested!)

Otherwise we should at least disable the UI editor for this property

Originally posted by @bwaidelich in #4563 (comment)

@mhsdesign
Copy link
Member Author

Unsetting properties is currently not possible see tests: #4620

But we could allow this see: #4618

This still has to be discussed and it might not be a good idea as @kitsunet stated:

I am not a fan, as said on Slack, I think if something proclaims to be isOfType($nodeTypeName) one should also be able to expect the set of properties from said $nodeTypeName

@mhsdesign
Copy link
Member Author

@bwaidelich wrote

But maybe we can at least hide it from the inspector by default?

@mhsdesign mhsdesign added the 9.0 label Oct 16, 2023
@mhsdesign
Copy link
Member Author

mhsdesign commented Oct 20, 2023

proposal from @mficzel

Neos.Neos:Mixin.UriPathSegment:
  properties:
    uriPathSegment:

'Neos.Neos:Document':
  superTypes:
    Neos.Neos:Mixin.UriPathSegment: true

Neos.Neos:Site:
  superTypes:
    'Neos.Neos:Document': true
    'Neos.Neos:Mixin.UriPathSegment': false
  • UriPathSegment has a intransitive dependency on the composing nodetype that the inspector group "document"
  • the i18n translation needs to be moved

additionall for another time we could add a Neos.Neos:Folder with 'Neos.Neos:Mixin.UriPathSegment': false

and @kitsunet noted to forbid changing inherited types -> maybe only allow to get narrower? -> new issue.

@mhsdesign mhsdesign self-assigned this Oct 27, 2023
@mhsdesign mhsdesign removed their assignment Dec 8, 2023
@mhsdesign
Copy link
Member Author

  • The node creation processor should not set the value to "home" (with condition instance of Neos.Neos:UriPathSegment)
  • Uri path projection should check against UriPathSegment

hiding the property would possibly result in validation error as its hidden: neos/neos-ui#3549 and also its not clean to be able to set this property via php api.

We came to the conclusion that unsetting supertypes is not necessary clean, but its a feature an we might as well use it for this.

 properties:
     uriPathSegment:
       ui:
         inspector:
           hidden: true

@mhsdesign
Copy link
Member Author

Oh no. I just found out that the uriPathSegment property still exists for node types inheriting from Neos.Neos:Site as that will not be inherited?

'Neos.Demo:Document.Homepage':
  superTypes:
    'Neos.Neos:Site': true

this Homepage for example WILL show the uriPathSegment flow nodetypes:show Neos.Demo:Document.Homepage --path properties.uriPathSegment while Neos.Neos:Site doesnt.
One would have to add here 'Neos.Neos:UriPathSegment': false as well explicitly. But this is of course unacceptable.

@kitsunet
Copy link
Member

I still think it would make sense to create a new AbstractDocument or something without uriPath and add it for the regular Document or a new DocumentWithUri and Document becomes the base one, either way people would have to refactor something though.

@mhsdesign
Copy link
Member Author

mhsdesign commented Jan 17, 2024

As removing a nodeType from the inheritance only works for the current (local) nodeType and is not further inherited, see:

foreach ($nodeType->declaredSuperTypes as $superTypeName => $superType) {
if ($superType === null) {
unset($flattenedSuperTypes[$superTypeName]);
}
}

we decided to against a mixin based solution for now. Also introducing a DocumentWithUri right now doesnt make much sense without a full concept for Folders #4829 in mind.

I will reopen my initial simpler pr which would just hide the property in the ui. #4826 and close #4828

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
2 participants