zulip-desktop icon indicating copy to clipboard operation
zulip-desktop copied to clipboard

tray: Allow switching to a specific realm tab from the context menu

Open punchagan opened this issue 6 years ago • 17 comments


What's this PR do?

This PR adds functionality to be able to switch to a specific Realm tab from the system tray menu.

Screenshots?

screenshot-tray-menu

You have tested this PR on:

  • [ ] Windows
  • [x] Linux/Ubuntu
  • [ ] macOS

punchagan avatar Jan 21 '19 04:01 punchagan

@punchagan thanks for working on this. The feature looks good to me. @timabbott, @rishig thoughts?

image

akashnimare avatar Jan 21 '19 09:01 akashnimare

Do we have more control over what to put in that menu? It'd be nice to have a title and/or the organization icons/unread counts in there, but if we don't have much control, that looks reasonable.

timabbott avatar Jan 22 '19 20:01 timabbott

Do we have more control over what to put in that menu? It'd be nice to have a title and/or the organization icons/unread counts in there, but if we don't have much control, that looks reasonable.

Yes, I think having unread counts would be quite useful too. Especially, when I see the 99+ badge on the icon. I've been poking around a little to try to figure out a good way of doing this.

If anyone has some advice on how to go about doing this, that would be helpful! /cc @abhigyank @akashnimare

punchagan avatar Jan 23 '19 02:01 punchagan

Here is the relevant code - https://github.com/zulip/zulip-electron/blob/7314c1f1dd0f34136277939560b031452d33ad35/app/renderer/js/main.js#L487

akashnimare avatar Jan 23 '19 10:01 akashnimare

One thing I was wondering about was, populating the menu only on click - instead of updating the menu for every new message. Do you have any thoughts/ideas/experience on this?

punchagan avatar Jan 23 '19 12:01 punchagan

I've pushed a change that displays the unread counts next to the realm name. I'm not sure my usage of ipcRenderer.send is the best way of doing it, though. @akashnimare can you review it and let me know if there's a better way of doing this?

punchagan avatar Jan 24 '19 13:01 punchagan

screenshot-tray-menu-1

punchagan avatar Jan 24 '19 14:01 punchagan

definitely don't want the unread; just Zulip Community (3)

rishig avatar Jan 24 '19 14:01 rishig

definitely don't want the unread; just Zulip Community (3)

Thanks for the feedback, Rishi. I'll change that.

punchagan avatar Jan 24 '19 14:01 punchagan

instead of updating the menu for every new message. Do you have any thoughts/ideas/experience on this?

Yeah, this is what I was thinking. Re-creating the tray on every new message is expensive (performance wise). There is an api available by which we could get the click event and then update the count.

akashnimare avatar Jan 25 '19 07:01 akashnimare

There is an api available by which we could get the click event and then update the count.

I tried playing with this a little, but seems like 'click' doesn't get fired on Gnome Shell in Ubuntu? The context menu appears, but the click handler is not getting called when I click on the tray icon. Not sure, what's happening here. :disappointed:

punchagan avatar Jan 25 '19 07:01 punchagan

On the other hand, I have been using this branch for the past day or so, with 5 realms (some not super active) and I haven't really seen any thing wonky.

punchagan avatar Jan 25 '19 10:01 punchagan

I tried playing with this a little, but seems like 'click' doesn't get fired on Gnome Shell in Ubuntu? The context menu appears, but the click handler is not getting called when I click on the tray icon. Not sure, what's happening here.

Oh, I remember the right click has some known issues on some Linux distros.

akashnimare avatar Jan 25 '19 10:01 akashnimare

Looks good to me I'm just worried about this. @abhigyank can you look into the performance part? In the worst case, I think we could go with the following approach (at least for Windows/macOS)-

There is an api available by which we could get the click event and then update the count.

akashnimare avatar May 09 '19 21:05 akashnimare

I think the click thing could be very shell or OS dependent, i.e. could be broken for some people (is the UX worth the broken risk? ). While the unread count does look good, we are already have people reporting issues with CPU usage so I am not sure if we should go an approach with re rendering everytime ( if we can get the click working then its great!) Though I'll give this a try and check if there are any perf issues with the current approach.

abhigyank avatar May 10 '19 09:05 abhigyank

I think the click thing could be very shell or OS dependent, i.e. could be broken for some people (is the UX worth the broken risk? ).

I think we could ship it on macOS and Windows since the click event works as expected on these platforms. For Linux, we could either drop this feature or ship with the current implementation.

akashnimare avatar May 11 '19 09:05 akashnimare

Heads up @punchagan, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Aug 17 '19 14:08 zulipbot