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

feat: show peer count in menu and tray tooltip #1759

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sutartmelson
Copy link
Contributor

@sutartmelson sutartmelson commented Feb 14, 2021

Addressing feature request from #1312. The peer count is retrieved whenever the menu is opened (all systems) and whenever the user hovers over the tray (Windows/macOS).

Fetching the information this way means the displayed peer count isn't always up to date. Depending on the OS, the number shown usually reflects the number of peers at the time of the previous menu open event.

If electron updates its API to allow dynamically editing menu labels, this feature could be improved.

In tray:
PeerCountInTray

In menu:
peerCountInMenu

Tray and menu when IPFS isn't running:
NoPeersIPFSStopped

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

I really want to avoid constant polling, even if it is once a minute.

Would it be possible to change this PR to remove polling?

I was looking at tray APIs while working on #1762 and perhaps we could refresh cached peer count via this event:

The above API is not available on Linux, but it should not be an issue in practice: the value is not real-time anyway, so we could simply display the last cached value + refresh cache asynchronously every time the menu is rendered (in addition to mouse-move event), so it still works on Linux.

@sutartmelson lmk your thoughts on this.

@sutartmelson
Copy link
Contributor Author

@lidel, Great idea, if polling is to be avoided (which is understandable), displaying the last cached value and fetching a new one at menu open is a good compromise. The displayed peer count isn't perfectly up to date, but I don't think it needs to be. I also updated the PR description accordingly. Hopefully this last change is a step in the right direction. Thanks!

@lidel lidel changed the title feat: Show discovered peer count feat: show peer count in menu and tray tooltip Apr 30, 2021
@lidel lidel force-pushed the feat-poll-peers branch from 9870675 to 848c9ab Compare April 30, 2021 15:47
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

I rebased this PR and fixed pluralization of peerCount label in the menu.

Hoped to include it in the release but was unable to make it work on Linux at all: turns out when "app indicator" tray backend is used on Linux, the click events are ignored, so we have no way to trigger peerCount refresh before menu is displayed.

TODO

  • figure out what to do on Linux (find a way to trigger this, disable, or poll)
  • we seem to have two "Peers" items now – one on the top, and then "Peers" link for opening Peers screen in webui. I think we want to deduplicate this (eg. remove "Peers" and make one with count clickable and open Peers screen win webui)

@lidel lidel marked this pull request as draft July 5, 2021 13:51
@AlexxNica
Copy link

@lidel, how is this showing on Linux atm? If it's already disabled I think we can just leave it as is for Linux until we have a fix so we're not holding this issue just for that.

For the second to-do item, I'm with you that only one "Peers" should remain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants