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.

Sign up for GitHub

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

Jump to bottom

Bug: Paste of multiple content nodes results in wrong order #3040

Closed
gjwnc opened this issue Feb 23, 2022 · 5 comments
Closed

Bug: Paste of multiple content nodes results in wrong order #3040

gjwnc opened this issue Feb 23, 2022 · 5 comments
Assignees
@Sebobo
Labels
7.3 8.3 Bug Label to mark the change as bugfix UI & UX

Comments

@gjwnc
Copy link
Contributor

gjwnc commented Feb 23, 2022 •
edited
Loading

Description

If you copy multiple content nodes from one document and paste them in another document the order of the copied content nodes can get somewhat mixed up.

The actual paste is done by /neos/ui-services/change, but I think the request payload provided has a wrong order. Thus I think it's an issue of the JS part of the neos ui - see below for request details.

Steps to Reproduce

  1. Open the backend UI and copy content nodes from a document
  2. Paste them in another document

Expected behavior

The pasted order is preserved.

Actual behavior

The order depends on where you paste it in the target document:

  1. Paste before some other content node - This works
  2. Paste after some content node - The order is reversed
  3. Click the main content collection of the target document and just click the paste button - The order is mixed in between content elements
Captured requests

I'm not quite sure what exactly is going on in case of the Mixed order, but for the CopyAfter operation, the set of nodes to paste should be reversed in the request payload.

If you look at the working request, that is the CopyBefore operation, the reason why the order is correct is, that the copied nodes are copied before the siblingDomAddress in the order given. Thus the same order appears.
In case of the CopyAfter, let's assume we want to copy nodes n1,n2,n3 in that order after node c. After the first operation we have the order c,n1. Looks good. Then n2 is copied after c, thus the order c, n2, n1 appears. The same with n3 and we get the final reversed order c, n3, n2, n1.

At least, that's what I think happens.

