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

Add new tray icons for macOS #577

Merged

Conversation

rak-phillip
Copy link
Contributor

@rak-phillip rak-phillip commented Sep 2, 2021

This adds new template icons for use in the tray for macOS.

I received an icon set that represents different states (one for starting, started, stopping, stopped, and error), and I noticed that we don't necessarily have corresponding icons in windows. We don't really make use of stopped and stopping anywhere in the project, but I was thinking that it might be a good idea to get the remaining icons now for ease of use.

Alternatively, we can remove stopped and stopping from the macOS collection.

#236

@rak-phillip rak-phillip requested a review from mook-as September 2, 2021 22:59
@rak-phillip rak-phillip self-assigned this Sep 2, 2021
@rak-phillip
Copy link
Contributor Author

Here's some examples of the new icons. I tried to include a blue-ish background to make a fair comparison

Screen Shot 2021-09-02 at 3 39 49 PM

Screen Shot 2021-09-02 at 3 44 49 PM

Screen Shot 2021-09-02 at 3 45 27 PM

@jandubois
Copy link
Member

We don't really make use of stopped and stopping anywhere in the project

I think the problem is that "stopping" is taking a very short amount of time, and then the app exits, so there is no way to show the "stopped" state.

I was going to create an issue about our mental app model (do we have one or two apps (gui and background/helper/...), and what is the relationship between them from the user's point of view (like what does it mean to stop one vs. the other). But I haven't been able to clarify my own thinking about it enough to actually write it up.

mook-as
mook-as previously approved these changes Sep 2, 2021
Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

Please add the DCO trailer. The run logs has instructions.

src/main/tray.ts Outdated
return os.platform() === 'darwin';
}

private trayIconsMacOs = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this should be readonly as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree - updated to include the readonly modifier

Copy link
Contributor

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

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

I like the stopping and stopped ones. I don't know if we'll have a case where we can show stopped. We can investigate that in the future.

These look better than what we have today.

@mattfarina mattfarina merged commit 76830ec into rancher-sandbox:main Sep 3, 2021
@rak-phillip rak-phillip deleted the bugfix/236-macos-tray-icons branch September 3, 2021 16:10
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