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

[FancyZones] App zone history on multiple desktops #3932

Conversation

vldmr11080
Copy link
Contributor

Summary of the Pull Request

  1. Add support to save zone history over multiple desktops for single app.
  2. After virtual desktop is deleted, remove app zone history as well.

PR Checklist

Validation Steps Performed

Multiple desktops app zone history:

  1. Create more than one desktop and assign some layout to them.
  2. Assign two instances of the same application to different desktops.
  3. Close those instances.
  4. Re-open them again.
  5. Check C:\Users\<user>\AppData\Local\Microsoft\PowerToys\FancyZones files app-zone-history.json.

Expected result: Information about zone history of single application contains information for different desktops, after application is opened, it goes to the different zone based on which desktop it is.

App zone history data of deleted desktop:

  1. Create more than one desktop and assign some layout to them.
  2. Assign some applications into zones on all desktops.
  3. Delete one of them.
  4. Check C:\Users\<user>\AppData\Local\Microsoft\PowerToys\FancyZones files app-zone-history.json and zones-settings.json.

Expected result: No data belonging to deleted desktops is found.

@enricogior enricogior requested review from yevhenii44-zz and ivan100sic and removed request for PrzemyslawTusinski June 1, 2020 07:04
@enricogior enricogior added the Product-FancyZones Refers to the FancyZones PowerToy label Jun 1, 2020
@enricogior
Copy link
Contributor

@vldmr11080
if you have more commits to come, please change this to a draft PR to avoid being reviewed before it's ready.

@vldmr11080
Copy link
Contributor Author

vldmr11080 commented Jun 1, 2020

@vldmr11080
if you have more commits to come, please change this to a draft PR to avoid being reviewed before it's ready.

No, last few of them were just minor changes in unit tests, and they are finished now. Core JSONHelpers implementation is finalized already.

@SeraphimaZykova
Copy link
Collaborator

SeraphimaZykova commented Jun 1, 2020

Is it expected that in app-zone-history was erased all history for app that was removed from one of layouts?

Repro steps:

  • Apply layout
  • Create VD
  • Snap window to both layouts
  • Remove window from any layout

App will be removed from all layouts.

@vldmr11080
Copy link
Contributor Author

vldmr11080 commented Jun 1, 2020

Is it expected that in app-zone-history was erased all history for app that was removed from one of layouts?

Repro steps:

  • Apply layout
  • Create VD
  • Snap window to both layouts
  • Remove window from any layout

App will be removed from all layouts.

Thanks for the catch! It is not intended for sure, I'm looking into it. Solved!

Copy link
Collaborator

@SeraphimaZykova SeraphimaZykova left a comment

Choose a reason for hiding this comment

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

Nice work! App history is saved for different VDs properly.
I played with it and tried besides described behavior removing VD while PT is not running and for me all scenarios works well.

Copy link
Contributor

@ivan100sic ivan100sic left a comment

Choose a reason for hiding this comment

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

Both validation scenarios work. I also checked that settings from v0.18.1 are migrated correctly. Unit tests work.

Copy link
Contributor

@yevhenii44-zz yevhenii44-zz left a comment

Choose a reason for hiding this comment

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

LGTM
Tested, both scenarios works.

@vldmr11080 vldmr11080 force-pushed the clear-app-zone-history-from-deleted-destkop-data branch from edaa7e0 to 2c25a3f Compare June 4, 2020 14:52
@enricogior
Copy link
Contributor

This fixes the virtual desktop issue, we still have to fix history for multi monitor.

@vldmr11080 vldmr11080 merged commit b0b2d3a into microsoft:master Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-FancyZones Refers to the FancyZones PowerToy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants