Skip to content

Added drop support to the NavigationBar #1507

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 14 commits into from
Jul 23, 2020

Conversation

soumyamahunt
Copy link
Contributor

@soumyamahunt soumyamahunt commented Jul 19, 2020

As requested in #634, you can drop files/folders in navigationbar items to move that file/folder to that path. Demo:

ILBqCV5Bkx

@ghost ghost added the needs - code review label Jul 19, 2020
@soumyamahunt
Copy link
Contributor Author

Currently, after I drop on an item the item is highlighted, I am working on a fix for it. May be related to #1052.

yaira2
yaira2 previously approved these changes Jul 19, 2020
@yaira2 yaira2 requested a review from tsvietOK July 19, 2020 14:18
Copy link
Contributor

@tsvietOK tsvietOK left a comment

Choose a reason for hiding this comment

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

When moving file to the same directory it create a new file with the same name with added '- copy". I think this operation should not be available.

@yaira2 yaira2 added changes requested Changes are needed for this pull request and removed needs - code review labels Jul 19, 2020
@soumyamahunt
Copy link
Contributor Author

When moving file to the same directory it create a new file with the same name with added '- copy". I think this operation should not be available.

Fixed that, already existing items are skipped.

@soumyamahunt soumyamahunt requested a review from tsvietOK July 19, 2020 20:09
@tsvietOK
Copy link
Contributor

I am still able to drop item to last navigation bar path item

@soumyamahunt
Copy link
Contributor Author

I am still able to drop item to last navigation bar path item

Does dropping create a duplicate file as you stated before??

@tsvietOK
Copy link
Contributor

No, it is doing nothing. But text "Move to" still exists, so it could mislead user.
image

@soumyamahunt
Copy link
Contributor Author

No, it is doing nothing. But text "Move to" still exists, so it could mislead user.
image

That is the default Windows Explorer behavior, when you drag items to any path items it displays the move to text.
explorer_qweO4voqXM
May be I should add a dialog to user explaining the move operation cannot complete.

@tsvietOK
Copy link
Contributor

tsvietOK commented Jul 19, 2020

Or just remove this operation from last path item? If it is not possible or too hard, add a dialog.

@yaira2 yaira2 added needs - code review and removed changes requested Changes are needed for this pull request labels Jul 20, 2020
@soumyamahunt
Copy link
Contributor Author

Or just remove this operation from last path item?

The reason I am deciding against that is because user might want to drop files to the pathitems from another instance or another app in case if their gridview is blocked by another window. This is the default behavior in Windows Explorer.

If it is not possible or too hard, add a dialog.

I tried to add a dialog but the method failed with RPC_E_WRONG_THREAD exception. Queuing the task in ui thread using dispatcher also failed. Similar behavior implemented for dragging a parent folder to child folder also fails. So I have disabled both methods for now until a solution is found.

Currently, I have implemented to check if item already exist in the folder in DragOver method.

Copy link
Contributor

@tsvietOK tsvietOK left a comment

Choose a reason for hiding this comment

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

If check of an item already exist in the folder now implemented, user dialog is no longer needed. Please, remove it.

@tsvietOK tsvietOK added changes requested Changes are needed for this pull request and removed needs - code review labels Jul 21, 2020
@soumyamahunt
Copy link
Contributor Author

If check of an item already exist in the folder now implemented, user dialog is no longer needed. Please, remove it.

I have commented out the dialog part, if any workaround for the issue is found, then it can be reenabled.

@soumyamahunt soumyamahunt requested a review from tsvietOK July 21, 2020 13:54
@soumyamahunt soumyamahunt requested a review from tsvietOK July 23, 2020 06:17
@tsvietOK tsvietOK added needs - code review and removed changes requested Changes are needed for this pull request labels Jul 23, 2020
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Jul 23, 2020
@yaira2 yaira2 changed the title Added drop support for navigationbar Added drop support to the NavigationBar Jul 23, 2020
@yaira2 yaira2 merged commit 060b39d into files-community:master Jul 23, 2020
@soumyamahunt soumyamahunt deleted the navigationbar-dragdrop branch July 23, 2020 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants