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

Added Image Resizer Settings #2324

Merged
11 commits merged into from
Apr 27, 2020

Conversation

ghost
Copy link

@ghost ghost commented Apr 22, 2020

Summary of the Pull Request

  • Added Image Resizer Settings
  • I will be adding tests during the course of the day.

References

PR Checklist

@ghost ghost requested review from arjunbalgovind, a team, yuyoyuppe and enricogior April 22, 2020 15:51
@ghost ghost added the Product-Settings The standalone PowerToys Settings application label Apr 22, 2020
This was linked to issues Apr 22, 2020
@traies traies self-requested a review April 22, 2020 18:25
@arjunbalgovind
Copy link
Contributor

I'll try this locally on my system to make sure the changes work since I already have the older JSON settings saved in the AppData folder.

Copy link
Contributor

@arjunbalgovind arjunbalgovind left a comment

Choose a reason for hiding this comment

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

I wasn't able to change the following settings (it did not make any change on the JSON file, so if you switch to another powertoys settings and come back it would be reset)

  • Sizes (editing names or values or even adding sizes did not make any change) [Even after clicking Save Sizes)
  • Fallback encoder

Also for sizes what is the expected behaviour when a new size is added/an existing size is modified while the ImageResizer app is open? Will the Image Resizer UI refresh to show the updated sizes? It was like that on the earlier version.

@ghost
Copy link
Author

ghost commented Apr 25, 2020

I wasn't able to change the following settings (it did not make any change on the JSON file, so if you switch to another powertoys settings and come back it would be reset)

  • Sizes (editing names or values or even adding sizes did not make any change) [Even after clicking Save Sizes)
  • Fallback encoder

Also for sizes what is the expected behaviour when a new size is added/an existing size is modified while the ImageResizer app is open? Will the Image Resizer UI refresh to show the updated sizes? It was like that on the earlier version.

Can you verify this on your machine thanks: fa7741b

@ghost
Copy link
Author

ghost commented Apr 25, 2020

I wasn't able to change the following settings (it did not make any change on the JSON file, so if you switch to another powertoys settings and come back it would be reset)

  • Sizes (editing names or values or even adding sizes did not make any change) [Even after clicking Save Sizes)
  • Fallback encoder

Also for sizes what is the expected behaviour when a new size is added/an existing size is modified while the ImageResizer app is open? Will the Image Resizer UI refresh to show the updated sizes? It was like that on the earlier version.

With regards to syncing: I cannot load a .NetFramwork based application into a .Netcore application. Syncing settings would require tracking the JSON file which is something we are planning to do in the future. Twoway IPC makes no sense to use it since we are going to deprecate it. @crutkas any guidance on this?

@ghost
Copy link
Author

ghost commented Apr 25, 2020

I wasn't able to change the following settings (it did not make any change on the JSON file, so if you switch to another powertoys settings and come back it would be reset)

  • Sizes (editing names or values or even adding sizes did not make any change) [Even after clicking Save Sizes)
  • Fallback encoder

Also for sizes what is the expected behaviour when a new size is added/an existing size is modified while the ImageResizer app is open? Will the Image Resizer UI refresh to show the updated sizes? It was like that on the earlier version.

With regards to syncing: I cannot load a .NetFramwork based application into a .Netcore application. Syncing settings would require tracking the JSON file which is something we are planning to do in the future. Twoway IPC makes no sense to use it since we are going to deprecate it. @crutkas any guidance on this?

The settings still

I wasn't able to change the following settings (it did not make any change on the JSON file, so if you switch to another powertoys settings and come back it would be reset)

  • Sizes (editing names or values or even adding sizes did not make any change) [Even after clicking Save Sizes)
  • Fallback encoder

Also for sizes what is the expected behaviour when a new size is added/an existing size is modified while the ImageResizer app is open? Will the Image Resizer UI refresh to show the updated sizes? It was like that on the earlier version.

Can you verify this on your machine thanks: fa7741b

Image resize still picks up the settings from the son file.

@ghost ghost requested review from traies and arjunbalgovind April 25, 2020 01:26
@arjunbalgovind
Copy link
Contributor

I wasn't able to change the following settings (it did not make any change on the JSON file, so if you switch to another powertoys settings and come back it would be reset)

  • Sizes (editing names or values or even adding sizes did not make any change) [Even after clicking Save Sizes)
  • Fallback encoder

Also for sizes what is the expected behaviour when a new size is added/an existing size is modified while the ImageResizer app is open? Will the Image Resizer UI refresh to show the updated sizes? It was like that on the earlier version.

With regards to syncing: I cannot load a .NetFramwork based application into a .Netcore application. Syncing settings would require tracking the JSON file which is something we are planning to do in the future. Twoway IPC makes no sense to use it since we are going to deprecate it. @crutkas any guidance on this?

I'll also add that earlier since the Settings was from ImageResizer the syncing was definitely required, however now since they are from two completely detached UIs I feel it may not be needed since it won't be very common that a user changes the settings while ImageResizer is open.

