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

fix: merge conflict dialog added when overwriting files on move-file action #43667

Merged
merged 4 commits into from
Feb 24, 2024

Conversation

emoral435
Copy link
Contributor

@emoral435 emoral435 commented Feb 20, 2024

Summary

  • When user decides to move file to new destination, if a file with that same name already exists, currently, our implementation overrides the old file and injects the new file

Resolution

  • In order to resolve this, use @nextcloud/upload to check if there is a conflict, and if there is, shows a conflict dialog to resolve how the user wishes to proceed with the conflicting dialogs
  • If the conflict dialog returns two empty arrays, selected and renamed, that means the user chose to keep the file already in the directory path, thereby choosing to delete the current selected file they want to move

Screenshots / GIF

firefox_1XwZZSoVxP

Checklist

@emoral435 emoral435 added this to the Nextcloud 29 milestone Feb 20, 2024
@emoral435 emoral435 self-assigned this Feb 20, 2024
@emoral435 emoral435 force-pushed the fix/43489/merge-conflict-dialog branch 3 times, most recently from 2077ddd to 65e080c Compare February 20, 2024 17:33
@emoral435 emoral435 marked this pull request as ready for review February 20, 2024 17:36
@Pytal
Copy link
Member

Pytal commented Feb 21, 2024

Not related to this change but the conflict dialog close button is focusable with the keyboard but unclickable
image
I believe this is an issue with the z-index

@emoral435
Copy link
Contributor Author

emoral435 commented Feb 21, 2024

Hm. Good observation, and I can replicate it. I will investigate this issue!

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.

Logic makes sense!

Signed-off-by: Eduardo Morales <[email protected]>
@emoral435 emoral435 force-pushed the fix/43489/merge-conflict-dialog branch from 429279d to b933b67 Compare February 24, 2024 02:15
Copy link
Contributor

Possible performance regression detected

Show Output
564 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test/new_file.txt
+ /remote.php/dav/files/test added with 45 queries
+ /remote.php/dav/files/test/test.txt added with 33 queries
+ /remote.php/dav/files/test/many_files added with 46 queries
+ /remote.php/dav/files/test/new_file.txt added with 67 queries
+ /remote.php/dav/files/test/new_file.txt added with 91 queries
+ /remote.php/dav/files/test added with 45 queries
+ /remote.php/dav/files/test/test.txt added with 33 queries
+ /remote.php/dav/files/test/many_files added with 46 queries
+ /remote.php/dav/files/test/new_file.txt added with 67 queries
+ /remote.php/dav/files/test/new_file.txt added with 91 queries

@emoral435 emoral435 merged commit 59f3c73 into master Feb 24, 2024
97 of 98 checks passed
@emoral435 emoral435 deleted the fix/43489/merge-conflict-dialog branch February 24, 2024 06:39
@solracsf
Copy link
Member

/backport to stable28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: When moving a folder, no merge dialog appears if the folder names are the same.
5 participants