-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Add theme global configuration from admin UI #35558
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -276,7 +276,7 @@ func TestCreateUserInvalidEmail(t *testing.T) { | |
Email: "[email protected]\r\n", | ||
Passwd: ";p['////..-++']", | ||
IsAdmin: false, | ||
Theme: setting.UI.DefaultTheme, | ||
Theme: setting.Config().Theme.DefaultTheme.Value(t.Context()), | ||
MustChangePassword: false, | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
package fileicon | ||
|
||
import ( | ||
"context" | ||
"html/template" | ||
"strings" | ||
|
||
|
@@ -34,7 +35,7 @@ func (p *RenderedIconPool) RenderToHTML() template.HTML { | |
} | ||
|
||
func RenderEntryIconHTML(renderedIconPool *RenderedIconPool, entry *EntryInfo) template.HTML { | ||
if setting.UI.FileIconTheme == "material" { | ||
if setting.Config().Theme.DefaultFileIconTheme.Value(context.Background()) == "material" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No I have spent enough time on cleaning up the rubbishes like
You have reviewed and approved those. If you don't agree that "context.Background()" should be avoided, just tell me and BLOCK my PRs. |
||
return DefaultMaterialIconProvider().EntryIconHTML(renderedIconPool, entry) | ||
} | ||
return BasicEntryIconHTML(entry) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,10 @@ func GetGeneralUISettings(ctx *context.APIContext) { | |
// "200": | ||
// "$ref": "#/responses/GeneralUISettings" | ||
ctx.JSON(http.StatusOK, api.GeneralUISettings{ | ||
DefaultTheme: setting.UI.DefaultTheme, | ||
AllowedReactions: setting.UI.Reactions, | ||
CustomEmojis: setting.UI.CustomEmojis, | ||
DefaultTheme: setting.Config().Theme.DefaultTheme.Value(ctx), | ||
DefaultFileIconTheme: setting.Config().Theme.DefaultFileIconTheme.Value(ctx), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why it needs to be exposed. What's the real use case? Who need it? If it is important, why no test to make sure it is a stable API? |
||
AllowedReactions: setting.UI.Reactions, | ||
CustomEmojis: setting.UI.CustomEmojis, | ||
}) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,7 +88,7 @@ func TestCreateUser(t *testing.T) { | |
Email: "[email protected]", | ||
Passwd: ";p['////..-++']", | ||
IsAdmin: false, | ||
Theme: setting.UI.DefaultTheme, | ||
Theme: setting.Config().Theme.DefaultTheme.Value(t.Context()), | ||
MustChangePassword: false, | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
package webtheme | ||
|
||
import ( | ||
"context" | ||
"regexp" | ||
"sort" | ||
"strings" | ||
|
@@ -107,19 +108,20 @@ func parseThemeMetaInfo(fileName, cssContent string) *ThemeMetaInfo { | |
|
||
func initThemes() { | ||
availableThemes = nil | ||
defaultTheme := setting.Config().Theme.DefaultTheme.Value(context.Background()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
defer func() { | ||
availableThemeInternalNames = container.Set[string]{} | ||
for _, theme := range availableThemes { | ||
availableThemeInternalNames.Add(theme.InternalName) | ||
} | ||
if !availableThemeInternalNames.Contains(setting.UI.DefaultTheme) { | ||
setting.LogStartupProblem(1, log.ERROR, "Default theme %q is not available, please correct the '[ui].DEFAULT_THEME' setting in the config file", setting.UI.DefaultTheme) | ||
if !availableThemeInternalNames.Contains(defaultTheme) { | ||
setting.LogStartupProblem(1, log.ERROR, "Default theme %q is not available, please correct the '[ui].DEFAULT_THEME' setting in the config file", defaultTheme) | ||
} | ||
}() | ||
cssFiles, err := public.AssetFS().ListFiles("/assets/css") | ||
if err != nil { | ||
log.Error("Failed to list themes: %v", err) | ||
availableThemes = []*ThemeMetaInfo{defaultThemeMetaInfoByInternalName(setting.UI.DefaultTheme)} | ||
availableThemes = []*ThemeMetaInfo{defaultThemeMetaInfoByInternalName(defaultTheme)} | ||
return | ||
} | ||
var foundThemes []*ThemeMetaInfo | ||
|
@@ -144,14 +146,14 @@ func initThemes() { | |
availableThemes = foundThemes | ||
} | ||
sort.Slice(availableThemes, func(i, j int) bool { | ||
if availableThemes[i].InternalName == setting.UI.DefaultTheme { | ||
if availableThemes[i].InternalName == defaultTheme { | ||
return true | ||
} | ||
return availableThemes[i].DisplayName < availableThemes[j].DisplayName | ||
}) | ||
if len(availableThemes) == 0 { | ||
setting.LogStartupProblem(1, log.ERROR, "No theme candidate in asset files, but Gitea requires there should be at least one usable theme") | ||
availableThemes = []*ThemeMetaInfo{defaultThemeMetaInfoByInternalName(setting.UI.DefaultTheme)} | ||
availableThemes = []*ThemeMetaInfo{defaultThemeMetaInfoByInternalName(defaultTheme)} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<h4 class="ui top attached header"> | ||
{{ctx.Locale.Tr "admin.config.theme"}} | ||
</h4> | ||
<div class="ui attached table segment dropdown-container"> | ||
<dl class="admin-dl-horizontal"> | ||
<dt>{{ctx.Locale.Tr "admin.config.default_theme"}}</dt> | ||
<dd> | ||
<div class="ui selection dropdown js-theme-config-dropdown" data-config-key="theme.default_theme"> | ||
{{svg "octicon-triangle-down" 14 "dropdown icon"}} | ||
<div class="text"> | ||
<strong>{{$.SystemConfig.Theme.DefaultTheme.Value ctx}}</strong> | ||
</div> | ||
<div class="menu"> | ||
{{range .AvailableThemes}} | ||
<div class="{{if eq ($.SystemConfig.Theme.DefaultTheme.Value ctx) .DisplayName}}active selected {{end}}item" data-value="{{.DisplayName}}">{{.DisplayName}}</div> | ||
{{end}} | ||
</div> | ||
</div> | ||
</dd> | ||
<div class="divider"></div> | ||
<dt>{{ctx.Locale.Tr "admin.config.default_file_icon_theme"}}</dt> | ||
<dd> | ||
<div class="ui selection dropdown js-theme-config-dropdown" data-config-key="theme.default_file_icon_theme"> | ||
{{svg "octicon-triangle-down" 14 "dropdown icon"}} | ||
<div class="text"> | ||
<strong>{{$.SystemConfig.Theme.DefaultFileIconTheme.Value ctx}}</strong> | ||
</div> | ||
<div class="menu"> | ||
{{range .AvailableFileIconThemes}} | ||
<div class="{{if eq ($.SystemConfig.Theme.DefaultFileIconTheme.Value ctx) .}}active selected {{end}}item" data-value="{{.}}">{{.}}</div> | ||
{{end}} | ||
</div> | ||
</div> | ||
</dd> | ||
</dl> | ||
</div> |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,12 @@ import ( | |
"time" | ||
|
||
repo_model "code.gitea.io/gitea/models/repo" | ||
system_model "code.gitea.io/gitea/models/system" | ||
"code.gitea.io/gitea/models/unit" | ||
"code.gitea.io/gitea/models/unittest" | ||
user_model "code.gitea.io/gitea/models/user" | ||
"code.gitea.io/gitea/modules/setting" | ||
"code.gitea.io/gitea/modules/setting/config" | ||
"code.gitea.io/gitea/modules/test" | ||
"code.gitea.io/gitea/modules/util" | ||
repo_service "code.gitea.io/gitea/services/repository" | ||
|
@@ -223,7 +225,13 @@ func testViewRepo1CloneLinkAuthorized(t *testing.T) { | |
|
||
func testViewRepoWithSymlinks(t *testing.T) { | ||
defer tests.PrintCurrentTest(t)() | ||
defer test.MockVariableValue(&setting.UI.FileIconTheme, "basic")() | ||
defer func() { | ||
err := system_model.SetSettings(t.Context(), map[string]string{ | ||
setting.Config().Theme.DefaultFileIconTheme.DynKey(): "basic", | ||
}) | ||
assert.NoError(t, err) | ||
config.GetDynGetter().InvalidateCache() | ||
}() | ||
Comment on lines
+228
to
+234
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure what you are doing?
But your "defer" is only executed after the test? |
||
session := loginUser(t, "user2") | ||
|
||
req := NewRequest(t, "GET", "/user2/repo20.git") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ | |
.admin dl.admin-dl-horizontal dd { | ||
line-height: var(--line-height-default); | ||
padding: 5px 0; | ||
display: flex; | ||
align-items: center; | ||
} | ||
|
||
.admin dl.admin-dl-horizontal dt { | ||
|
@@ -39,6 +41,10 @@ | |
overflow-x: auto; /* if the screen width is small, many wide tables (eg: user list) need scroll bars */ | ||
} | ||
|
||
.admin .ui.table.segment.dropdown-container { | ||
overflow: visible; /* allow dropdown menus to extend beyond container boundaries */ | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which problematic |
||
|
||
.admin .table th { | ||
white-space: nowrap; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import {showTemporaryTooltip} from '../../modules/tippy.ts'; | ||
import {POST} from '../../modules/fetch.ts'; | ||
import {fomanticQuery} from '../../modules/fomantic/base.ts'; | ||
|
||
const {appSubUrl} = window.config; | ||
|
||
|
@@ -21,4 +22,28 @@ export function initAdminConfigs(): void { | |
} | ||
}); | ||
} | ||
|
||
// Handle theme config dropdowns | ||
for (const el of elAdminConfig.querySelectorAll('.js-theme-config-dropdown')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future, every time you introduce a "system setting config option", then you will copy&paste another tens of lines code? |
||
fomanticQuery(el).dropdown({ | ||
async onChange(value: string, _text: string, _$item: any) { | ||
if (!value) return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why |
||
|
||
const configKey = this.getAttribute('data-config-key'); | ||
if (!configKey) return; | ||
Comment on lines
+32
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why |
||
|
||
try { | ||
const resp = await POST(`${appSubUrl}/-/admin/config`, { | ||
data: new URLSearchParams({key: configKey, value}), | ||
}); | ||
const json: Record<string, any> = await resp.json(); | ||
if (json.errorMessage) throw new Error(json.errorMessage); | ||
} catch (ex) { | ||
showTemporaryTooltip(this, ex.toString()); | ||
// Revert the dropdown to the previous value on error | ||
fomanticQuery(el).dropdown('restore defaults'); | ||
} | ||
}, | ||
}); | ||
} | ||
} |
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.
You should not use
context.Background()
, as always.And the
AfterLoad
could be completely removed, if you have read the code.The
UserThemeName
used by templates is already able to handle empty theme name correctly