Skip to content

Feature: Improved behavior when dragging items from an unfocused Files window #16508

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 10 commits into from
Dec 3, 2024

Conversation

Jack251970
Copy link
Contributor

@Jack251970 Jack251970 commented Nov 22, 2024

Resolved / Related Issues

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Opened Files and File Explorer.
  2. Select files in Files.
  3. Move File Explorer on the top, which is topper than Files. Make sure it doesn't cover Files so that you can drag files from Files.
  4. Drag the selected files to the File Explorer. And you can find the Files won't move to the top when dragging files.

@yaira2 yaira2 self-requested a review November 22, 2024 15:23
@yaira2 yaira2 changed the title Feature: Added support for not focusing Files window when dragging a file/folder (#13255) Feature: Added support for not focusing Files window when dragging a file/folder Nov 22, 2024
@yaira2 yaira2 requested a review from hishitetsu November 24, 2024 15:58
yaira2
yaira2 previously approved these changes Nov 24, 2024
@yaira2 yaira2 requested a review from 0x5bfa November 24, 2024 15:59
@yaira2
Copy link
Member

yaira2 commented Nov 24, 2024

@Jack251970 the behavior is working nicely when Files is maximized. Is there a way to also do this when Files isn't maximized?

@Jack251970
Copy link
Contributor Author

@yaira2 Sorry, I cannot get your meaning. What is the difference between the behaviour when File is maximized and when it is not?

@Jack251970
Copy link
Contributor Author

https://github.com/user-attachments/assets/91b17452-b706-47fe-be16-89460ec12508
I think the behaviour like this is enough compared to system File Explorer?

@yaira2
Copy link
Member

yaira2 commented Nov 25, 2024

@yaira2 Sorry, I cannot get your meaning. What is the difference between the behaviour when File is maximized and when it is not?

I meant that Files is brought to the foreground if the window isn't maximized.

@Jack251970
Copy link
Contributor Author

Jack251970 commented Nov 25, 2024

@yaira2 Sorry, I cannot get your meaning. What is the difference between the behaviour when File is maximized and when it is not?

I meant that Files is brought to the foreground if the window isn't maximized.

Sorry, still no idea about why Files needs to be brought to the foreground if the window isn't maximized. (Under what situation?)

As you can see in the video above, my codes implement a function that the Files will stay in the current z index (not be brought to front for covering another topper window that need to drag files into) if dragging selected files so that it will have the same experience as system File Explorer.

@0x5bfa
Copy link
Member

0x5bfa commented Nov 25, 2024

Putting aside the behavior, you can use DidPositionChanged from AppWindow.Changed event instead of utilizing win32 apis.

@Jack251970
Copy link
Contributor Author

Putting aside the behavior, you can use DidPositionChanged from AppWindow.Changed event instead of utilizing win32 apis.

I am not quite familiar with this api. Will it make some differences?

@yaira2
Copy link
Member

yaira2 commented Nov 25, 2024

@Jack251970 I'm referring to the behavior (around two seconds into your recording) where the Files window is brought to the foreground.

  1. Open Files regularly (not maximized)
  2. Select a file
  3. Switch the focus to File Explorer
  4. Start dragging the selected file from Files
  5. Files is brought to the foreground

Isn't the expected behavior for File Explorer to remain the topmost window?

@Jack251970
Copy link
Contributor Author

@Jack251970 I'm referring to the behavior (around two seconds into your recording) where the Files window is brought to the foreground.

  1. Open Files regularly (not maximized)
  2. Select a file
  3. Switch the focus to File Explorer
  4. Start dragging the selected file from Files
  5. Files is brought to the foreground

Isn't the expected behavior for File Explorer to remain the topmost window?

Get it. I will try to make this work.

@yaira2
Copy link
Member

yaira2 commented Nov 25, 2024

Thanks! I should note, your current changes already helped to resolve this issue when the window is maximized.

@0x5bfa
Copy link
Member

0x5bfa commented Nov 25, 2024

Will it make some differences?

Yeah you don't have to hook/unhook win32 window message loop, there's high-level API wrapper called WinRT API as opposed to low-level Win32 API.

This API let you just subscribe an event. Go to MainWindow and subscribe this.AppWindow.Changed event and get args.DidPositionChange property value.

@Jack251970
Copy link
Contributor Author

Will it make some differences?

Yeah you don't have to hook/unhook win32 window message loop, there's high-level API wrapper called WinRT API as opposed to low-level Win32 API.

This API let you just subscribe an event. Go to MainWindow and subscribe this.Changed event and get e.DidPositionChanged property value.

Sure, I will have a try.

@Jack251970
Copy link
Contributor Author

@yaira2 Sorry about the misunderstanding description (I have removed the useless step that asks you to maximize the Files window). This feature can work while Files isn't a maximized window.
In my recording around 2 seconds, the reason why Files is brought to the front is that I have selected a new item (09-20-37.mp4) instead of the original one (09-28-05.mp4). This behaviour is just like the system File Explorer.

@Jack251970
Copy link
Contributor Author

@0x5bfa Yeah I have found that AppWindow.Changed event contains information about DidPositionChange & ZOrderChange, but I cannot handle these changes. (AppWindowChangedEventArgs does not have Handle property, and I cannot stop those changing event)

So I think WindowMessageReceived event in WinUIEx.WindowManager is the only way to implement this and WinUIEx package has provided a good solution for this.

@yaira2
Copy link
Member

yaira2 commented Nov 25, 2024

@Jack251970 thank you for clarifying. I'll test again to confirm the behavior works on my side.

@0x5bfa
Copy link
Member

0x5bfa commented Nov 25, 2024

Yeah I have found that AppWindow.Changed event contains information about DidPositionChange & ZOrderChange, but I cannot handle these changes. (AppWindowChangedEventArgs does not have Handle property, and I cannot stop those changing event)

Thanks, I missed that. I'll file a feature request on WinAppSdk for this later.

@yaira2 yaira2 changed the title Feature: Added support for not focusing Files window when dragging a file/folder Feature: Improved behavior when dragging items from an unfocused Files window Nov 26, 2024
@yaira2
Copy link
Member

yaira2 commented Nov 26, 2024

@Jack251970 I can confirm the behavior is working as expected. I'm going to wait for a review from @hishitetsu, but as far as I'm concerned I have no requested changes.

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

This improvement does not work once the app goes to the background and then reopens.
I also found a few behaviors that differ from Explorer.

  • In Files, if you try to drag a file that is not selected, the Files window comes into focus; this is not the case in Explorer.
  • Once dragged to another window, returning the cursor and releasing the mouse button on the original window will focus the original window in Explorer, but not in Files.

@Jack251970
Copy link
Contributor Author

Jack251970 commented Nov 30, 2024

@hishitetsu Thank you for testing this function so carefully!

As for few behaviors that differ from Explorer above, I don not think it is necessary to ensure Files has the same behavior as the Explorer and I wonder what is the difference in user experience. (Sorry, I am not familiar with focus behaviors.)

For This improvement does not work once the app goes to the background and then reopens, do you mean close the Files window into tray icon and can you provide the reproduction steps? (I have test it when minimize File window into task bar, and I found it can work.)

@hishitetsu
Copy link
Member

can you provide the reproduction steps?

  1. Set Leave app running in the background when the window is closed option to on.
  2. Close (not minimize) the Files window.
  3. Click the task tray icon to reopen Files.
  4. Test the behavior.

@Jack251970
Copy link
Contributor Author

@hishitetsu Now fixed, I think it can work when you close window and keep it in the background.

@yaira2
Copy link
Member

yaira2 commented Dec 1, 2024

As for few behaviors that differ from Explorer above, I don not think it is necessary to ensure Files has the same behavior as the Explorer and I wonder what is the difference in user experience. (Sorry, I am not familiar with focus behaviors.)

For what it's worth, it's probably better if we only implement the behaviors as they are requested. Thanks for all your work!

@yaira2 yaira2 requested a review from hishitetsu December 2, 2024 22:15
yaira2
yaira2 previously approved these changes Dec 2, 2024
Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

It seems to work fine, but there are a few comments about the code quality.

@Jack251970
Copy link
Contributor Author

@hishitetsu Resolved.

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>
@yaira2
Copy link
Member

yaira2 commented Dec 3, 2024

@Jack251970 thank you for all your work on this improvement! @hishitetsu thank you for testing and reviewing the PR!

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Dec 3, 2024
@yaira2 yaira2 merged commit 29a3ce5 into files-community:main Dec 3, 2024
6 checks passed
@Jack251970 Jack251970 deleted the dev branch February 6, 2025 02:21
# for free to join this conversation on GitHub. Already have an account? # 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.

Feature: Don't focus Files window when dragging a file/folder
4 participants