google / UIforETW

User interface for recording and managing ETW traces
https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/
Apache License 2.0
1.57k stars 201 forks source link

Minimize to system tray #108

Closed aerworker closed 4 years ago

aerworker commented 7 years ago

The main window shows an icon in system tray when minimized and hides itself in task bar. Click on icon restores the main window. Does it need to introduce an option for this functionality?

googlebot commented 7 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


randomascii commented 7 years ago

Can you explain why this is a good idea, preferably as part of the commit message? I'm a believer in more detailed commit messages - see this post for some good guidelines:

https://chris.beams.io/posts/git-commit/

But, I can't look at the patch until the CLA is signed. Thanks!

aerworker commented 7 years ago

I signed it!

From: googlebot Sent: Monday, August 07, 2017 4:51 PM To: google/UIforETW Cc: aerworker ; Author Subject: Re: [google/UIforETW] Minimize to system tray (#108)

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

googlebot commented 7 years ago

CLAs look good, thanks!

aerworker commented 7 years ago

I've updated the commit.

randomascii commented 7 years ago

So... I'm not seeing an updated commit message with an explanation of why, and I definitely have concerns. This seems like unusual behavior, and confusing.

Currently I can tell whether UIforETW is running by looking in one place for its icon (especially since I have it pinned). With this change it might be there, or it might be in the notification area, and the notification might be hidden.

Now, if this was hidden behind a switch then this would not be as severe a problem. But that's more code, more complexity in the settings dialog, more ways for users to get themselves into a confused state.

This doesn't necessarily mean "no", but it does mean "convince me of why this is good". Precedent for this UI pattern would be helpful.