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

Use a custom tooltip window for the tray icon #670

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

selvanair
Copy link
Collaborator

@selvanair selvanair commented Feb 10, 2024

Built-in tray notification icon has a tip text length limit of 128 characters which is often limited for showing the connected profile name, connected since time and IP addresses. If the profile name is long the IP numbers could get truncated.

Fix by using a custom tooltip window and display it when mouse hovers over the icon. As the status bar need not be at the bottom of the screen (could be at right, left or top as well), the location of the window is chosen based on the mouse co-ordinates that trigger the hover event.

In case of errors while setting up the tooltip window, fall back to the current behaviour.

If the message is too long to include time and IP, truncate the profile name part of the message.

Fixes issue #666

Edit: force pushed after fixing some typos in commit message and buggy text truncation logic.

old

Screenshot from 2024-02-10 17-11-34

new

Screenshot from 2024-02-10 17-14-56

@cron2
Copy link
Contributor

cron2 commented Feb 11, 2024

I like the idea, but do not feel qualified today to ACK or NAK the code. @lstipakov something for you?

@lstipakov
Copy link
Member

Hi,

On Windows 11 there are two issues;

  • When there is no connection, there is no tooltip at all. Currently we display OpenVPN GUI there.
  • When there is active connection, tooltip is displayed on top left. Currently we display it next to the tray.
    Näyttökuva 2024-02-12 110020

Näyttökuva 2024-02-12 110139

@selvanair
Copy link
Collaborator Author

linking to my comment #666 (comment)

Built-in tray notification icon has a tip text length limit of 128
characters which is often limited for showing the connected profile name,
connected since time and IP addresses. If the profile name is long the IP
numbers could get truncated.

Fix by using a custom tooltip window and display it when mouse hovers over
the icon. As the status bar need not be at the bottom of the screen (could be
at right, left or top as well), the location of the window is chosen based
on the mouse co-ordinates that trigger the hover event.

In case of errors while setting up the tooltip window, fall back to the current
behaviour.

If the message is too long to include time and IP, truncate the profile name
part of the message.

v2: Do not use wParam in NIN_POPUOPEN message as it does not seem to work
    on Windows 11. Instead use GetCursorPos() for mouse location.

Fixes issue OpenVPN#666

Signed-off-by: Selva Nair <[email protected]>
@selvanair
Copy link
Collaborator Author

Force pushed with changes:

  • For some reason wParam does not contain the position in NIN_POPUOPEN message. Fixed by using GetCrursorPos() to determine the mouse location.
  • Also fixed the empty message bug.

@selvanair
Copy link
Collaborator Author

selvanair commented Feb 13, 2024

(Moving this to the right place)
Pushed two commits:
(i) Remove title and prepend it to the message. And make tip_msg local.
(ii) Position the tooltip window at a fixed distance above the icon instead of tracking the mouse position
(figured how to do this :)

On Win11 the tip window should probably have more rounded corners, but will leave it at that.

selvanair and others added 2 commits February 13, 2024 19:12
Also change scope of tip_msgi[] to local

Co-authored-by: Lev Stipakov <[email protected]>
Signed-off-by: Selva Nair <[email protected]>
Use Shell_NotifyGetRect to find the icon location and place the
tip window a fixed distance above/below it.

It appears GUID_NULL used for above is not pulled in by shellapi.h.
Define locally when absent.

Also add TTF_RTLREADING for RTL languages. How to right justify
as well in this case is unclear.

Signed-off-by: Selva Nair <[email protected]>
Copy link
Member

@lstipakov lstipakov left a comment

Choose a reason for hiding this comment

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

Tested on Windows 11. Both tooltip and code look good.

@selvanair selvanair merged commit d1756f0 into OpenVPN:master Feb 14, 2024
10 checks passed
@selvanair selvanair deleted the notify-icon branch April 21, 2024 17:57
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.

3 participants