Change default theme from light to the system's default theme #2544
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller
Unchanged No assets were unchanged |
adam-rozen
changed the title
|
Hello! This automated visual testing stuff is very cool and makes it easy to tell what changed. The visual regression test failed, looks to be due to the fact that the symbol on the default theme is the computer, while it expects the light theme to be set and using the sun symbol. I can't check all the photos due to some file system limitations (file names are too long). Would appreciate some advice here on the next steps! |
There are instructions how to run the VRT tests locally and update the snapshots here: https://github.com/actualbudget/actual/tree/master/packages/desktop-client/README.md |
|
Thanks for updating that. I just figured out late yesterday that the reason I was struggling with running the tests was my computer's speed. Updating the timeout fixed it, but I hadn't had a chance to commit & push it yet. |
I guess I should have been a bit more patient. Glad you got your end figured out |
|
(totally unrelated to your PR and it should not block merging this PR you did here) Do folks understand what the desktop icon means? I get what the "sun" icon (or moon) would represent. But if we're now defaulting to "desktop" version - do people actually understand what the default desktop icon represents? Personally, I wouldn't understand what it is (unless I clicked on it). |
It had been discussed removing that button unless in a dev environment. I imagine people really only use it to compare the themes, then don't use it again unless testing changes |
That makes sense! It would also reduce the amount of buttons we have on screen (thus reduce clutter) :) |
trafico-bot
bot
added
✨ Merged
This PR is to change the default visual theme from light to the system default.
At the moment, light theme is always default. There is already functionality built-in to Actual for using the system theme, so this change will just set the system theme be the default for Actual. Users can still override the theme manually like before, but now Actual will obey the user's set preference right away. This change only comes into play when the theme is not set.
I tried to test with the playwright module, but the test
Schedules › creates a new schedule, posts the transaction and later completes itfailed saying that theTarget page, context, or browser has been closedand then it timed out. Might be a local issue, so submitting the PR to see if the automated build passes.