Working request 
{
  "changes": [
  	{
  		"payload": {
  			"baseNodeType": "",
  			"parentDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  				"fusionPath": "root<Neos.Fusion:Case>/documentType<Neos.Fusion:Matcher>/element<Webco.Fou001:Document.Article>/body<Neos.Fusion:Array>/content<Webco.Fou001:Component.Template.Article>/main<Neos.Neos:PrimaryContent>/default<Neos.Fusion:Matcher>/renderer"
  			},
  			"siblingDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main/node-i5r8tys0jngkm@user-gj;language=en,all"
  			}
  		},
  		"subject": "/sites/hub/node-qq1qu5bhs68k5/node-np2yc2v4ue6jf/node-ki3oy5drp9bv4/node-bi4u02u3lhfm4/main/node-8smjfu7zyxjpc@user-gj;language=en,all",
  		"type": "Neos.Neos.Ui:CopyBefore"
  	},
  	{
  		"payload": {
  			"baseNodeType": "",
  			"parentDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  				"fusionPath": "root<Neos.Fusion:Case>/documentType<Neos.Fusion:Matcher>/element<Webco.Fou001:Document.Article>/body<Neos.Fusion:Array>/content<Webco.Fou001:Component.Template.Article>/main<Neos.Neos:PrimaryContent>/default<Neos.Fusion:Matcher>/renderer"
  			},
  			"siblingDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main/node-i5r8tys0jngkm@user-gj;language=en,all"
  			}
  		},
  		"subject": "/sites/hub/node-qq1qu5bhs68k5/node-np2yc2v4ue6jf/node-ki3oy5drp9bv4/node-bi4u02u3lhfm4/main/node-a5991ukjekq57@user-gj;language=en,all",
  		"type": "Neos.Neos.Ui:CopyBefore"
  	}
  ]
}
Reverse order 
{
  "changes": [
  	{
  		"payload": {
  			"baseNodeType": "",
  			"parentDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  				"fusionPath": "root<Neos.Fusion:Case>/documentType<Neos.Fusion:Matcher>/element<Webco.Fou001:Document.Article>/body<Neos.Fusion:Array>/content<Webco.Fou001:Component.Template.Article>/main<Neos.Neos:PrimaryContent>/default<Neos.Fusion:Matcher>/renderer"
  			},
  			"siblingDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main/node-i888xtewx12b1@user-gj;language=en,all"
  			}
  		},
  		"subject": "/sites/hub/node-qq1qu5bhs68k5/node-np2yc2v4ue6jf/node-ki3oy5drp9bv4/node-bi4u02u3lhfm4/main/node-8smjfu7zyxjpc@user-gj;language=en,all",
  		"type": "Neos.Neos.Ui:CopyAfter"
  	},
  	{
  		"payload": {
  			"baseNodeType": "",
  			"parentDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  				"fusionPath": "root<Neos.Fusion:Case>/documentType<Neos.Fusion:Matcher>/element<Webco.Fou001:Document.Article>/body<Neos.Fusion:Array>/content<Webco.Fou001:Component.Template.Article>/main<Neos.Neos:PrimaryContent>/default<Neos.Fusion:Matcher>/renderer"
  			},
  			"siblingDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main/node-i888xtewx12b1@user-gj;language=en,all"
  			}
  		},
  		"subject": "/sites/hub/node-qq1qu5bhs68k5/node-np2yc2v4ue6jf/node-ki3oy5drp9bv4/node-bi4u02u3lhfm4/main/node-a5991ukjekq57@user-gj;language=en,all",
  		"type": "Neos.Neos.Ui:CopyAfter"
  	}
  ]
}
Mixed in between 
{
  "changes": [
  	{
  		"payload": {
  			"baseNodeType": "",
  			"parentContextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  			"parentDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  				"fusionPath": "root<Neos.Fusion:Case>/documentType<Neos.Fusion:Matcher>/element<Webco.Fou001:Document.Article>/body<Neos.Fusion:Array>/content<Webco.Fou001:Component.Template.Article>/main<Neos.Neos:PrimaryContent>/default<Neos.Fusion:Matcher>/renderer"
  			}
  		},
  		"subject": "/sites/hub/node-qq1qu5bhs68k5/node-np2yc2v4ue6jf/node-ki3oy5drp9bv4/node-bi4u02u3lhfm4/main/node-8smjfu7zyxjpc@user-gj;language=en,all",
  		"type": "Neos.Neos.Ui:CopyInto"
  	},
  	{
  		"payload": {
  			"baseNodeType": "",
  			"parentContextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  			"parentDomAddress": {
  				"contextPath": "/sites/hub/node-u2lv5o25g5ncd/node-2xvtpsap7me4d/node-kyzm5axp32nmn/main@user-gj;language=en,all",
  				"fusionPath": "root<Neos.Fusion:Case>/documentType<Neos.Fusion:Matcher>/element<Webco.Fou001:Document.Article>/body<Neos.Fusion:Array>/content<Webco.Fou001:Component.Template.Article>/main<Neos.Neos:PrimaryContent>/default<Neos.Fusion:Matcher>/renderer"
  			}
  		},
  		"subject": "/sites/hub/node-qq1qu5bhs68k5/node-np2yc2v4ue6jf/node-ki3oy5drp9bv4/node-bi4u02u3lhfm4/main/node-a5991ukjekq57@user-gj;language=en,all",
  		"type": "Neos.Neos.Ui:CopyInto"
  	}
  ]
}

Affected Versions

Neos: 7.2 - presumably earlier versions

UI:
@gjwnc gjwnc changed the title Paste of multiple content nodes results in wrong order Bug: Paste of multiple content nodes results in wrong order Feb 23, 2022
@gjwnc
Copy link
Contributor Author

gjwnc commented Sep 22, 2022

@markusguenther Maybe we can provide a PR for this, at least for some parts of it.

But first I would like to know if you, like I do, think the fix should go into the NEOS UI and not in the mentioned service /neos/ui-services/change.

I ask, because it might be possible to fix this in the service, but imho the service should not make a decision and rearrange the order to insert which might cause more confusion. What do you think?

@bielerj
Copy link

bielerj commented Apr 29, 2023 •
edited
Loading

The actual paste is done by /neos/ui-services/change, but I think the request payload provided has a wrong order. Thus I think it's an issue of the JS part of the neos ui - see below for request details.

We actually found that the payload seems to have the right order.
[Edit]: The payload does appear to have a reversed order when the nodes are selected from bottom to top.

The Problem lies within the anchor (siblingDomAddress) beeing used to insert the nodes. The siblingDomAddress passed via each copyAfter event is always the same (as shown in your example above). If we would like to keep the current style of sending multiple copy events we would have to adjust the siblingDomAddress for the n+1 copy events which is not possible as we dont know the sibling which will be created in the future.

To us the solution seems to lie in having a copyAfter event wich can hold multiple copied nodes instead of having multiple events. As far as i can think this should also fix the problem 3.

Tested in Neos 8.1

@bielerj
Copy link

bielerj commented Apr 29, 2023

Running into the same issue when moving nodes in batches.

@bielerj bielerj mentioned this issue Apr 29, 2023
Open
@bielerj
Copy link

bielerj commented Apr 29, 2023

Workaround for editors: select nodes in bottom to top order when inserting after or top to bottom order when inserting before.

@grebaldi grebaldi added Bug Label to mark the change as bugfix UI & UX 7.3 8.3 labels May 1, 2023
@Sebobo Sebobo self-assigned this Feb 1, 2024
Sebobo added a commit that referenced this issue Feb 1, 2024
@Sebobo
BUGFIX: Insert nodes in order of selection on „insertAfter“
2182ed6 
Resolves: #3040
Sebobo added a commit that referenced this issue Feb 1, 2024
@Sebobo
BUGFIX: Prevent index mixup when inserting nodes „into“ a node with c…
5f10fef 
…hildnodes

Without this change the copied nodes get their „index“ copied too,
which could position them in between existing nodes and creating a
seemingly random result for the editor.

Resolves: #3040
@Sebobo Sebobo mentioned this issue Feb 1, 2024
Merged
@mhsdesign
Copy link
Member

For visualisation, this is the common error encountered. And it is especial noticeable because every step that would lead to the result is exactly as we humans think :D

We select from top to bottom:

image

We insert after:

image

But its in reverse:

image

if it was selected from bottom to top it would be the correct order.
if insert before would be chosen it would be the correct order.

For extra special effects use command click and select each node in a random pattern, see that pattern be preserved and depending on after or before be reversed.

Sebobo added a commit that referenced this issue Feb 7, 2024
@Sebobo
BUGFIX: Reverse items added to clipboard if they are selected from bo…
5ed9fe9 
…ttom to top

Relates: #3040
Sebobo added a commit that referenced this issue Apr 10, 2024
@Sebobo
BUGFIX: Insert nodes in order of selection on „insertAfter“
29d18d5 
Resolves: #3040
Sebobo added a commit that referenced this issue Apr 10, 2024
@Sebobo
BUGFIX: Prevent index mixup when inserting nodes „into“ a node with c…
445e275 
…hildnodes

Without this change the copied nodes get their „index“ copied too,
which could position them in between existing nodes and creating a
seemingly random result for the editor.

Resolves: #3040
Sebobo added a commit that referenced this issue Apr 10, 2024
@Sebobo
BUGFIX: Reverse items added to clipboard if they are selected from bo…
157521c 
…ttom to top

Relates: #3040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Sebobo Sebobo

Labels
7.3 8.3 Bug Label to mark the change as bugfix UI & UX
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Sebobo @grebaldi @gjwnc @mhsdesign @bielerj