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

Expose default theme in meta and API #13809

Merged
merged 2 commits into from
Dec 3, 2020
Merged

Conversation

jolheiser
Copy link
Member

As title, this was requested in Discord primarily for a dark reader to be able to check the DOM for the default theme.

As well, this adds the default theme to the UI API.

@jolheiser jolheiser added topic/ui Change the appearance of the Gitea UI modifies/api This PR adds API routes or modifies them labels Dec 3, 2020
@jolheiser jolheiser added this to the 1.14.0 milestone Dec 3, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Dec 3, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 3, 2020
Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

This is default theme, we should report what the user’s current theme is as well

@lunny
Copy link
Member

lunny commented Dec 3, 2020

@techknowlogick That could be another PR. :)

@techknowlogick techknowlogick dismissed their stale review December 3, 2020 05:02

Dismissed per Lunny comment

@jolheiser
Copy link
Member Author

This is default theme, we should report what the user’s current theme is as well

A separate meta tag (that would only show up when the user is signed it)?

@lafriks
Copy link
Member

lafriks commented Dec 3, 2020

What is use case for default theme in meta tag as we don't use it in javascript?

@silverwind
Copy link
Member

silverwind commented Dec 3, 2020

Default theme is probably fine, but please don't do user's theme. I plan on moving the user's theme setting to JavaScript (e.g. removing it from the database and having it in localStorage only) later.

A separate meta tag

Theme is already a className on the HTML tag, isn't that enough?

@silverwind
Copy link
Member

silverwind commented Dec 3, 2020

Thought, I'm not sure yet how to do the theme loading is JS because accessing localStorage during page load will be problematic (requires inline script), so we might end up keeping the theme in the database after all.

@6543
Copy link
Member

6543 commented Dec 3, 2020

🚀

@6543 6543 merged commit e306c29 into go-gitea:master Dec 3, 2020
@jolheiser jolheiser deleted the theme-meta branch December 3, 2020 14:39
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
@silverwind
Copy link
Member

silverwind commented May 27, 2023

We'll remove this meta tag in #24960, it is non-standard and does not reflect current theme, just the server's default, so it was never accurate.

I don't know what "dark reader" the original PR is refering to, but it's not the official one. Such extensions should either check the standard color-scheme or our custom --is-dark-theme CSS variable set on the <html> node to derive whether the current theme is dark.

--is-dark-theme: true;

color-scheme: dark;

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants