-
Notifications
You must be signed in to change notification settings - Fork 906
feat: remove dark blue theme #14890
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
feat: remove dark blue theme #14890
Conversation
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
the
|
pardon our messy e2e test output. that line you're looking at is unrelated, the real error was:
with no extra details. 🙃 I'm assuming it's a flake and just started a re-run. I'll be back in a bit to check on it and give you a review. |
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.
we'll also want to add a db migration that sets the theme of any users currently using the dark blue theme back to the default, otherwise they won't see anything selected on the appearance settings page.
it should just look like:
UPDATE users SET theme_preference = '' WHERE theme_preference = 'darkBlue';
and the down migration can just be an empty file with a comment saying something like
-- Nothing to restore
You can create the migration files by running...
./coderd/database/migrations/create_migration.sh remove-dark-blue-theme
85c93a5
to
5f78f57
Compare
5f78f57
to
a9274ee
Compare
a9274ee
to
f68e15e
Compare
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.
looks great! thank you!
saw this issue #14811, looks like an easy change
considered trying to modify the type exported from
UpdateUserAppearanceSettingsRequest
intypesGenerated.ts
to be"light" | "dark"
, but not super sure how to generate types from the Go codebase, so have left this as it is