-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
fix(files): Close sidebar and update fileid when current node is deleted #44897
Conversation
/backport to stable29 |
/backport to stable28 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense to me to put this here, as other features would most likely want to follow the same functionality. But if others decide to move this elsewhere, it is fine by me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait 🙈
Actually, I think we're mixing two different approaches here.
I would agree with Louis, whichever service handle the routing should monitor itself.
I think having the same type of handling as you did on the sidebar would be cleaner.
The action is not really responsible for the routing, it doesn't care. It deletes the file and send the event.
The routing is managed by the FileList, if a file disappear, it should be the one adjusting the fileid param.
If we put this into the navigation (still not sure it fits there) and we would then want to move to trashbin here later, then we have two router calls conflicting.
I fail to understand why we would have conflicting rule? If a file is removed from the list (moved or deleted), and the fileid is still in the URL, we should fallback to the folder fileid. What we're doing here on the deleteAction does also apply to the moveOrCopyAction :)
e0dd5e5
to
4e9facd
Compare
@skjnldsv I refactored the deleted node handling of the current view. |
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
We do not change the view to the trash bin but stay in the current view, so we need to update the current fileid on the route if that was deleted. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
4e9facd
to
21b96c0
Compare
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Summary
In case we remove a node for which the sidebar is currently open, we need to do two things:
This is needed because we do not change the route to the trashbin, but stay on the current route.
So we need to change the current fileid to the parent folder.
Checklist