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

Get things in place for a color scheme setting. #629

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

trey
Copy link
Member

@trey trey commented Oct 14, 2020

I think this is close if I knew how to create a database migration. :)

Issue #627

I think this is close if I knew how to create a database migration. :)
@bradchoate
Copy link
Collaborator

@heyitsal @spaceninja What do you think? Should this be a "color scheme" account setting, or should it be a "Theme" account setting? I think the latter allows for eventual alternate themes beyond light/dark/"default" selections.

@bradchoate
Copy link
Collaborator

@trey In addition to creating a database migration, you'll need to update the setup/db-install.sql and setup/db-fixtures.sql files. Those are used to create an initial database structure for tests and for local development. All of the migrations are "applied" from running those two scripts (since db-install.sql represents the current schema, any schema changes should be made there also).

Feel free to document this for the project README file. I mention how to apply any migrations against a local instance, but now how to develop one.

@spaceninja
Copy link
Member

I said "color scheme" in the issue, but I don't have any particularly strong feelings about it. I'd be fine with calling it "Theme"

@@ -1,5 +1,5 @@
<!doctype html>
<html>
<html{% if current_user_object and current_user_object != 'match' %} class="{{ current_user_object.color_scheme }}"{% end %}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know Python, but think this is what you mean?

Suggested change
<html{% if current_user_object and current_user_object != 'match' %} class="{{ current_user_object.color_scheme }}"{% end %}>
<html{% if current_user_object and current_user_object.color_scheme != 'match' %} class="{{ current_user_object.color_scheme }}"{% end %}>

Copy link
Member

@spaceninja spaceninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for starting this!

@bradchoate
Copy link
Collaborator

The macOS setting is labelled "Appearance".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants