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.

Sign up for GitHub

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

Jump to bottom

Implement large notification icon support for windows #85

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Implement large notification icon support for windows #85

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
655e0ba
Implement large notification icon support for windows
ogzhnsbs Jan 25, 2021
0e9f38a
Calculate icon size for current dpi
ogzhnsbs Jan 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/pystray/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ class Icon(object):
#: notification.
HAS_NOTIFICATION = True

#: Whether this particular implementation forces displaying a
#: large notification icon
FORCE_LARGE_NOTIFICATION_ICON = False
Copy link
Owner

Choose a reason for hiding this comment

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

These class-level constants have previously served as an indication that the backend supports a particular feature whereas this new field serves as application-wide configuration.

I understand why you have implemented it this way, but I think a more suitable approach would be to specify this with a constructor argument, but there is currently no way to pass a constructor argument to one implementation only. What do you think of a backend specific options system like this?

Copy link
Author

@ogzhnsbs ogzhnsbs Jan 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

When we set NIIF_ICON_MASK flag to dwInfoFlags parameter, toast notification adds a text that is the file description of the executable. In our case it is python.exe. It's not a problem for me because I will create an executable via pyinstaller and set a version file for my application. For other library users may not want to see ugly pyhon text.


def __init__(
self, name, icon=None, title=None, menu=None):
self._name = name
Expand Down
3 changes: 3 additions & 0 deletions lib/pystray/_util/win32.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@
NIF_REALTIME = 0x00000040
NIF_SHOWTIP = 0x00000080

NIIF_NONE = 0x00000000
NIIF_ICON_MASK = 0x0000000F

NIM_ADD = 0x00000000
NIM_MODIFY = 0x00000001
NIM_DELETE = 0x00000002
Expand Down
10 changes: 7 additions & 3 deletions lib/pystray/_win32.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
class Icon(_base.Icon):
_HWND_TO_ICON = {}

#: Force large notification icon size on loading the file
FORCE_LARGE_NOTIFICATION_ICON = False

def __init__(self, *args, **kwargs):
super(Icon, self).__init__(*args, **kwargs)

Expand Down Expand Up @@ -97,7 +100,8 @@ def _notify(self, message, title=None):
win32.NIM_MODIFY,
win32.NIF_INFO,
szInfo=message,
szInfoTitle=title or self.title or '')
szInfoTitle=title or self.title or '',
dwInfoFlags=win32.NIIF_ICON_MASK if Icon.FORCE_LARGE_NOTIFICATION_ICON else win32.NIIF_NONE)

def _remove_notification(self):
self._message(
Expand Down Expand Up @@ -347,8 +351,8 @@ def _assert_icon_handle(self):
None,
icon_path,
win32.IMAGE_ICON,
0,
0,
128 if Icon.FORCE_LARGE_NOTIFICATION_ICON else 0,
128 if Icon.FORCE_LARGE_NOTIFICATION_ICON else 0,
Copy link
Owner

Choose a reason for hiding this comment

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

Is there any reason you would not always want to do this? Are there cases where this causes regressions?

Copy link
Author

Choose a reason for hiding this comment

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

My screen dpi value is 240 (scaled 250%). I tested it for different dpi. But the result was similar for all. I setted it 128, because my screen resolution 4k and 64x64 is stil a bit blurry. I calculated icon size for the current dpi (96 -> 32x32, 240 -> 80x80) but the blur did not changed. I also tested my code with 1080p resolution (scaled 100%), no difference for 128x128 or default icon size.

Comparison:

image

win32.LR_DEFAULTSIZE | win32.LR_LOADFROMFILE)

def _register_class(self):
Expand Down