Add tooltip indicator - Githubissues

pongloongyeat commented 3 years ago

PR to close #197.

Screenshot from 2020-12-01 03-34-54

Todo:

[x] Probe WiFi name
[ ] Probe VPN name
[x] Fix Connecting to "" issue

pongloongyeat commented 3 years ago

Works as expected for now Screenshot from 2020-12-02 02-14-41

Note to self for tomorrow morning: Network name not present when connecting to a network (Connecting to ""). Screenshot from 2020-12-02 02-16-06

pongloongyeat commented 3 years ago

hey @cassidyjames, just wondering how we would display the tooltip for a wired connection? Currently, it shows Connected to "Wired" and if you have multiple wired connections i.e. ethernet and USB tethering then it would show which it is connected to.

usb0

Also, I'm assuming that if the user is connected to both wired AND wifi, the tooltip for wired connection takes precedence.

cassidyjames commented 3 years ago

For no connection name, we could just say Connected to wireless network without the quotes, I think. Similarly with wired: Connected to wired network. I like the name showing for USB tethering, that's actually pretty useful!

For multiple connections, I think we have two options: name whatever the icon itself is indicating, or name both, i.e. Connected to wired network and “Wi-Fi Network Name”

Or I guess a third option: actually just show an additional icon for additional networks… which I kind of like better, but is outside the scope of this work; I have filed that as a separate issue (https://github.com/elementary/wingpanel-indicator-network/issues/200). Showing whatever the icon is indicating is probably the easiest/simplest approach.

pongloongyeat commented 3 years ago

@cassidyjames How about VPN connections? Is there a specific way we can show this to the user? Perhaps something like Connected to wired network with "<VPN_NAME>"? I feel like this might overcrowd the tooltip with quotation marks when we're connecting to a specific wifi i.e. Connected to "thamlinchan2.4Ghz@unifi" with "NordVPN".

I can't tell what the icon behaviour is like as I don't have a VPN. Is there a way I could spoof a VPN connection on elementary?

Edit: Please ignore the fact that commits are not in sync with the messages in this PR. My network time was messed up.

pongloongyeat commented 3 years ago

Marking this as ready to review since this mostly works except for VPN.

Since I'm practically new to Vala, feel free to critique anything you see fit. Some things to note:

I'm accessing wifi_box and other_box in the popover widget to know which is the currently active connection. This is done by making them public and I'm not sure if that has any consequences.
Currently, if both wired and wifi connections are active, the tooltip shows only the wired connection (this is in line with how the icon is shown).
If there is only one active wired connection, it shows Connected to wired connection. If there are multiple sources, it shows Connected to "<YOUR_ETHERNET_DEVICE>" i.e,

I don't think it should show the ethernet device name. It should be showing the name of the network it is connected to but this is what the popover widget shows and I've left it as is. It would look inconsistent if we show the network name in the tooltip and the ethernet device name in the popover widget. I'll file an issue later on this.

andirsun commented 3 years ago

Tooltip indicator show connected to (null) Screenshot from 2021-01-02 10-50-44

pongloongyeat commented 3 years ago

Mine is working fine. Could you mention the steps to reproduce this? Also, I think it may be better to file this as a separate issue since this PR has already been merged :/

elementary / wingpanel-indicator-network

Add tooltip indicator #199