Add system theme to site themes #20107
Conversation
This is so that not-logged-in users will have their system preferences respected out of the box.
There was a problem hiding this comment.
Overall this looks like a really good change, though I need to think about it a bit more as far as third-party themes are concerned (many instances fork the code and add a theme).
With my glitch-soc (a Mastodon fork) hat on, I'm also a bit concerned about how I'd port that to glitch-soc's theming system, but obviously that should not be a showstopper for upstream.
|
Thanks for the prompt review, @ClearlyClaire! And thanks for the added insight—didn't know that it was common for forks to exist that added themes. I'm all ears on any suggestions should you think of any. |
Use "Automatic (use system theme)" per suggested commented in GitHub.
nshki
changed the title
|
Have you considered having the 'system' theme just switch between a user-selected light and dark theme? |
That's a great point. @Gaelan had a great proof of concept in #10976 that allowed for user-selected light/dark themes. I wasn't considering custom themes and thought that having a system theme option would be simpler, but now that I'm aware of the existence of custom themes, maybe that's a better approach. @ClearlyClaire would having explicit, user-selected light/dark themes make it easier for forks like glitch-soc to pull from upstream? |
|
I was thinking something along these lines for the settings menu (sorry about burning your eyeballs).
|
|
I like that! (No eyeballs were burned.) There was a comment in the associated issue that shows a similar idea where the user checks an option to explicitly select different themes for light and dark mode. I'm leaning towards perhaps adding a "dark mode theme" dropdown that gets shown when the checkbox is checked. |
|
Sorry for coming back to this late, but I was kinda busy with other things—including the security fix that touched some of that very code. Thinking about it, I think being able to configure themes separately would indeed be good, but I think this can be a separate improvement. Having an automatic selection between two defaults theme would already be an improvement. |
No worries at all! I figured you probably have a lot of other things to deal with. Thanks for taking the time to chip in here.
And understood! I'll spend some time fixing up the merge conflicts and will rewrite the markup generation portion using DOM functions instead of a template string. Happy to work on the theme configuration PR separately once everything is good here as well. EDIT: I've pushed a commit that resolves merge conflicts and swaps template string usage for |
|
Ah, I was just checking the contributing guidelines and missed the part about authorizing CircleCI to run things from my forked repo. Just logged into CircleCI and gave it access to my forked EDIT: Hm. It seems like there are a few issues:
I'm likely missing something here. If someone could give me pointers, that would be appreciated. EDIT: Found the source of the |
|
This change would default Mastodon to using the light theme for the vast majority of users, including unauthenticated users and users who haven't specifically overridden it. I do not believe our light theme is in a polished enough state for this change to be merged and I would strongly suggest we continue use the dark theme as the "default" theme unless the user has manually chosen to override it. |
|
As a counterpoint: right now, for any instance you don't have an account on, it's impossible to choose a theme at all. So, I feel like the downside of having the light theme become default (for users who haven't set their browser/OS to dark mode), might be easily outweighed by everyone gaining the ability to choose between light and dark for all Mastodon instances they visit. |
|
@nightpool while I definitely understand the concern, I haven't seen any indication that the light theme is too unpolished. (I've been using the light theme for a majority of my time with Mastodon web.) Are there any specific examples that come to mind? @Cykelero also brings up a great point that I agree with—giving unauthenticated users the ability to pick a theme depending on their OS preferences seems to outweigh the cons here. |
This bundles the new application stylesheet pack tags into a shared partial that's then used across multiple layouts. This wasn't necessary before because only one pack tag was being rendered based on the user's current theme. Now we must conditionally render different pack tags with media queries if applicable.
Follow-up from last commit. This attempts to fix an issue where `Warden::Proxy` isn't available during the precompilation step. Instead of calling out to `current_theme` in layouts where it didn't previously, this reverts the rendering of stylesheet pack tags to explicit stylesheets in `layouts/embedded.html.haml` and `layouts/error.html.haml`.
|
I'm still not sure why they aren't reflected in the
|
|
I'd just like to say a thank you to everyone. IMHO this is an invaluable addition. |
|
This pull request has resolved merge conflicts and is ready for review. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
This pull request has resolved merge conflicts and is ready for review. |
|
Whats the status of this PR? |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
This pull request has resolved merge conflicts and is ready for review. |
|
@Gargron Any chance of getting this merged for 4.2.0? Eagerly awaiting this feature and disappointed to see it lingering for months at final review |
|
@Gargron It would be great if this PR can be merged for 4.2.2. Competing social networks, such as Bluesky and Threads, already have auto dark mode, and it's on by default. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
This pull request has resolved merge conflicts and is ready for review. |
Using single quotes instead of double quotes where interpolation isn't needed.
|
Got tired of waiting, and I'm actually using an alternative web client. Good luck with this PR. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
This pull request has resolved merge conflicts and is ready for review. |
Issue
Closes #10976.
Overview
This PR adds a "System settings" option to site themes, allowing users to not have to explicitly select their preferred app theme whenever they change light/dark mode on their OS. This PR takes into consideration the raised points in #10976. Notably:
Screenshots/video
Screen.Recording.2022-11-08.at.1.31.58.AM.mov
Misc notes
I just wanted to say thank you Eugen and all the maintainers for this wonderful project. As a recent adopter of Mastodon, I'm super enthusiastic about its future and hope to be able to make small contributions when I can.