Enable tray icon on Linux without breaking Windows #23

ConorIA · 2016-08-27T18:57:38Z

Here is a suggestion to resolve issue #22, and offer a more elegant solution to issue #1. The png icons are of my own making, having traced the ico files in inkscape. They can and maybe should be replaced by official icons, but they do the trick for now!

lipis · 2016-08-30T05:42:44Z

electron/js/menu/tray.js

@@ -24,14 +24,22 @@ const {app, BrowserWindow, Menu, Tray} = require('electron');
 const path = require('path');
 const config = require('./../config');

-const iconPath = path.join(app.getAppPath(), 'img', 'tray.ico');
-const iconBadgePath = path.join(app.getAppPath(), 'img', 'tray.badge.ico');
+var iconExt = null;


var iconExt = 'png';

lipis · 2016-08-30T05:44:29Z

You should also update this part: https://github.com/wireapp/wire-desktop/blob/master/electron/js/util.js#L59-L69

to something like:

if (process.platform === 'darwin') {
  app.dock.setBadge(count ? count[1] : '');
} else if (count) {
  tray.useBadgeIcon();
} else {
  tray.useDefaultIcon();
}

lipis · 2016-08-30T05:48:38Z

Having done all that.. it doesn't work on my Ubuntu 16.04 :/ Can you send us a screenshot wiht yours?

ConorIA · 2016-08-30T13:42:12Z

I'm out of town / away from my laptop today. I'll send a screenshot over as soon as I can.

lipis · 2016-08-30T18:06:45Z

🌴 🍺 🍸

ConorIA · 2016-08-30T20:59:12Z

There's the icon with the context menu open. I use the 'top icons' extension on Gnome, so on other systems it will be in a tray on the bottom left corner. This is using the build based on my PR. I will try with the changes you have suggested.

ConorIA · 2016-08-30T21:31:42Z

Yay! I got a message! 😉

aaaaand I read it!

lipis · 2016-08-30T22:09:46Z

Your next task is to make the travis tests green.. and we'll ask our designers to provide the correct PNGs :)

ConorIA · 2016-08-31T00:12:15Z

Getting a type error now. A little over my head. Any suggestions? Also, if you want to ask your designers for an SVG app icon to go with the PNG tray icons, that'd be great! 😃

ConorIA · 2016-09-01T21:53:29Z

Played around a little, but still no luck. I tried replacing line 33 (let appIcon = null;) with var appIcon = new Tray(iconPath);, but I get an error:

TypeError: Error processing argument at index 0, conversion failure from /home/conor/git/wire-desktop/node_modules/electron-prebuilt/dist/resources/default_app.asar/img/tray.png

Might it be related to: electron/electron#1006

bennycode · 2016-09-01T22:14:35Z

TypeError: Cannot read property 'setImage' of null
at Object.useBadgeIcon (/home/travis/build/wireapp/wire-desktop/electron/js/menu/tray.js:63:12)

It looks like appIcon isn't defined?

ConorIA · 2016-09-01T23:45:17Z

As far as I can tell, the app icon is originally set to null but should be replaced as long as we're not on Darwin. Not sure why it isn't getting replaced later on. I'm not familiar with JS, so I might be reading it wrong.

ConorIA · 2016-09-02T23:58:52Z

Could it be that the test is simply timing out before the badge updates? On my machine, when I run npm test --verbose, the window barely opens before it is closed and warns:

  util
    #updateBadge()
TypeError: Cannot read property 'setImage' of null
    at Object.useBadgeIcon (/home/conor/wire-desktop/electron/js/menu/tray.js:63:12)
    at Timeout._onTimeout (/home/conor/wire-desktop/electron/js/util.js:62:14)
    at tryOnTimeout (timers.js:224:11)
    at Timer.listOnTimeout (timers.js:198:5)
TypeError: Cannot read property 'setImage' of null
    at Object.useBadgeIcon (/home/conor/wire-desktop/electron/js/menu/tray.js:63:12)
    at Timeout._onTimeout (/home/conor/wire-desktop/electron/js/util.js:62:14)
    at tryOnTimeout (timers.js:224:11)
    at Timer.listOnTimeout (timers.js:198:5)
      1) should update badge according to window title
      ✓ should update badge according to window title (1498ms)

I have tried to adjust the timeout with this.timeout, but it seems that the use of arrow functions breaks that option (https://mochajs.org/; mochajs/mocha#2018).

lipis · 2016-09-06T15:37:28Z

Can you add please these images for now and we'll deal with the exact resolutions later on..

ConorIA · 2016-09-06T15:54:20Z

Done! 😄

lipis · 2016-09-06T15:56:04Z

As for the tests it's failing because the tray.createTrayIcon() is never called and it's a bigger thing ATM.. For now just to fix the test and have it green.. update the code in tray.js to something like that:

  useDefaultIcon: function() {
    if (appIcon == null) return;
    appIcon.setImage(iconPath);
  },

  useBadgeIcon: function() {
    if (appIcon == null) return;
    appIcon.setImage(iconBadgePath);
  }

lipis · 2016-09-06T15:57:22Z

Also.. did you sign our CLA?

lipis · 2016-09-06T17:28:50Z

Let's see if Travis is going to wake up today! :)

Can you post screenshots of the two icons from your machine?

ConorIA · 2016-09-06T18:30:44Z

Strange, Travis just passed on the commit before the Travis fixes you suggested!

I'll get the screenshots and CLA to you as soon as I'm back in my office after a meeting.

~~Who should I send the CLA to?~~ EDIT: Nevermind, found the address. We're good to go.

ConorIA · 2016-09-06T20:49:58Z

Here is the icon with no messages. ~~I'm not very popular today, but I will send an image with the badge if someone writes to me.~~ 😉

And here it is with the badge.

david-wire · 2016-09-07T07:14:38Z

CLA approved

lipis · 2016-09-07T08:55:39Z

Boom..

ConorIA added 2 commits August 27, 2016 14:49

Enable tray icon in Linux without breaking Windows

e7bb25e

Tray icon was too big

0bb9a5a

ConorIA mentioned this pull request Aug 27, 2016

No (native) tray icon #22

Closed

lipis reviewed Aug 30, 2016
View reviewed changes

Implement fixes in the PR comments

1a29577

Some guy named Travis asked for a semicolon.

bdc2f93

bennycode assigned lipis Sep 2, 2016

ConorIA changed the title ~~Enable tray icon on Linux withouth breaking Windows~~ Enable tray icon on Linux without breaking Windows Sep 2, 2016

Push official icons from lipis

3ff98d3

lipis' suggested travis fixes

cbfe835

lipis merged commit 0fadf07 into wireapp:master Sep 7, 2016

ConorIA deleted the tray branch September 7, 2016 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable tray icon on Linux without breaking Windows #23

Enable tray icon on Linux without breaking Windows #23

ConorIA commented Aug 27, 2016

lipis Aug 30, 2016 •

edited

Loading

lipis commented Aug 30, 2016

lipis commented Aug 30, 2016

ConorIA commented Aug 30, 2016

lipis commented Aug 30, 2016

ConorIA commented Aug 30, 2016

ConorIA commented Aug 30, 2016 •

edited

Loading

lipis commented Aug 30, 2016

ConorIA commented Aug 31, 2016 •

edited

Loading

ConorIA commented Sep 1, 2016 •

edited

Loading

bennycode commented Sep 1, 2016

ConorIA commented Sep 1, 2016

ConorIA commented Sep 2, 2016

lipis commented Sep 6, 2016 •

edited by bennycode

Loading

ConorIA commented Sep 6, 2016

lipis commented Sep 6, 2016

lipis commented Sep 6, 2016 •

edited

Loading

lipis commented Sep 6, 2016 •

edited

Loading

ConorIA commented Sep 6, 2016 •

edited

Loading

ConorIA commented Sep 6, 2016 •

edited

Loading

david-wire commented Sep 7, 2016

lipis commented Sep 7, 2016

Enable tray icon on Linux without breaking Windows #23

Enable tray icon on Linux without breaking Windows #23

Conversation

ConorIA commented Aug 27, 2016

lipis Aug 30, 2016 • edited Loading

Choose a reason for hiding this comment

lipis commented Aug 30, 2016

lipis commented Aug 30, 2016

ConorIA commented Aug 30, 2016

lipis commented Aug 30, 2016

ConorIA commented Aug 30, 2016

ConorIA commented Aug 30, 2016 • edited Loading

lipis commented Aug 30, 2016

ConorIA commented Aug 31, 2016 • edited Loading

ConorIA commented Sep 1, 2016 • edited Loading

bennycode commented Sep 1, 2016

ConorIA commented Sep 1, 2016

ConorIA commented Sep 2, 2016

lipis commented Sep 6, 2016 • edited by bennycode Loading

ConorIA commented Sep 6, 2016

lipis commented Sep 6, 2016

lipis commented Sep 6, 2016 • edited Loading

lipis commented Sep 6, 2016 • edited Loading

ConorIA commented Sep 6, 2016 • edited Loading

ConorIA commented Sep 6, 2016 • edited Loading

david-wire commented Sep 7, 2016

lipis commented Sep 7, 2016

lipis Aug 30, 2016 •

edited

Loading

ConorIA commented Aug 30, 2016 •

edited

Loading

ConorIA commented Aug 31, 2016 •

edited

Loading

ConorIA commented Sep 1, 2016 •

edited

Loading

lipis commented Sep 6, 2016 •

edited by bennycode

Loading

lipis commented Sep 6, 2016 •

edited

Loading

lipis commented Sep 6, 2016 •

edited

Loading

ConorIA commented Sep 6, 2016 •

edited

Loading

ConorIA commented Sep 6, 2016 •

edited

Loading