@arjunbalgovind
Copy link
Contributor

arjunbalgovind commented Apr 26, 2020

@laviusmotileng-ms I'm seeing some weird interaction between the add size and the Fallback encoder options. If I add a new size, I can see the change in the JSON file. But after that when I change the fallback encoder the newly added size disappears from the JSON file (but still shows in UI). Even the reverse applies where I change Fallback encoder, then add a new size then the fallback encoder reverts back in the JSON file. The only way for it to persist seems to be add a new size, switch to another powertoy settings and come back and then change fallback encoder

@arjunbalgovind
Copy link
Contributor

arjunbalgovind commented Apr 26, 2020

Verified that it is working with the old settings.json file.
Just wanted to confirm now from the Settings side when the JSON is saved its formatted in multiple lines instead of one line (whereas when Resize is clicked from Image Resizer.exe it saves it in a single line (it saves on pressing Resize because it remembers the last selected index and the other settings that you can set from the Image Resizer UI)). I hope that is as expected.

@ghost
Copy link
Author

ghost commented Apr 26, 2020

@laviusmotileng-ms I'm seeing some weird interaction between the add size and the Fallback encoder options. If I add a new size, I can see the change in the JSON file. But after that when I change the fallback encoder the newly added size disappears from the JSON file (but still shows in UI). Even the reverse applies where I change Fallback encoder, then add a new size then the fallback encoder reverts back in the JSON file. The only way for it to persist seems to be add a new size, switch to another powertoy settings and come back and then change fallback encoder

Thanks for the catch. I will look into it.

@ghost
Copy link
Author

ghost commented Apr 26, 2020

@laviusmotileng-ms I'm seeing some weird interaction between the add size and the Fallback encoder options. If I add a new size, I can see the change in the JSON file. But after that when I change the fallback encoder the newly added size disappears from the JSON file (but still shows in UI). Even the reverse applies where I change Fallback encoder, then add a new size then the fallback encoder reverts back in the JSON file. The only way for it to persist seems to be add a new size, switch to another powertoy settings and come back and then change fallback encoder

Thanks for the catch. I will look into it.

I saved the settings in a prettified format for debugging purposes. it shouldn't negatively affect the loading of the json file by image resizer IMO. I will revert the change.

@ghost
Copy link
Author

ghost commented Apr 26, 2020

@laviusmotileng-ms I'm seeing some weird interaction between the add size and the Fallback encoder options. If I add a new size, I can see the change in the JSON file. But after that when I change the fallback encoder the newly added size disappears from the JSON file (but still shows in UI). Even the reverse applies where I change Fallback encoder, then add a new size then the fallback encoder reverts back in the JSON file. The only way for it to persist seems to be add a new size, switch to another powertoy settings and come back and then change fallback encoder

Thanks a lot for checking Arjun, I resolved that here: 26fd731

Copy link
Contributor

@arjunbalgovind arjunbalgovind left a comment

Choose a reason for hiding this comment

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

LGTM! Verified the latest changes with old and new settings.json files.

@arjunbalgovind
Copy link
Contributor

@laviusmotileng-ms I'm seeing some weird interaction between the add size and the Fallback encoder options. If I add a new size, I can see the change in the JSON file. But after that when I change the fallback encoder the newly added size disappears from the JSON file (but still shows in UI). Even the reverse applies where I change Fallback encoder, then add a new size then the fallback encoder reverts back in the JSON file. The only way for it to persist seems to be add a new size, switch to another powertoy settings and come back and then change fallback encoder

Thanks a lot for checking Arjun, I resolved that here: 26fd731

Out of curiosity, what is the effect of adding the static resource binding? I'm not too familiar with it.

@ghost
Copy link
Author

ghost commented Apr 26, 2020

@laviusmotileng-ms I'm seeing some weird interaction between the add size and the Fallback encoder options. If I add a new size, I can see the change in the JSON file. But after that when I change the fallback encoder the newly added size disappears from the JSON file (but still shows in UI). Even the reverse applies where I change Fallback encoder, then add a new size then the fallback encoder reverts back in the JSON file. The only way for it to persist seems to be add a new size, switch to another powertoy settings and come back and then change fallback encoder

Thanks a lot for checking Arjun, I resolved that here: 26fd731

Out of curiosity, what is the effect of adding the static resource binding? I'm not too familiar with it.

I was using two different type of binding in the XML view file. As a result, two instances of the viewmodel class was created resulting in one instance updating the Encoder and the other sizes. As a result the settings were overriding each other on save. With regards to the actual difference here is a list:

image
https://techdifferences.com/difference-between-static-and-dynamic-binding.html

@ghost ghost merged commit 8f2a33d into dev/build-features Apr 27, 2020
@ghost ghost deleted the user/lavius/image_resizer_settings_migration branch April 27, 2020 00:34
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Settings The standalone PowerToys Settings application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings V2 UI Task: ImageResizer Settings Fluent UX: Settings
2 participants