Improve macOS and Windows platform integration #5851

phoerious · 2020-12-18T23:42:22Z

Changes in this PR (macOS and Windows only at the moment):

Allow switching between themes without restart (except classic)
Rework icon loading and recolouring logic to react to theme changes
Automatically react to light/dark theme change
Remove explicit selection of monochrome tray icon variant (selected
automatically now)
Update theme background colours for Big Sur
Update application icon to match Big Sur HIG

The tray icon doesn't respond perfectly to theme changes yet on Big Sur, since we need different icons for dark and light theme and cannot simply let the OS recolour the icon for us (we do that, too, but only as an additional fallback). At the moment, there is no signal to listen to that would allow this.

This patch adds a few generic methods to OSUtils for detecting and communicating theme changes, which are only stubs for Linux at the moment and need to be implemented in future commits.

Fixes #4933
Fixes #5349

Screenshots

New icon:

Type of change

✅ Bug fix (non-breaking change that fixes an issue)
✅ New feature (change that adds functionality)

droidmonkey · 2020-12-19T00:31:51Z

Can we add the big sur style app icon as well

varjolintu · 2020-12-19T09:04:17Z

Does not work on Catalina. Only the title bar changes color, the applications stays the same. I also there's no option to switch to classic theme, or switch themes manually.

And the tray icon is just pure white if set to "Colorful":

With Mojave there's a compliation error with:

src/gui/osutils/macutils/AppKitImpl.mm:78:50: error: unused parameter 'notification'
- (void) interfaceThemeChanged:(NSNotification*) notification

After setting #pragma unused (notification) inside the function the code compiles, and theme switching works, but the same colorful tray icon bug remains.

Built with Qt 5.15.2.

phoerious · 2020-12-19T09:35:24Z

Ah, the colorful icon is probably changed by the OS. I need to unset the mask flag if it's not the monochrome version.

phoerious · 2020-12-19T10:10:18Z

@varjolintu I fixed the compiler warning, the icon bug, and another bug I discovered while switching themes.
I do not know why it's not working on Catalina (but Mojave it is?!) and I cannot test it. Would you be willing to play around with delays in MacUtils.cpp and see if that does anything?

varjolintu · 2020-12-19T10:13:09Z

@phoerious Sure. I'll give it a try.

phoerious · 2020-12-23T23:15:16Z

Added Windows support in a separate commit.

droidmonkey · 2020-12-25T20:33:17Z

Are you going to attempt Linux integration before we merge this?

phoerious · 2020-12-25T21:43:36Z

No. I will do that in a separate PR later.

stonerl · 2020-12-30T20:08:34Z

It is working for me in BS. The only problem I have is the missing icon.

phoerious · 2020-12-31T09:17:13Z

You have to package it. If you run the application from the build directory, it won't pick up the icon. That's always been the case, but the default app icon was different pre-big sur.

stonerl · 2020-12-31T17:17:25Z

I see. I was following the build instructions in INSTALL.md instead of the wiki. Thanks. It is working now.

src/gui/Application.cpp

- Allow switching between themes without restart (except classic) - Rework icon loading and recolouring logic to react to theme changes - Automatically react to light/dark theme change - Remove explicit selection of monochrome tray icon variant (selected automatically now) - Update theme background colours for Big Sur - Update application icon to match Big Sur HIG The tray icon doesn't respond perfectly to theme changes yet on Big Sur, since we need different icons for dark and light theme and cannot simply let the OS recolour the icon for us (we do that, too, but only as an additional fallback). At the moment, there is no signal to listen to that would allow this. This patch adds a few generic methods to OSUtils for detecting and communicating theme changes, which are only stubs for Windows and Linux at the moment and need to be implemented in future commits. Fixes #4933 Fixes #5349

