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

Trash manager #14058

Merged
merged 19 commits into from Nov 7, 2018
Merged

Trash manager #14058

merged 19 commits into from Nov 7, 2018

Conversation

alroniks
Copy link
Collaborator

@alroniks alroniks commented Aug 23, 2018

See original PR here #13764 for details.

This is rebased version with additional improvements. Original will be closed after finish if work on improvements.

Template build required

The PR needs to run _build/templates/default/grunt build. Otherwise the hover state of the buttons is not visible and the changes in modx.tree.resource.js are not in the compressed scripts.

$this->success[] = $id;
}

// TODO this still has to be discussed: what happens to the children?
Copy link
Contributor

Choose a reason for hiding this comment

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

Issue #11973 might be sort of relevant here

Jako and others added 3 commits August 29, 2018 15:27
- Trash icon tooltip is updated on resource delete/undelete
- Trash icon is clickable now
- Pagetitle column tooltip contains some resource informations
- Search/Clear works now
- Manager controller moved to resource/trash
- Lexicon issues
- Button hover colors
@Jako
Copy link
Collaborator

Jako commented Aug 30, 2018

Checked, refactored, working fine.

Jako
Jako previously approved these changes Aug 30, 2018
@Jako Jako added area-core proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. pr/ready-for-merging Pull request reviewed and tested and ready for merging. labels Aug 30, 2018
@Jako Jako added this to the v2.7.0 milestone Aug 30, 2018
@Jako Jako changed the title [WIP] Trash manager Trash manager Aug 30, 2018
@alroniks
Copy link
Collaborator Author

I also want to do some refactoring in grid component, but right now I have a lack of time. So I removed state ready-for-merging

@alroniks alroniks removed the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Aug 31, 2018
@Jako Jako added the pr/review-needed Pull request requires review and testing. label Sep 11, 2018
Copy link
Collaborator

@Mark-H Mark-H left a comment

Choose a reason for hiding this comment

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

While it looks like there's still work being done, I'll go ahead and approve the current state. Looks really promising.

@opengeek
Copy link
Member

opengeek commented Nov 5, 2018

Is this ready for merge or not? It's approved for it but says it needs review and @Mark-H mentioned it looks like work is still ongoing? @alroniks ?

@alroniks
Copy link
Collaborator Author

alroniks commented Nov 5, 2018

I wanted to improve some in code, fix some smelling places, but it works well and can be merged, all further improvements I can add later. So feel free to merge. I don't want to block a release.

@Jako Jako added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Nov 6, 2018
@Jako Jako merged commit cc2438e into modxcms:2.x Nov 7, 2018
@alroniks alroniks deleted the trash-restored branch November 7, 2018 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core pr/ready-for-merging Pull request reviewed and tested and ready for merging. proposal Proposal about improvement aka RFC. Need to be discussed before start implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants