-
Notifications
You must be signed in to change notification settings - Fork 86
fix: properly handle light/dark theme switch #18324
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
Conversation
Jenkins Builds
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! I tested on Linux and Mac and it works really nice.
The thing that is questionable to me is using system theme on login page even if dark/light is explicitly specified in settings:
Screencast.from.09.07.2025.11.49.50.webm
I think that we should respect user's choice also on the login page.
The other thing is why we need to keep this on the nim side. I think that property should not be synced - it's specific to given device, therefore regular qt's Settings
should be suitable here.
It's done that since before the login, there are no user preferences available (no user/profile logged in yet -> no settings), so we opt to always follow the system theme. Nothing we could do/fix here really |
Component.onCompleted: {
console.info(">>> %1 %2 started, using Qt version %3".arg(Qt.application.name).arg(Qt.application.version).arg(SystemUtils.qtRuntimeVersion()))
Theme.changeTheme(Theme.Style.System); ^^ this is main.qml, during Onboarding but the settings are stored in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
It's funny how the color switch when using the System theme is super smooth and when switching manually in the app is very sudden, but it's not a real issue
Yeah because it's the system/OS compositor doing the color transition, not us 😆 |
- under Qt6, we can now listen to the styleHints color scheme property changes and react accordingly - remove the hack of detecting the current dark theme by checking the hsl lightness of the window; also removes propagating the systemPalette object theu the hierarchy - as a result, we can get rid of the dedicated C++ OSThemeEvent Fixes #17764
814575d
to
e6b2d68
Compare
What does the PR do
Fixes #17764
Fixes #14856
Needs status-im/dotherside#95
Needs status-im/nimqml#67
Affected areas
App/Settings
Architecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
Zaznam.obrazovky.z.2025-07-08.20-04-35.mp4
Storybook:

Impact on end user
Settings/Appearance/System should follow system light/dark theme changes