Added - Support Argon2id KDF [#5778] - Support XMLv2 key files [#5798] Changed - Improve CSV Import/Export, include time fields and TOTP [#5346] - Support empty area dragging of the application window [#5860] - Display default Auto-Type sequence in preview pane [#5654] - Remove strict length limit on generated passwords [#5748] - Hide key file path by default when unlocking database [#5779] - Document browser extension use with Edge in managed mode [#5692] - Windows: Prevent clipboard history and cloud sync [#5853] - macOS: Update the application icon to Big Sur styling [#5851] Fixed - Re-select previously selected entry on database unlock [#5559] - Properly save special character choice in password generator [#5610] - Fix crash in browser integration with multiple similar entries [#5653] - Remove offset on username field in classic theme [#5788] - Ensure entry history is copied when drag/dropping entries and groups [#5817] - Close modal dialogs when database is locked [#5820] - Prevent crash when KeeShare modifies an entry that is currently being edited [#5827] - Improve preview of entry attributes [#5834] - Always activate/focus database open dialog preventing mistype [#5878] - Reports: fix calculation of average password length [#5862] - Linux: Delay startup on login to correct tray icon issues [#5724]

Release 2.6.3 Added - Support Argon2id KDF [keepassxreboot#5778] - Support XMLv2 key files [keepassxreboot#5798] Changed - Improve CSV Import/Export, include time fields and TOTP [keepassxreboot#5346] - Support empty area dragging of the application window [keepassxreboot#5860] - Display default Auto-Type sequence in preview pane [keepassxreboot#5654] - Remove strict length limit on generated passwords [keepassxreboot#5748] - Hide key file path by default when unlocking database [keepassxreboot#5779] - Document browser extension use with Edge in managed mode [keepassxreboot#5692] - Windows: Prevent clipboard history and cloud sync [keepassxreboot#5853] - macOS: Update the application icon to Big Sur styling [keepassxreboot#5851] Fixed - Re-select previously selected entry on database unlock [keepassxreboot#5559] - Properly save special character choice in password generator [keepassxreboot#5610] - Fix crash in browser integration with multiple similar entries [keepassxreboot#5653] - Remove offset on username field in classic theme [keepassxreboot#5788] - Ensure entry history is copied when drag/dropping entries and groups [keepassxreboot#5817] - Close modal dialogs when database is locked [keepassxreboot#5820] - Prevent crash when KeeShare modifies an entry that is currently being edited [keepassxreboot#5827] - Improve preview of entry attributes [keepassxreboot#5834] - Always activate/focus database open dialog preventing mistype [keepassxreboot#5878] - Reports: fix calculation of average password length [keepassxreboot#5862] - Linux: Delay startup on login to correct tray icon issues [keepassxreboot#5724]

smlu · 2021-01-21T23:24:19Z

src/gui/Icons.cpp

-    if (!icon.isNull() && !overrideColor.isValid()) {
-        return icon;
+    painter->save();
+    painter->drawPixmap(rect, m_baseIcon.pixmap(pixmapSize, mode, state));


I get very ugly icons on HDPI screen on Windows if icon is painted via drawPixmap method:

Icons are drawn correctly If I change the icon pixelmap to be painted via QIcon::paint method e.g. m_baseIcon.paint(painter, rect, Qt::AlignCenter, mode, state)

phoerious · 2021-01-22T07:53:29Z

Interesting. It's fine on my system.

phoerious · 2021-01-22T08:14:25Z

Actually, looking very closely I can see the same thing, just way less severe than what you have.

smlu · 2021-01-22T14:03:25Z

It seems the incorrect painting of an icon on Windows is caused by the screen scale settings. When I set the scale to 100% the icons are drawn correctly, and incorrectly If I increase the scale.
Looking at what is returned by devicePixelRatioF() method, it seems it always return 1.0. No matter what the global screen scale factor is. Setting custom ration via dpr var also doesn't fix the incorrect drawing of an icon.

Therefore, I think it's best to pain the icon image directly via QIcon::paint method. At least on the Windows, it works for every scale setting.

phoerious · 2021-01-22T15:20:58Z

I have a scaling of 225% and I can barely see jagged edges.

smlu · 2021-01-22T19:04:00Z

I have it set to 125% and it looks like this is the worst scale setting to have. Increasing scaling above 125%, smooths a bit the jagged edges but the icon is still not clear.

phoerious · 2021-01-23T08:29:21Z

Can you submit your fix as a PR?

smlu · 2021-01-24T20:27:32Z

Ok yes, I can make PR.

phoerious added user interface platform: macOS ux labels Dec 18, 2020

phoerious added this to the v2.7.0 milestone Dec 18, 2020

phoerious requested review from droidmonkey and varjolintu December 18, 2020 23:42

phoerious changed the title ~~Improve macOS platform integration.~~ Improve macOS platform integration Dec 18, 2020

phoerious force-pushed the feature/macos-fixes branch 6 times, most recently from d793753 to aa190bb Compare December 19, 2020 00:28

phoerious force-pushed the feature/macos-fixes branch from aa190bb to dab965e Compare December 19, 2020 00:51

phoerious force-pushed the feature/macos-fixes branch from dab965e to 8f8bb83 Compare December 19, 2020 10:09

phoerious force-pushed the feature/macos-fixes branch 8 times, most recently from 137a057 to c5bd808 Compare December 20, 2020 16:38

phoerious changed the title ~~Improve macOS platform integration~~ Improve macOS and Windows platform integration Dec 23, 2020

phoerious force-pushed the feature/macos-fixes branch 3 times, most recently from 8f1c7cf to 3f110ef Compare December 24, 2020 09:33

phoerious force-pushed the feature/macos-fixes branch from 3f110ef to 425292a Compare December 30, 2020 14:34

droidmonkey approved these changes Jan 7, 2021

View reviewed changes

src/gui/Application.cpp Outdated Show resolved Hide resolved

phoerious added 2 commits January 7, 2021 14:45

Add dynamic theme switching on Windows 10

1349266

phoerious force-pushed the feature/macos-fixes branch from 425292a to 1349266 Compare January 7, 2021 14:03

phoerious merged commit 9a7b20c into develop Jan 7, 2021

phoerious deleted the feature/macos-fixes branch January 7, 2021 14:22

phoerious mentioned this pull request Jan 7, 2021

Backports #5899

Merged

smlu reviewed Jan 21, 2021

View reviewed changes

smlu mentioned this pull request Jan 24, 2021

Fix adaptive icon painting #5989

Merged

yan12125 mentioned this pull request Jan 28, 2021

TOTP and attachment icons are black after #5851 #6006

Closed

phoerious mentioned this pull request Jan 31, 2021

Backport live theme switching #6034

Merged

droidmonkey mentioned this pull request Oct 29, 2021

macOS: Keepassxc settings stay always in dark mode #3959

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve macOS and Windows platform integration #5851

Improve macOS and Windows platform integration #5851

phoerious commented Dec 18, 2020 •

edited

Loading

droidmonkey commented Dec 19, 2020

varjolintu commented Dec 19, 2020

phoerious commented Dec 19, 2020

phoerious commented Dec 19, 2020

varjolintu commented Dec 19, 2020

phoerious commented Dec 23, 2020

droidmonkey commented Dec 25, 2020

phoerious commented Dec 25, 2020

stonerl commented Dec 30, 2020

phoerious commented Dec 31, 2020

stonerl commented Dec 31, 2020

smlu Jan 21, 2021

phoerious commented Jan 22, 2021 •

edited

Loading

phoerious commented Jan 22, 2021

smlu commented Jan 22, 2021

phoerious commented Jan 22, 2021

smlu commented Jan 22, 2021

phoerious commented Jan 23, 2021

smlu commented Jan 24, 2021

Improve macOS and Windows platform integration #5851

Improve macOS and Windows platform integration #5851

Conversation

phoerious commented Dec 18, 2020 • edited Loading

Screenshots

Type of change

droidmonkey commented Dec 19, 2020

varjolintu commented Dec 19, 2020

phoerious commented Dec 19, 2020

phoerious commented Dec 19, 2020

varjolintu commented Dec 19, 2020

phoerious commented Dec 23, 2020

droidmonkey commented Dec 25, 2020

phoerious commented Dec 25, 2020

stonerl commented Dec 30, 2020

phoerious commented Dec 31, 2020

stonerl commented Dec 31, 2020

smlu Jan 21, 2021

Choose a reason for hiding this comment

phoerious commented Jan 22, 2021 • edited Loading

phoerious commented Jan 22, 2021

smlu commented Jan 22, 2021

phoerious commented Jan 22, 2021

smlu commented Jan 22, 2021

phoerious commented Jan 23, 2021

smlu commented Jan 24, 2021

phoerious commented Dec 18, 2020 •

edited

Loading

phoerious commented Jan 22, 2021 •

edited

Loading