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

Add custom light and dark themes #4110

Merged
merged 2 commits into from
Mar 5, 2020
Merged

Add custom light and dark themes #4110

merged 2 commits into from
Mar 5, 2020

Conversation

phoerious
Copy link
Member

@phoerious phoerious commented Jan 6, 2020

Type of change

  • ✅ New feature (non-breaking change which adds functionality)

Description and Context

Since the platform-native widgets are very hard to customise without breaking stuff and people keep (rightfully) complaining about KeePassXC's "oldfashioned" design, this PR adds a custom-made light and dark theme.

The themes are based on Andrew Richard's Phantomstyle (https://github.com/randrew/phantomstyle) with many modifications and custom CSS styling. Since they are implemented as QStyles, they require no UI rewrite and can be provided alongside the "classic" look for those who may prefer it.

Screenshots

Light:

Screenshot 2020-01-18 at 17 16 03
image
image

Dark:

image

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

@phoerious phoerious added this to the v2.6.0 milestone Jan 6, 2020
@phoerious phoerious force-pushed the feature/ui-styling branch 2 times, most recently from 5d16d8e to 71e3e0b Compare January 6, 2020 21:54
@droidmonkey
Copy link
Member

I don't like the centered tabs or section labels

@phoerious
Copy link
Member Author

phoerious commented Jan 7, 2020

Yeah, we can discuss that. It's something I wanted to try, but I'm not quite sure yet if I'm satisfied with it. There are also other things I don't yet like such as the buttons in the light theme. The colour shades also need to be tuned for better harmony.

It turns out Qt stylesheets are more powerful than I thought, but less powerful than I'd actually need them to be for proper theming. A lot of things are only possible with complicated C++ drawing instructions.

I would also like to make the settings dialogues responsive, but I don't know if that's possible without rewriting everything in true HTML+CSS.

@droidmonkey
Copy link
Member

The search help panel needs to be written in HTML + CSS

@varjolintu
Copy link
Member

varjolintu commented Jan 8, 2020

Here's how it looks on non-HiDPI display with macOS High Sierra, which doesn't have a native dark theme support like Mojave:

EDIT: Removed outdated images.

@phoerious
Copy link
Member Author

Did you restart KeePassXC after switching to native? On KDE it looked more or less okay, but it's possible that reapplying the proxy style doesn't always work properly.

@varjolintu
Copy link
Member

@phoerious Restarting fixed it. However, every time you change the theme for the first time, it takes around 5-10 seconds on my machine and KeePassXC hits 100% CPU. Plus the warning message for dev build still has white text when dark theme is enabled.

@phoerious
Copy link
Member Author

phoerious commented Jan 8, 2020

I have not fixed the message widget yet, but I am aware of it.
On Linux I could change the theme without issues. It only took a second or so.

@OLLI-S
Copy link

OLLI-S commented Jan 8, 2020

I really love the new themes, this makes KeePassXC more modern.

In the 3rd screen shot (settings) I see that the headlines of the groups (in the old layout you had group boxes) are centered.
Under usability issues / software-ergonomics issues this is problematic, because you have a long distance between the headline and the fields below the headlines (If you open the screen shot to see it in normal size, you see that the distance is not small):

image

If the user opens the settings then he automatically focuses elements that are on the left side, because this is a learned behavior from other applications (we also read from left to right).
So the first element that the user recognizes is the list of checkboxes.

But in what context are these checkboxes?
There is no headline shown above the checkboxes (the headline is not left-aligned) so it is not recognized at once.
This irritates the user and this is a little stop in the workflow, because he has to search any information what is the context of the checkboxes.

This searching requires user-action (he has to move the eyes and also the read to search for the headline).
This costs a little bit time, not much, but it interrupts the workflow.

After the user has read the headline, he has to focus the checkboxes again (also move the head and the eyes).

If you show the headlines left-aligned, they are recognized much faster and the user does not have to search for them.
This saves the user some time, does not interrupt the process and this layout (headlines left-aligned) is a common and learned layout.

Same for the tabs, they should also be left-aligned.

Here speaks my "usability" side (I work in usability, ux, accessibility and software-ergonomics.

@Aetf
Copy link
Contributor

Aetf commented Jan 9, 2020

How does this integrate with native Qt themes, like KDE Breeze?

@phoerious
Copy link
Member Author

phoerious commented Jan 9, 2020

@OLLI-S Centred tabs is something I wanted to try, since it looks familiar for macOS users (and macOS users deserve a better-looking UI the most I suppose). I do agree that it doesn't really work here and will change it back to left-aligned tabs. There are also a few other things I have changed already which haven't been pushed yet (such as getting rid of rounded corners entirely). Thanks for your analysis, it is much appreciated. If you find anything else, please do tell.

@Aetf It doesn't, but for KDE, you can always stick with the "classic" theme. I will integrate some platform-specific styles, though, for instance to match the background colour with the window title colour on macOS. We already have some macOS-specific alignment fixes for the "classic" theme, particularly for buttons, which I need to adjust as well (see the weird layout bugs and unreadable tabs above).

Someone else also wrote something here, but I can't find the comment anymore. The fact that the new themes contain more white space is by design (punny, I know) as the previous theme was very cramped. Perhaps I can make that configurable, but for now it's there to stay.

@OLLI-S
Copy link

OLLI-S commented Jan 9, 2020

@phoerious If you have changed the layout so the posted screen shots are no longer up-to-date, can you please post new ones?

I showed the screen shots above to a colleague of mine.
He uses Apple devices (also iPhone and Apple Watch) and also has the newest Beta of the OS on all devices.
He also is a designer.

His feedback after a quick look is that the layout is "old fashioned" (it was used many years ago) and not state-of-the-art.
I will investigate him for deeper feedback.

@phoerious
Copy link
Member Author

The layout is a pretty standard GUI layout, not a fancy web UI. I would like to keep these two things separate. This PR is only about the theme, not about the GUI layout, which we can only change to a certain degree anyway due to the underlying widget toolkit.

@droidmonkey
Copy link
Member

What's old is new again 😜

@phoerious
Copy link
Member Author

phoerious commented Jan 9, 2020

Yeah, I'm not against redesigning individual widgets, but I would like to keep the extra effort in balance with the gains. But in any case, that's a different (set of) PRs.

@OLLI-S
Copy link

OLLI-S commented Jan 9, 2020

Do you have to be a programmer to design the widgets?
Or is there an "editor" that you can use without programming skills?

@phoerious
Copy link
Member Author

There is an editor, but it is extremely technical and not good for anything beyond stitching together standard widgets.

@phoerious phoerious force-pushed the feature/ui-styling branch from 0252680 to 03d9e5b Compare March 4, 2020 22:44
@droidmonkey
Copy link
Member

This looks fantastic, I recommend merging!

@phoerious phoerious merged commit 557736e into develop Mar 5, 2020
@phoerious phoerious deleted the feature/ui-styling branch March 5, 2020 08:24
@michaelblyons
Copy link

Can we see post-merge screenshots to get excited about? 😉

@phoerious
Copy link
Member Author

You can certainly build and test it.

@droidmonkey
Copy link
Member

Even easier! Download the snapshot: https://snapshot.keepassxc.org

@wolframroesler
Copy link
Contributor

@wolframroesler
Copy link
Contributor

The new theme looks really cool :) Great work @phoerious.

However, may I suggest you change the default theme to "platform native" for macOS builds? Most Mac people want all apps to look alike, and application-specific themes are usually frowned upon in the Mac world. It's OK to support the theme as an option, but it shouldn't be the default.

@phoerious
Copy link
Member Author

No, absolutely not. The classic theme is horrendous and far from what a platform-native look should be on macOS. Qt is doing a very poor job at supporting macOS properly. Also, there are probably more custom themes these days than actual "native" ones, so the whole "frowned upon" thing is more folklore than anything else.

