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

Issue #1895: add check for updates and common features on Tray Icon #1902

Closed
wants to merge 4 commits into from

Conversation

sqrlmn
Copy link
Contributor

@sqrlmn sqrlmn commented Apr 2, 2020

…tem Tray Icon

Windows System Tray Icon - check for updates and show common features

Summary of the Pull Request

References

PR Checklist

  • [] Applies to #xxx
  • [] CLA signed. If not, go over here and sign the CLA
  • [] Tests added/passed
  • [] Requires documentation to be updated
  • [] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

…ndows System Tray Icon

Windows System Tray Icon - check for updates and show common features
@timpetroclio
Copy link

@sqrlmn
This branch is working great for System Tray Icon #1895

@enricogior
Copy link
Contributor

Hi @sqrlmn
thank your for your contribution.
Regarding the "check for update" menu item, since PowerToys has already automatic checking every 24 hours and since in 0.17 we will also auto download the MSI, having the command in the tray icon menu is not a planned feature.

Regarding the "window walker" menu item, since Window Walker will become part of the Launcher, we may want to hold off until the Launcher is released, but let's wait for @crutkas feedback on this one as well as for Image Resizer.

@crutkas
Copy link
Member

crutkas commented Apr 8, 2020

WW will be merged into Launcher. Need to work with beta to figure out time schedules.

Does “check for updates” help make you feel better supported? I am not opposed As I know I do this too despite knowing most apps do exactly what we do.

@enricogior
Copy link
Contributor

enricogior commented Apr 8, 2020

@crutkas
if we want to add a check for update on the tray icon menu, we should use the function that we introduced to query the GitHub API and compare the current version, and show a dialog with the result and not open the Github release web page in the browser.

@crutkas
Copy link
Member

crutkas commented Apr 9, 2020

agreed. we should also do the same for the "check for updates" in settings.

Windows System Tray Icon - check for updates and show common features
Image Resizer should be able to open one window at the same time
Image Resizer should be able to open one window at the same time
@crutkas
Copy link
Member

crutkas commented Apr 14, 2020

@sqrlmn, looks like right now maybe a missing var?

Check failure on line 96 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L96
src\tests\win-app-driver\PowerToysTrayTests.cs(96,13): Error CS0103: The name 'ShortWait' does not exist in the current context
 Check failure on line 100 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L100
src\tests\win-app-driver\PowerToysTrayTests.cs(100,13): Error CS0103: The name 'ShortWait' does not exist in the current context
 Check failure on line 106 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L106
src\tests\win-app-driver\PowerToysTrayTests.cs(106,13): Error CS0103: The name 'ShortWait' does not exist in the current context
 Check failure on line 125 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L125
src\tests\win-app-driver\PowerToysTrayTests.cs(125,13): Error CS0103: The name 'ShortWait' does not exist in the current context

@sqrlmn
Copy link
Contributor Author

sqrlmn commented Apr 16, 2020

@sqrlmn, looks like right now maybe a missing var?

Check failure on line 96 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L96
src\tests\win-app-driver\PowerToysTrayTests.cs(96,13): Error CS0103: The name 'ShortWait' does not exist in the current context
 Check failure on line 100 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L100
src\tests\win-app-driver\PowerToysTrayTests.cs(100,13): Error CS0103: The name 'ShortWait' does not exist in the current context
 Check failure on line 106 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L106
src\tests\win-app-driver\PowerToysTrayTests.cs(106,13): Error CS0103: The name 'ShortWait' does not exist in the current context
 Check failure on line 125 in src\tests\win-app-driver\PowerToysTrayTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.PowerToys

src\tests\win-app-driver\PowerToysTrayTests.cs#L125
src\tests\win-app-driver\PowerToysTrayTests.cs(125,13): Error CS0103: The name 'ShortWait' does not exist in the current context

@crutkas
Yes, I am trying to fix it

@crutkas
Copy link
Member

crutkas commented Apr 17, 2020

@sqrlmn maybe want me to look at the branch?

@sqrlmn
Copy link
Contributor Author

sqrlmn commented Apr 17, 2020

@sqrlmn maybe want me to look at the branch?

Yes, please. Thank you!

@crutkas
Copy link
Member

crutkas commented Apr 17, 2020

i don't think so i don't think i agree with how this is implemented here after running it. I think we should have discussed how stuff act before doing the work.

I think we should hold on this until

The experience for end user is odd here.

  • WW doesn't launch
  • about pulls up old about which was removed intentionally.
  • Image resizer experience is odd and feels disjointed. I feel like if the [Fluent UX] Image Resizer #1053 update happens,

My takeaways:

  1. need to adjust our contributor update to crisply state work is being started and agreement
  2. assignment for who is doing work

image

@crutkas crutkas closed this Apr 17, 2020
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.

4 participants