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

Rename the default themes to gitea-light, gitea-dark, gitea-auto #27419

Merged
merged 30 commits into from
Oct 6, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Oct 3, 2023

Part of #27097:

  • gitea theme is renamed to gitea-light
  • arc-green theme is renamed to gitea-dark
  • auto theme is renamed to gitea-auto

I put both themes in separate CSS files, removing all colors from the base CSS. Existing users will be migrated to the new theme names. The dark theme recolor will follow in a separate PR.

⚠️ BREAKING ⚠️

  1. If there are existing custom themes with the names gitea-light, gitea-dark or gitea-auto, rename them before this upgrade and update the theme column in the user table for each affected user.
  2. The theme name in the <html> tag has moved from class="theme-name" to data-theme="name", existing customizations that depend on the previous class need to be updated.
  3. If your configuration sets one of the previous themes as ui.DEFAULT_THEME, you need to update to the new values.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 3, 2023
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 3, 2023
@github-actions github-actions bot added type/docs This PR mainly updates/creates documentation topic/ui Change the appearance of the Gitea UI labels Oct 3, 2023
@silverwind silverwind added this to the 1.22.0 milestone Oct 3, 2023
@silverwind
Copy link
Member Author

silverwind commented Oct 3, 2023

user is a reserved word in postgres (where it should be escaped as "user") and mssql (where it should be escaped as [user]). Does xorm handle this kind of escaping correctly when using backticks? I've seen these backticks in use in a number of other migrations, so I assume xorm abstracts this escaping.

@GiteaBot GiteaBot added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Oct 4, 2023
@lunny
Copy link
Member

lunny commented Oct 4, 2023

user is a reserved word in postgres (where it should be escaped as "user") and mssql (where it should be escaped as [user]). Does xorm handle this kind of escaping correctly when using backticks? I've seen these backticks in use in a number of other migrations, so I assume xorm abstracts this escaping.

Yes, xorm will convert `word` according to different databases.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 4, 2023
@delvh
Copy link
Member

delvh commented Oct 4, 2023

Hmm… I'd like to wait with this PR until 1.21.0 has been released.
Otherwise, backports mostly won't be possible anymore.

@silverwind
Copy link
Member Author

Otherwise, backports mostly won't be possible anymore.

Which backports? Only backports that have CSS variable changes will present merge conflicts that are easily resolved.

@caesar
Copy link
Contributor

caesar commented Oct 4, 2023

Should the auto theme also be renamed to gitea-auto?

@silverwind
Copy link
Member Author

silverwind commented Oct 4, 2023

I guess we could, I like such consistency. Or we go the other way and name them auto,light,dark.

@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 Oct 4, 2023
@denyskon
Copy link
Member

denyskon commented Oct 4, 2023

I think gitea-* is better to indicate that these are our official themes

* origin/main:
  When comparing with an non-exist repository, return 404 but 500 (go-gitea#27437)
  Fix pr template (go-gitea#27436)
  Use minimal required version on CI and remove unnecessary services (go-gitea#27429)
  Fix  missing `ctx`  in new_form.tmpl  (go-gitea#27434)
  Use flex-container for repo and org settings (go-gitea#27418)
  Fix yet another `ctx` template bug (go-gitea#27417)
  Add Index to `action.user_id` (go-gitea#27403)
  [skip ci] Updated translations via Crowdin
@silverwind
Copy link
Member Author

silverwind commented Oct 4, 2023

One more change added: The theme name was previously on HTML class="theme-<name>", and has moved to data-theme="<name>". This was unused since the introduction of --is-dark-theme CSS variable, but I still find it good to retain under this new dedicated attribute for customization. I added a breaking note for it.

modules/templates/helper.go Outdated Show resolved Hide resolved
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 6, 2023
@silverwind silverwind merged commit 023e937 into go-gitea:main Oct 6, 2023
25 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 6, 2023
@silverwind silverwind deleted the lightdark branch October 6, 2023 07:46
* locale
* Flash
* ErrorMsg
* SignedUser (optional)
*/}}
<!DOCTYPE html>
<html lang="{{.locale.Lang}}" class="theme-{{if .SignedUser.Theme}}{{.SignedUser.Theme}}{{else}}{{DefaultTheme}}{{end}}">
<html lang="{{.locale.Lang}}" data-theme="{{ThemeName .SignedUser}}">
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the data-theme used for? Especially in the 500 template.

Copy link
Member Author

@silverwind silverwind Oct 6, 2023

Choose a reason for hiding this comment

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

Not used at all by our first-party code, but it's useful to have imho, if only for customization. Previously the class was used at one point in time for Monaco, but that has been replaced by --is-dark-theme.

zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 8, 2023
* giteaoffical/main: (79 commits)
  Pre-register OAuth application for tea (go-gitea#27509)
  Fix mermaid flowchart margin issue (go-gitea#27503)
  add a shortcut to user's profile page to admin user details (go-gitea#27299)
  Fix actionlint (go-gitea#27513)
  [skip ci] Updated translations via Crowdin
  Update JS and PY dependencies (go-gitea#27501)
  Improve feed icons and feed merge text color (go-gitea#27498)
  Downgrade `go-co-op/gocron` to v1.31.1 (go-gitea#27511)
  Enable markdownlint `no-duplicate-header` (go-gitea#27500)
  bump go-deps (go-gitea#27489)
  Apply to became a maintainer (go-gitea#27491)
  change runner for binary
  [skip ci] Updated translations via Crowdin
  Remove .exe suffix when cross-compiling on Windows (go-gitea#27448)
  move re-useable workflow
  add checkout to disk-clean
  use hosted runners for nightly actions (go-gitea#27485)
  Avoid run change title process when the title is same (go-gitea#27467)
  Fix panic in storageHandler (go-gitea#27446)
  Rename the default themes to gitea-light, gitea-dark, gitea-auto (go-gitea#27419)
  ...
@silverwind silverwind mentioned this pull request Oct 9, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 5, 2024
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. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants