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

Color mode preference #5337

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

Conversation

AntonKhorev
Copy link
Collaborator

A quick implementation of auto/light scheme switch for those who want to stay on the light scheme even when their browser says they want the dark one.

image
image
image

Its on the /preferences page but it's not actually a preference stored in the User model. It's stored in a cookie. The question is do we want to store it in a cookie? Maybe we do because what browser reports as a preferred scheme is per browser, not per user, then perhaps its override should also be per browser. And it will be easier to extend the availability of this option to anonymous users. It is kind of available already, if they know how to edit cookies.

@tomhughes
Copy link
Member

If it's in the user preferences it should be stored in the database not in cookies.

@AntonKhorev
Copy link
Collaborator Author

But should it be in user preferences?

@AntonKhorev
Copy link
Collaborator Author

And if stored in the database, should it be in User or UserPreference? This one depends on how do we want to make it available to other apps.

@hlfan
Copy link
Contributor

hlfan commented Nov 16, 2024

I don't think this preference mashup between local and database storage is a great idea.

But if logged-in users have the preference synchronized and non-logged-in users have the color and language (editor choice seems pointless for non-logged-in users) stored in the cookies it could be interesting.

Or just decuple the local settings from the synced ones.

@AntonKhorev
Copy link
Collaborator Author

By making the setting available to other apps I meant iD of course. Its color scheme preferably should match the entire site, that means it has to know somehow whether to use prefers-color-scheme or ignore it. Let's suppose we made this setting somehow readable through the api. iD will look at it and adjust its color scheme accordingly.

Now let's suppose we don't use iD embedded in the osm-website. We'll run it from https://ideditor-release.netlify.app/, It can still read the setting in the api, but the reason to apply it is gone because iD is no longer embedded. Then maybe it's better to pass it as a startup parameter through

<% data = { :configured => Settings.key?(:id_application) }

In this case we wouldn't need to worry how exactly we store the setting.

@hlfan
Copy link
Contributor

hlfan commented Nov 16, 2024

How would not logged-in users change this setting?

@AntonKhorev
Copy link
Collaborator Author

AntonKhorev commented Nov 16, 2024

How would not logged-in users change this setting?

At first they they wouldn't be able to, like with languages.

Whether this will work depends on how disruptive is whatever they run instead of the native dark mode, whether it will prevent them from logging in for their preferences to take effect.

@gravitystorm
Copy link
Collaborator

I would prefer #5339

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.

4 participants