search

allyourbot / hostedgpt

An open version of ChatGPT you can host anywhere or run locally.
MIT License
193 stars 75 forks source link

#169 Enabling Color Theme User Settings #285

Closed jasonpaulso closed 3 weeks ago

jasonpaulso commented 4 weeks ago

Settings form accepts color theme selection options of "dark", "light", or "system". Either of the former two are read by the script added to document head immediately after submitting changes. Loosely based upon docs here.

Screenshot 2024-04-19 at 4 32 58 PM Screenshot 2024-04-19 at 4 32 45 PM
jasonpaulso commented 4 weeks ago

@krschacht Same here re: The section #messages was able to move down so it was not at the bottom.

krschacht commented 3 weeks ago

@jasonpaulso Actually, I was reflecting on this a bit more. First, on naming, let’s not call it theme. Very soon I want to implement multiple complete front-end designs and I think we should reserve theme for that (e.g. theme may be “chatgpt” or “claude” since I’d like to support the styling of claude next).

That means we need a different word for dark mode. I can’t think of a single word that people use and the most common phrase is probably “dark mode” so let’s just go with dark_mode as the key. That’s the first thing.

Second, it’s unfortunate that we have to use javascript. It feels a bit kludgy. If I take a step back the most elegant implementation of this would be: the <body class=“” can receive dark, light, or system. I just spent a little while googling and asking chatgpt and I don’t think tailwind supports this level of elegance. It’s unfortunate. But we can approximate it.

So how about… update the layout file to do just that: apply one of those 3 words. This will be an elegant one-liner:

<body class=“<%= user.preferences.dark_mode %>”

That’s it. This means you need to add a customer deserializer to the User class so that user.preferences.dark_mode returns system in the event this key is missing. But I think that’s a good thing. It’s nice to have this fallback mode handled within the model and then we can avoid any kind of conditionals at the view layer.

But lastly, light vs system class will not actually work so then you can update the javascript to reflect this. Your javascript snippet can: check the body class and if the body class is system then it checks the media just as you’re doing and based on the media it adds light or dark.

I realize that adding light does not actually do anything, but that’s okay. It will feel intuitive when we view source.

krschacht commented 3 weeks ago

Ohh, another thing: the bit of JavaScript that realizes we are in “system” mode and automatically applies light or dark, let’s make that a stimulus controller rather than inline JavaScript. That will help ensure this is durable in a world where we are doing Turbo magic.

So on the body tag itself let’s do the usual data-controller and the controller can be called dark mode. Then in its connect function it can check the body class, if neither light or dark is applied then it adds one.

jasonpaulso commented 3 weeks ago

hey @krschacht I'm not sure what happened here. the merge/rebase seems to have been botched. I think I need to close this PR, do some resetting and re-PR.