-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add theme selector button to footer #27576
base: main
Are you sure you want to change the base?
Conversation
{{$currentTheme := ThemeName .SignedUser}} | ||
<span class="flex-text-inline">{{svg (ThemeIcon $currentTheme) 14}} {{$currentTheme}}</span> | ||
<div class="menu theme-menu"> | ||
{{range Themes}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do that, I think it might also be a good idea to sort the themes alphabetically beforehand (once, on Gitea startup).
Otherwise, we might confuse users if the order changes because the admin changed the value of the setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sorting is a good idea, can add. Not sure if really warranted to store in-memory because sorting a small array is fast, but can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, we store the array already, so I don't see why it would consume extra memory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, we can just sort setting.UI.Themes
after config parsing, so no additional memory needed.
Regarding "nice theme name" and icons, I was thinking we could accept a alternate value for THEMES = gitea-auto,gitea-light,gitea-dark It would also accept THEMES = '[{"name":"gitea-auto", "title": "Auto", "icon": "gitea-eclipse"},{"name": "gitea-light", "title": "Light", "icon": "octicon-sun"},{"name":"gitea-dark", "title": "Dark", "icon": "octicon-moon"}]' A bit unusual for our configs, but this would offer maximum flexibility to both us and the users. |
if theme == "gitea-dark" { | ||
return "octicon-moon" | ||
} else if theme == "gitea-light" { | ||
return "octicon-sun" | ||
} else if theme == "gitea-auto" { | ||
return "gitea-eclipse" | ||
} else { | ||
return "octicon-paintbrush" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if theme == "gitea-dark" { | |
return "octicon-moon" | |
} else if theme == "gitea-light" { | |
return "octicon-sun" | |
} else if theme == "gitea-auto" { | |
return "gitea-eclipse" | |
} else { | |
return "octicon-paintbrush" | |
} | |
switch theme { | |
case "gitea-dark": | |
return "octicon-moon" | |
case "gitea-light": | |
return "octicon-sun" | |
case "gitea-auto": | |
return "gitea-eclips" | |
default: | |
return "octicon-paintbrush" | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the switch way of writing, but gitea doesn't seem to have a normalized way of writing this, so it's the same with either one of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will go with the JSON config, making this switch unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> Improve theme display #30671
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that seems nice, waiting for that PR here.
Will pick this up again soon. |
Maybe for built-in themes names could come from locale files so that they could be translated, for custom just use theme name |
Would only be possible for the default one, unless we extend that config json with translation keys, but I think no one is going to bother to translate their theme names. At some point the translation madness has to stop 😆. |
Yes, I meant only to have translatable Auto, Light, Dark theme names |
Add this button to the footer to quickly switch themes:
Custom themes currently render with this icon, but I guess we could add some heuristics to detect dark/light/auto for theme if they have such a suffix in the name.
Maybe we want to "beautify" these names, e.g.
gitea-dark
toGitea Dark
or justDark
.Also, I cleaned up various code related to theme selection.