Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Apr 17, 2024

Summary

In case we remove a node for which the sidebar is currently open, we need to do two things:

  1. Close and reset the sidebar
  2. Change the current fileid in the current route

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

@susnux susnux added this to the Nextcloud 30 milestone Apr 17, 2024
@susnux susnux requested review from Pytal, artonge and emoral435 April 17, 2024 15:52
@susnux susnux requested a review from skjnldsv as a code owner April 17, 2024 15:52
@susnux
Copy link
Contributor Author

susnux commented Apr 17, 2024

/backport to stable29

@susnux
Copy link
Contributor Author

susnux commented Apr 17, 2024

/backport to stable28

Copy link
Contributor

@emoral435 emoral435 left a 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!

skjnldsv

This comment was marked as resolved.

Copy link
Member

@skjnldsv skjnldsv left a 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 :)

@susnux susnux force-pushed the fix/side-bar-share-removed branch from e0dd5e5 to 4e9facd Compare May 31, 2024 10:30
@susnux
Copy link
Contributor Author

susnux commented May 31, 2024

@skjnldsv I refactored the deleted node handling of the current view.
So instead of doing it within the action, the action does not care about it now, it is moved to the view like handling updated nodes.

@susnux susnux requested review from artonge and skjnldsv May 31, 2024 10:31
@susnux susnux requested a review from nfebe May 31, 2024 12:23
susnux added 3 commits June 4, 2024 12:40
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 <[email protected]>
@susnux susnux force-pushed the fix/side-bar-share-removed branch from 4e9facd to 21b96c0 Compare June 4, 2024 10:40
Signed-off-by: Ferdinand Thiessen <[email protected]>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 4, 2024
@skjnldsv skjnldsv merged commit dbf0d93 into master Jun 4, 2024
@skjnldsv skjnldsv deleted the fix/side-bar-share-removed branch June 4, 2024 11:43
@blizzz blizzz mentioned this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: UI bug when deleting a share

5 participants