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

Fix Linux tray menu #612

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Fix Linux tray menu #612

merged 1 commit into from
Feb 19, 2021

Conversation

nogarcia
Copy link
Contributor

Related issue

Closes #568

Context / Background

As per the title: the tray context menu on Linux (specifically Ubuntu GNOME3) doesn't work as it does on Windows with the three actions, "Punch Time," "Show App," and "Quit," and instead just has the default tray context menu.

What change is being introduced by this PR?

On Linux, app trays in Electron have to have their context menus set explicitly rather than dynamically through the click event, as per the Tray documentation.

  • When app indicator is used on Linux, the click event is ignored.

If you want to keep exact same behaviors on all platforms, you should not rely on the click event and always attach a context menu to the tray icon.

To do this, I called tray.setContextMenu(contextMenu) directly after contextMenu is built on app startup. This may actually be a bad practice — I'm quite unfamiliar with this codebase — and I'd be glad to receive any feedback, positive or negative.

Original behavior:

image

Patched behavior:

image

How will this be tested?

All Jest tests passed without issue, and building succeeded on both Windows and Linux; both distributions work as expected.

(Sorry about the lack of a feature branch; it slipped my mind when I made the commit.)

@codecov
Copy link

codecov bot commented Feb 18, 2021

Codecov Report

Merging #612 (e1e0b70) into main (01f0dfd) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
+ Coverage   67.80%   67.81%   +0.01%     
==========================================
  Files          28       28              
  Lines        2171     2172       +1     
  Branches      309      309              
==========================================
+ Hits         1472     1473       +1     
  Misses        634      634              
  Partials       65       65              
Impacted Files Coverage Δ
js/main-window.js 59.78% <100.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01f0dfd...e1e0b70. Read the comment docs.

Copy link
Collaborator

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Thanks!
This looks fine. I'll just double check the behavior on a mac, but I think it's good to go.

@thamara
Copy link
Collaborator

thamara commented Feb 19, 2021

\changelog-update
Message: Fix: [#612] Fix Linux tray menu to have options: Punch time, Show App and Quit
User: radiohazard-dev

@thamara thamara merged commit f48c085 into TTLApp:main Feb 19, 2021
@NetizenAbel
Copy link
Collaborator

I appreciate the code coverage, and it isn't at all bad practice. Electron will always have issues with Linux desktop standards, because there are too many, but this is good.

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.

Tray menu is not working on Linux
3 participants