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

feat: create new window when dragging tab outside the app #465

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

soumyamahunt
Copy link
Contributor

@soumyamahunt soumyamahunt commented Apr 24, 2020

Fixed SetDraggedOutside limit. Dragging any type of set item out side the app will launch a new instance and open that set in the instance.

PR Type

What kind of change does this PR introduce?

  • Feature

Other information

2020-04-24_23-47-54

@0x7c13
Copy link
Owner

0x7c13 commented Apr 24, 2020

Did you test if dragging a tab out when the file has been deleted or renamed? Try to see if it works.

@soumyamahunt
Copy link
Contributor Author

Did you test if dragging a tab out when the file has been deleted or renamed? Try to see if it works.

I fixed it to work when filecontent is modified outside of app. For renamed/moved/deleted I am adding their content to the new file, can't think of a better solution. Anything on mind??

@0x7c13
Copy link
Owner

0x7c13 commented Apr 24, 2020

Did you test if dragging a tab out when the file has been deleted or renamed? Try to see if it works.

I fixed it to work when filecontent is modified outside of app. For renamed/moved/deleted I am adding their content to the new file, can't think of a better solution. Anything on mind??

This is exactly why I am limiting it to empty file only.......... I could not make it work for deleted files scenarios.

@0x7c13
Copy link
Owner

0x7c13 commented Apr 24, 2020

Did you test if dragging a tab out when the file has been deleted or renamed? Try to see if it works.

I fixed it to work when filecontent is modified outside of app. For renamed/moved/deleted I am adding their content to the new file, can't think of a better solution. Anything on mind??

This is exactly why I am limiting it to empty file only.......... I could not make it work for deleted files scenarios.

If you cannot solve the problem, just disable the drag out in this case and re-pop the message to notify user that file has been moved or deleted (so he or she knows why dragging out of such tab won't work).

@soumyamahunt
Copy link
Contributor Author

If you cannot solve the problem, just disable the drag out in this case and re-pop the message to notify user that file has been moved or deleted (so he or she knows why dragging out of such tab won't work).

Fixed it.

src/Notepads/Core/NotepadsCore.cs Show resolved Hide resolved
src/Notepads/Views/NotepadsMainPage.IO.cs Outdated Show resolved Hide resolved
src/Notepads/Core/NotepadsCore.cs Outdated Show resolved Hide resolved
@0x7c13
Copy link
Owner

0x7c13 commented Apr 27, 2020

Another thing is that when you drag a tab outside to a desktop for example, it shows a "move" menu instead of creating the new instance directly and you have to manually cancel the menu before it can work. This needs to be solved otherwise the UX will be very confusing.

@soumyamahunt
Copy link
Contributor Author

soumyamahunt commented May 8, 2020

Another thing is that when you drag a tab outside to a desktop for example, it shows a "move" menu instead of creating the new instance directly and you have to manually cancel the menu before it can work. This needs to be solved otherwise the UX will be very confusing.

Fixed this, however some apps like VS Code and Firefox still able to intercept drop action even if the tab doesn't contain any file. Also, some downside of this fix is no more drag and drop sharing between notepads and other programs may be implementing "sharing" or "open with another app" features will be the way to go.

@0x7c13
Copy link
Owner

0x7c13 commented May 9, 2020

Another thing is that when you drag a tab outside to a desktop for example, it shows a "move" menu instead of creating the new instance directly and you have to manually cancel the menu before it can work. This needs to be solved otherwise the UX will be very confusing.

Fixed this, however some apps like VS Code and Firefox still able to intercept drop action even if the tab doesn't contain any file. Also, some downside of this fix is no more drag and drop sharing between notepads and other programs may be implementing "sharing" or "open with another app" features will be the way to go.

If this is going to break the sharing between notepads then I would say don't do it....

Another approach I think:

  1. Keep everything as it is: 1) User can still drag out empty tabs. 2) Dragging out tab which has a file to third party apps will still work. 3) Dragging between notepads is un-touched.

  2. Add one more menu item in TabContextFlyout and call it "Open in new window". Put 100% of your logic/changes there.

@soumyamahunt
Copy link
Contributor Author

  1. Keep everything as it is: 1) User can still drag out empty tabs. 2) Dragging out tab which has a file to third party apps will still work. 3) Dragging between notepads is un-touched.
  2. Add one more menu item in TabContextFlyout and call it "Open in new window". Put 100% of your logic/changes there.

I went for the opposite approach. I kept my implementation of tab drag and drop. To share files to other apps via drag and drop user can drag the PathIndicator to the other app. This will clear any ux confusion. Is this acceptable??

@0x7c13
Copy link
Owner

0x7c13 commented May 9, 2020

  1. Keep everything as it is: 1) User can still drag out empty tabs. 2) Dragging out tab which has a file to third party apps will still work. 3) Dragging between notepads is un-touched.
  2. Add one more menu item in TabContextFlyout and call it "Open in new window". Put 100% of your logic/changes there.

I went for the opposite approach. I kept my implementation of tab drag and drop. To share files to other apps via drag and drop user can drag the PathIndicator to the other app. This will clear any ux confusion. Is this acceptable??

Still feeling awkward since this is not a well established UX behavior. Since WinUI's TabView will eventually abandon the ListView based TabView and will use ItemRepeater for proper implementation. So we will be able to use it as well. At the time, we can get rid of the annoying drag n drop behavior brought by the ListViewBase. Once that happens, we can just point it to the code you have written. That's also why I suggest you to go with "Open in new window" route and keep everything as it is.

Btw, you have to make sure the file has been successfully moved and shown from one instance to another before you call "DeleteTextEditor", you can not assume the file write will work. I have seen it on my end that in some cases we are not able to write file to the package folder. This will get things even messier if you are taking this into account. This safety check can only be done with something like a background service and you sync with two instance on the handling of the drag n drop file. But this is too overkill for such small benefits.

Since Notepads already supports move from one to another. I would say let's focus on something else and revisit this one when WinUI's TabView gets a newer implementation so that we can borrow.

@soumyamahunt
Copy link
Contributor Author

Still feeling awkward since this is not a well established UX behavior.

The typical behavior of dragging tab outside of an app is to create new window of the app. This seems to be standard across various tabbed interfaces like browsers and editors. That's why I think implementing file transfer through setsitem drag-drop will confuse user.

Since WinUI TabView will eventually abandon the ListView based TabView and will use ItemRepeater for proper implementation. So we will be able to use it as well. At the time, we can get rid of the annoying drag n drop behavior brought by the ListViewBase. Once that happens, we can just point it to the code you have written. That's also why I suggest you to go with "Open in new window" route and keep everything as it is.

If you don't want to implement this now I would suggest to keep it open until WinUI 3 releases. Currently users can create a new window and drag the instance to that window I don't think cluttering the TabContextFlyout is necessary for this.

@soumyamahunt
Copy link
Contributor Author

@Jasonstein I have made this behavior consistent for tabs for which files are either renamed or moved. Hope this removes any kind of confusion. You can implement this whenever you want.

wuhFS9nDiM

LiBp3I8Sjm

@soumyamahunt soumyamahunt changed the title Fixed SetDraggedOutside limit. feat: Create new window when dragging tab outside the app Mar 29, 2021
@soumyamahunt soumyamahunt changed the title feat: Create new window when dragging tab outside the app feat: create new window when dragging tab outside the app Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants