Skip to content

[3.x] Fix issue where moving resource above another would make the "target" a container. #16001

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

Merged
merged 3 commits into from
Feb 2, 2022

Conversation

JoshuaLuckers
Copy link
Contributor

What does it do?

Make sure we only mark the target as folder when the node is appended to another node.
If the point we are dragging/dropping the node is "above/below", the target resource is a sibling and not a parent and therefore it should not become a container/folder.

Why is it needed?

Fix issue where moving resource above another would make the "target" a container.

How to test

Drag and drop a resource above another resource that's not a container/folder.
Also see #15991

Related issue(s)/PR(s)

Resolves #15991

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Jan 25, 2022
Copy link
Collaborator

@Ruslan-Aleev Ruslan-Aleev left a comment

Choose a reason for hiding this comment

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

Seems to work as it should

@Mark-H
Copy link
Collaborator

Mark-H commented Feb 2, 2022

Made two minor code suggestions to make sure the validation actually does something, but I can confirm this resolves the issue with random assignments.

Co-authored-by: Mark Hamstra <[email protected]>
@opengeek opengeek merged commit de7abc9 into modxcms:3.x Feb 2, 2022
@opengeek
Copy link
Member

opengeek commented Feb 2, 2022

I think a botched trying to add brevity to the title of this merged change. It would be helpful if we can more concisely describe changes like this when submitting. I know it's not easy sometimes.

@JoshuaLuckers JoshuaLuckers deleted the issue-15991 branch February 2, 2022 18:01
@JoshuaLuckers
Copy link
Contributor Author

I think a botched trying to add brevity to the title of this merged change. It would be helpful if we can more concisely describe changes like this when submitting. I know it's not easy sometimes.

What would have been a better title?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[3.0.0-rc1] container flag is set seemingly randomly after dragdrop in resource tree
4 participants