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: Add options.moveNodeStrategy configuration to NodeTypes #3876

Merged

Conversation

pKallert
Copy link
Contributor

@pKallert pKallert commented Oct 23, 2024

Resolves: #3587
Resolves partially: neos/neos-development-collection#5368

In order to mimic the behavior of 8.3 we decided to introduce a setting for move strategies on NodeType options.

In 8.3 when moving a document node, we (try to) move it in all dimensions. When moving a content node in 8.3 we do not try to move it in all dimensions.

Currently the move strategy is hard-coded in the Neos UI. With this PR we load the move strategy from the NodyType to specify it in the command.

This gives us the flexibility in the future to implement some custom move strategies in the UI but lets us keep the 8.3 behavior for now.

Further this neos pr removes the old aggregate flag fully as its totally unused but was in 8.3 indicator for this behaviour neos/neos-development-collection#5314

What I did

How I did it

How to verify it

@github-actions github-actions bot added Feature Label to mark the change as feature 9.0 labels Oct 23, 2024
@@ -34,6 +35,7 @@
nodeCreationHandlers:
promotedElements:
factoryClassName: 'Neos\Neos\Ui\Infrastructure\ContentRepository\CreationDialog\PromotedElementsCreationHandlerFactory'
moveNodeStrategy: scatter
Copy link
Member

Choose a reason for hiding this comment

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

as discussed today for content, especially as scatter also moves virtual variants the behaviour is correct

Still, specializations pointing to the same node using the fallback mechanism will be kept gathered.

@@ -24,6 +24,7 @@
factoryClassName: 'Neos\Neos\Ui\Infrastructure\Neos\UriPathSegmentNodeCreationHandlerFactory'
promotedElements:
factoryClassName: 'Neos\Neos\Ui\Infrastructure\ContentRepository\CreationDialog\PromotedElementsCreationHandlerFactory'
moveNodeStrategy: gatherAll
Copy link
Member

Choose a reason for hiding this comment

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

We also discussed that it might make sense to use gatherSpecializations or scatter on document level from time to time (to create different structures for each dimension). Maybe we need to introduce some ui dialog with a little "magnet" slider which dimensions to also gather

But we should also keep in mind that non gatherAll behaviour might be broken for the Neos Ui as it relies on the information here: https://github.com/neos/neos-development-collection/blob/a14bbe701335c013df85579d1703f9dd4b018cc5/Neos.Neos/Classes/Controller/Service/NodesController.php#L334-L335

$contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId);

$rawMoveNodeStrategy = $this->getNodeType($this->subject)?->getConfiguration('options.moveNodeStrategy');
Copy link
Member

Choose a reason for hiding this comment

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

this btw. should still be part of the schema @mhsdesign in Neos.Neos as otherwise you cannot validate it....

Copy link
Member

Choose a reason for hiding this comment

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

irigh zrk who actually uses the validator 😅 but if you think so ... are the validation things merged? then it can live here in the ui ... other wise id say no its not worth it. its not something anyone is gonna use.

Copy link
Member

Choose a reason for hiding this comment

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

😂 I think we cannot merge schemas? Lets skip it for now, but IMHO would be good to have somewhere...

@kitsunet kitsunet merged commit 4e3824a into neos:9.0 Nov 27, 2024
7 of 9 checks passed
@mhsdesign mhsdesign changed the title FEATURE: Automatically set nodemove strategy in core FEATURE: Add options.moveNodeStrategy configuration to NodeTypes Dec 1, 2024
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.0 Feature Label to mark the change as feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG 9.0 NodeMove should respect aggregate: true
3 participants