@phoerious
Copy link
Member Author

phoerious commented Mar 6, 2020

That said, it's more important to follow the platform HIGs and provide an overall pleasant experience than it is to use native widgets. Qt itself certainly does neither. And macOS users from my experience are usually quite content with whatever they get as long as it looks decent. Linux users tend to be much worse in this regard, as those are usually the guys who do insist that every application look the same. Linux users are actually the main reason why I kept the classic option to begin with. If it were only macOS, I might have removed it entirely, not least since classic on macOS looks the worst of all platforms.

droidmonkey added a commit that referenced this pull request Jul 7, 2020
Added

- Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915]
- Compact mode to use classic Group and Entry line height [#4910]
- View menu to quickly switch themes, compact mode, and toggle UI elements [#4910]
- Search for groups and scope search to matched groups [#4705]
- Save Database Backup feature [#4550]
- Sort entries by "natural order" and move lines up/down [#4357]
- Option to launch KeePassXC on system startup/login [#4675]
- Caps Lock warning on password input fields [#3646]
- Add "Size" column to entry view [#4588]
- Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305]
- Password Generator: Define additional characters to choose from [#3876]
- Reports: Database password health check (offline) [#3993]
- Reports: HIBP online service to check for breached passwords [#4438]
- Auto-Type: DateTime placeholders [#4409]
- Browser: Show group name in results sent to browser extension [#4111]
- Browser: Ability to define a custom browser location (macOS and Linux only) [#4148]
- Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591]
- CLI: `db-info` command [#4231]
- CLI: Use wl-clipboard if xclip is not available (Linux) [#4323]
- CLI: Incorporate xclip into snap builds [#4697]
- SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545]
- SSH Agent: Context menu actions to add/remove keys [#4290]

Changed

- Complete replacement of default database icons [#4699]
- Complete replacement of application icons [#4066, #4161, #4203, #4411]
- Complete rewrite of documentation and manpages using Asciidoctor [#4937]
- Complete refactor of config files; separate between local and roaming [#4665]
- Complete refactor of browser integration and proxy code [#4680]
- Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843]
- Significantly improve performance when saving and opening databases [#4309, #4833]
- Remove read-only detection for database files [#4508]
- Overhaul of password fields and password generator [#4367]
- Replace instances of "Master Key" with "Database Credentials" [#4929]
- Change settings checkboxes to positive phrasing for consistency [#4715]
- Improve UX of using entry actions (focus fix) [#3893]
- Set expiration time to Now when enabling entry expiration [#4406]
- Always show "New Entry" in context menu [#4617]
- Issue warning before adding large attachments [#4651]
- Improve importing OPVault [#4630]
- Improve AutoOpen capability [#3901, #4752]
- Check for updates every 7 days even while still running [#4752]
- Improve Windows installer UI/UX [#4675]
- Improve config file handling of portable distribution [#4131, #4752]
- macOS: Hide dock icon when application is hidden to tray [#4782]
- Browser: Use unlock dialog to improve UX of opening a locked database [#3698]
- Browser: Improve database and entry settings experience [#4392, #4591]
- Browser: Improve confirm access dialog [#2143, #4660]
- KeeShare: Improve monitoring file changes of shares [#4720]
- CLI: Rename `create` command to `db-create` [#4231]
- CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313]
- CLI: Use stderr for help text and password prompts [#4086, #4623]
- FdoSecrets: Display existing secret service process [#4128]

Fixed

- Fix changing focus around the main window using tab key [#4641]
- Fix search field clearing while still using the application [#4368]
- Improve search help widget displaying on macOS and Linux [#4236]
- Return keyboard focus after editing an entry [#4287]
- Reset database path after failed "Save As" [#4526]
- Use SHA256 Digest for Windows code signing [#4129]
- Improve handling of ccache when building [#4104, #4335]
- macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909]
- Auto-Type: Fix crash when performing on new entry [#4132]
- Browser: Send legacy HTTP settings to recycle bin [#4589]
- Browser: Fix merging browser keys [#4685]
- CLI: Fix encoding when exporting database [#3921]
- SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595]
- FdoSecrets: Fix crash when editing settings before service is enabled [#4332]
@michaelblyons
Copy link

The 2.6.0 went out on Homebrew recently, and I'm sure you're getting a flurry of issue reports about one thing or another...

But I feel compelled to say these themes are amazing. Great work!

@phoerious
Copy link
Member Author

Appreciated.

@zyv
Copy link

zyv commented Jul 11, 2020

Hey, as a Mac person, I was suffering from the native theme being horrible and even more horrible in the dark mode, but I kept suffering because KeePassXC is just too good.

Now, I don't need to suffer anymore. I'm amazed by the greatness of the new theme after I installed the update. It looks absolutely brilliant in the dark mode, and blends a lot better with the native environment than the old one ever did. I haven't used it extensively, but the first impression was "WOW!!!11" - even if there are small bugs here & there, it's just such a huge leap forward as compared to the old theme, and I'm sure with Big Sur it will only get better in terms of blending into the environment.

Everyone who has worked on making this a reality, huge thanks to all of you, you work is greatly appreciated!

@zyv
Copy link

zyv commented Jul 11, 2020

Probably the first macOS-specific bug ;) the separator is rendered as a box with stripes:

Screenshot 2020-07-11 at 10 56 08

@phoerious
Copy link
Member Author

Could you make a separate bug report for that please? Looks like your font has a different understanding og what bullet points are supposed to look like.

@kattjevfel
Copy link

kattjevfel commented Jul 13, 2020

Well this certainly looks terribly out of place now, can we get a version with system colours please? Or the very least an option to bring the old theme back?

Application to the right is using native dark theme colours, like it should.

EDIT: Apparently there is the option to fall back to the classic theme, very well hidden I might add. Adding it under "User Interface" would make too much sense I guess.

@The-Compiler
Copy link

If anyone else is looking for the option @kattjevfel mentioned: It's in the menu under "View -> Theme -> Classic". I also expected to find this in the UI options and ended up searching in the source code to find the menu entry 😉

@droidmonkey
Copy link
Member

droidmonkey commented Jul 21, 2020

Should have searched the documentation 😛

https://keepassxc.org/docs/KeePassXC_GettingStarted.html#_application_settings

@The-Compiler
Copy link

The-Compiler commented Jul 21, 2020

@droidmonkey Fair point, thanks! Note to self: Only because the documentation for my own open-source project is rather terse, don't assume that's the case for things I use as well 😆. Props for the nice documentation with topics and screenshots and all!

@Kadah
Copy link

Kadah commented Dec 11, 2020

Just wanted to note that "Classic" only affects the color scheme, the name of the option gave the impression that it would possibly do more. The flat Modern UI does not agree with my eyes, the text is difficult to read (something about Modern UI causes my vision to blur over time). Not saying that is KeepPassXC's issue, I lay that blame on W10 and the Modern UI design spec, the whole OS in defaults does this to me too.

If anything was to change about this... "Classic" could be labeled "Use system colors", or just a hover tip of the latter.

I'm a edge case, so for W10 I'll stick with 2.5.4.

@droidmonkey
Copy link
Member

droidmonkey commented Dec 11, 2020

2.5.4 themeing and classic themeing in 2.6 are exactly the same thing....

@Kadah
Copy link

Kadah commented Jan 12, 2021

2.5.4 themeing and classic themeing in 2.6 are exactly the same thing....

Went back to double check. You're partly correct, the text rendering is actually the same in classic+compact on 2.6.2.

Would seem its just the new flat blurry iconography combined with W10 UI that is triggering the problem and makes the whole window painful to interact with. On an old W7 install in classic but not compact, the new icons aren't quite as troublesome, but still too harsh to be usable for me.

@phoerious phoerious added pr: new feature Pull request that adds a new feature and removed new feature labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EPIC pr: new feature Pull request that adds a new feature user interface ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.