mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: add theme_mode, theme_light, theme_dark to UserAppearanceSettings (#25076)
Part 1: Backend portion of a change broken into 2 PRs. Part 2: #25077 Adds three new UserAppearanceSettings fields (theme_mode, theme_light, theme_dark) on top of the existing theme_preference and terminal_font. Replaces GetUserThemePreference and GetUserTerminalFont with a single GetUserAppearanceSettings aggregate query. The PUT handler is wrapped in db.InTx so sync-mode's mode + slot writes can never half-apply.
This commit is contained in:
+19
-24
@@ -354,6 +354,16 @@ func execTmpl(tmpl *template.Template, state htmlState) ([]byte, error) {
|
||||
return buf.Bytes(), err
|
||||
}
|
||||
|
||||
func userAppearanceSettingsFromRow(settings database.GetUserAppearanceSettingsRow) codersdk.UserAppearanceSettings {
|
||||
return codersdk.UserAppearanceSettings{
|
||||
ThemePreference: settings.ThemePreference,
|
||||
ThemeMode: codersdk.ThemeMode(settings.ThemeMode),
|
||||
ThemeLight: settings.ThemeLight,
|
||||
ThemeDark: settings.ThemeDark,
|
||||
TerminalFont: codersdk.TerminalFontName(settings.TerminalFont),
|
||||
}
|
||||
}
|
||||
|
||||
// renderWithState will render the file using the given nonce if the file exists
|
||||
// as a template. If it does not, it will return an error.
|
||||
func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state htmlState) ([]byte, error) {
|
||||
@@ -396,8 +406,7 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
|
||||
|
||||
var eg errgroup.Group
|
||||
var user database.User
|
||||
var themePreference string
|
||||
var terminalFont string
|
||||
var userAppearance codersdk.UserAppearanceSettings
|
||||
orgIDs := []uuid.UUID{}
|
||||
var userOrgs []database.Organization
|
||||
eg.Go(func() error {
|
||||
@@ -406,22 +415,12 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
|
||||
return err
|
||||
})
|
||||
eg.Go(func() error {
|
||||
var err error
|
||||
themePreference, err = h.opts.Database.GetUserThemePreference(ctx, apiKey.UserID)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
themePreference = ""
|
||||
return nil
|
||||
settings, err := h.opts.Database.GetUserAppearanceSettings(ctx, apiKey.UserID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return err
|
||||
})
|
||||
eg.Go(func() error {
|
||||
var err error
|
||||
terminalFont, err = h.opts.Database.GetUserTerminalFont(ctx, apiKey.UserID)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
terminalFont = ""
|
||||
return nil
|
||||
}
|
||||
return err
|
||||
userAppearance = userAppearanceSettingsFromRow(settings)
|
||||
return nil
|
||||
})
|
||||
eg.Go(func() error {
|
||||
memberIDs, err := h.opts.Database.GetOrganizationIDsByMemberIDs(ctx, []uuid.UUID{apiKey.UserID})
|
||||
@@ -446,7 +445,7 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
|
||||
})
|
||||
err := eg.Wait()
|
||||
if err == nil {
|
||||
h.populateHTMLState(ctx, &state, af, actor, user, orgIDs, userOrgs, themePreference, terminalFont)
|
||||
h.populateHTMLState(ctx, &state, af, actor, user, orgIDs, userOrgs, userAppearance)
|
||||
}
|
||||
|
||||
return execTmpl(tmpl, state)
|
||||
@@ -463,8 +462,7 @@ func (h *Handler) populateHTMLState(
|
||||
user database.User,
|
||||
orgIDs []uuid.UUID,
|
||||
userOrgs []database.Organization,
|
||||
themePreference string,
|
||||
terminalFont string,
|
||||
userAppearance codersdk.UserAppearanceSettings,
|
||||
) {
|
||||
var wg sync.WaitGroup
|
||||
wg.Go(func() {
|
||||
@@ -474,10 +472,7 @@ func (h *Handler) populateHTMLState(
|
||||
}
|
||||
})
|
||||
wg.Go(func() {
|
||||
data, err := json.Marshal(codersdk.UserAppearanceSettings{
|
||||
ThemePreference: themePreference,
|
||||
TerminalFont: codersdk.TerminalFontName(terminalFont),
|
||||
})
|
||||
data, err := json.Marshal(userAppearance)
|
||||
if err == nil {
|
||||
state.UserAppearance = html.EscapeString(string(data))
|
||||
}
|
||||
|
||||
@@ -81,6 +81,72 @@ func TestInjection(t *testing.T) {
|
||||
require.Equal(t, db2sdk.User(user, []uuid.UUID{}), got)
|
||||
}
|
||||
|
||||
func TestInjectionUserAppearance(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
siteFS := fstest.MapFS{
|
||||
"index.html": &fstest.MapFile{
|
||||
Data: []byte("{{ .UserAppearance }}"),
|
||||
},
|
||||
}
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
handler, err := site.New(&site.Options{
|
||||
Telemetry: telemetry.NewNoop(),
|
||||
Database: db,
|
||||
SiteFS: siteFS,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
user := dbgen.User(t, db, database.User{})
|
||||
ctx := context.Background()
|
||||
_, err = db.UpdateUserThemePreference(ctx, database.UpdateUserThemePreferenceParams{
|
||||
UserID: user.ID,
|
||||
ThemePreference: "dark-tritan",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = db.UpdateUserThemeMode(ctx, database.UpdateUserThemeModeParams{
|
||||
UserID: user.ID,
|
||||
ThemeMode: string(codersdk.ThemeModeSync),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = db.UpdateUserThemeLight(ctx, database.UpdateUserThemeLightParams{
|
||||
UserID: user.ID,
|
||||
ThemeLight: "light-tritan",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = db.UpdateUserThemeDark(ctx, database.UpdateUserThemeDarkParams{
|
||||
UserID: user.ID,
|
||||
ThemeDark: "dark-tritan",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = db.UpdateUserTerminalFont(ctx, database.UpdateUserTerminalFontParams{
|
||||
UserID: user.ID,
|
||||
TerminalFont: string(codersdk.TerminalFontFiraCode),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, token := dbgen.APIKey(t, db, database.APIKey{
|
||||
UserID: user.ID,
|
||||
ExpiresAt: time.Now().Add(time.Hour),
|
||||
})
|
||||
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
r.Header.Set(codersdk.SessionTokenHeader, token)
|
||||
rw := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(rw, r)
|
||||
require.Equal(t, http.StatusOK, rw.Code)
|
||||
var got codersdk.UserAppearanceSettings
|
||||
err = json.Unmarshal([]byte(html.UnescapeString(rw.Body.String())), &got)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, codersdk.UserAppearanceSettings{
|
||||
ThemePreference: "dark-tritan",
|
||||
ThemeMode: codersdk.ThemeModeSync,
|
||||
ThemeLight: "light-tritan",
|
||||
ThemeDark: "dark-tritan",
|
||||
TerminalFont: codersdk.TerminalFontFiraCode,
|
||||
}, got)
|
||||
}
|
||||
|
||||
func TestRenderPermissionsResolvesMe(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -289,6 +289,9 @@ export const updateAppearanceSettings = (
|
||||
// more responsive.
|
||||
queryClient.setQueryData(myAppearanceKey, {
|
||||
theme_preference: patch.theme_preference,
|
||||
theme_mode: patch.theme_mode,
|
||||
theme_light: patch.theme_light,
|
||||
theme_dark: patch.theme_dark,
|
||||
terminal_font: patch.terminal_font,
|
||||
});
|
||||
},
|
||||
|
||||
Generated
+44
@@ -7908,6 +7908,11 @@ export const TerminalFontNames: TerminalFontName[] = [
|
||||
"",
|
||||
];
|
||||
|
||||
// From codersdk/users.go
|
||||
export type ThemeMode = "single" | "sync" | "";
|
||||
|
||||
export const ThemeModes: ThemeMode[] = ["single", "sync", ""];
|
||||
|
||||
// From codersdk/users.go
|
||||
export type ThinkingDisplayMode =
|
||||
| "always_collapsed"
|
||||
@@ -8398,6 +8403,28 @@ export interface UpdateTemplateMeta {
|
||||
// From codersdk/users.go
|
||||
export interface UpdateUserAppearanceSettingsRequest {
|
||||
readonly theme_preference: string;
|
||||
/**
|
||||
* ThemeMode is optional for backward compatibility. When empty,
|
||||
* the server leaves theme_mode, theme_light, and theme_dark
|
||||
* unchanged so older CLI clients do not erase sync-mode settings.
|
||||
* Legacy auto preferences are the exception: they clear theme_mode
|
||||
* so clients can migrate the old sync-with-system setting.
|
||||
*/
|
||||
readonly theme_mode: ThemeMode;
|
||||
/**
|
||||
* ThemeLight is required when ThemeMode is "sync". In "single"
|
||||
* mode an empty value means "preserve the previously persisted
|
||||
* slot" rather than "clear the slot", so partial updates that send
|
||||
* only one slot keep the other intact.
|
||||
*/
|
||||
readonly theme_light: string;
|
||||
/**
|
||||
* ThemeDark is required when ThemeMode is "sync". In "single" mode
|
||||
* an empty value means "preserve the previously persisted slot"
|
||||
* rather than "clear the slot", so partial updates that send only
|
||||
* one slot keep the other intact.
|
||||
*/
|
||||
readonly theme_dark: string;
|
||||
readonly terminal_font: TerminalFontName;
|
||||
}
|
||||
|
||||
@@ -8699,7 +8726,24 @@ export interface UserActivityInsightsResponse {
|
||||
|
||||
// From codersdk/users.go
|
||||
export interface UserAppearanceSettings {
|
||||
/**
|
||||
* ThemePreference is the legacy single-field appearance setting. In
|
||||
* "single" mode it mirrors the active theme. In "sync" mode modern
|
||||
* clients normally mirror the active OS slot, but older clients can
|
||||
* update only this field, so it may diverge from ThemeLight or
|
||||
* ThemeDark until a modern client saves the full appearance state
|
||||
* again.
|
||||
*/
|
||||
readonly theme_preference: string;
|
||||
readonly theme_mode: ThemeMode;
|
||||
/**
|
||||
* Ignored when ThemeMode is "single"
|
||||
*/
|
||||
readonly theme_light: string;
|
||||
/**
|
||||
* Ignored when ThemeMode is "single"
|
||||
*/
|
||||
readonly theme_dark: string;
|
||||
readonly terminal_font: TerminalFontName;
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,12 @@ type Story = StoryObj<typeof AppearanceForm>;
|
||||
|
||||
export const Example: Story = {
|
||||
args: {
|
||||
initialValues: { theme_preference: "", terminal_font: "" },
|
||||
initialValues: {
|
||||
theme_preference: "",
|
||||
theme_mode: "",
|
||||
theme_light: "",
|
||||
theme_dark: "",
|
||||
terminal_font: "",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@@ -52,6 +52,9 @@ export const AppearanceForm: FC<AppearanceFormProps> = ({
|
||||
}
|
||||
await onSubmit({
|
||||
theme_preference: theme,
|
||||
theme_mode: "",
|
||||
theme_light: "",
|
||||
theme_dark: "",
|
||||
terminal_font: currentTerminalFont,
|
||||
});
|
||||
};
|
||||
@@ -62,6 +65,9 @@ export const AppearanceForm: FC<AppearanceFormProps> = ({
|
||||
}
|
||||
await onSubmit({
|
||||
theme_preference: currentTheme,
|
||||
theme_mode: "",
|
||||
theme_light: "",
|
||||
theme_dark: "",
|
||||
terminal_font: terminalFont,
|
||||
});
|
||||
};
|
||||
|
||||
@@ -12,6 +12,9 @@ describe("appearance page", () => {
|
||||
vi.spyOn(API, "updateAppearanceSettings").mockResolvedValueOnce({
|
||||
...MockUserOwner,
|
||||
theme_preference: "dark",
|
||||
theme_mode: "single",
|
||||
theme_light: "light",
|
||||
theme_dark: "dark",
|
||||
terminal_font: "fira-code",
|
||||
});
|
||||
|
||||
@@ -29,6 +32,9 @@ describe("appearance page", () => {
|
||||
...MockUserOwner,
|
||||
terminal_font: "geist-mono",
|
||||
theme_preference: "light",
|
||||
theme_mode: "single",
|
||||
theme_light: "light",
|
||||
theme_dark: "dark",
|
||||
});
|
||||
|
||||
const light = await screen.findByText("Light");
|
||||
@@ -36,10 +42,12 @@ describe("appearance page", () => {
|
||||
|
||||
// Check if the API was called correctly
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledTimes(1);
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledWith({
|
||||
terminal_font: "geist-mono",
|
||||
theme_preference: "light",
|
||||
});
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
terminal_font: "geist-mono",
|
||||
theme_preference: "light",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("changes font to fira code", async () => {
|
||||
@@ -49,6 +57,9 @@ describe("appearance page", () => {
|
||||
...MockUserOwner,
|
||||
terminal_font: "fira-code",
|
||||
theme_preference: "dark",
|
||||
theme_mode: "single",
|
||||
theme_light: "light",
|
||||
theme_dark: "dark",
|
||||
});
|
||||
|
||||
const firaCode = await screen.findByText("Fira Code");
|
||||
@@ -56,10 +67,12 @@ describe("appearance page", () => {
|
||||
|
||||
// Check if the API was called correctly
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledTimes(1);
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledWith({
|
||||
terminal_font: "fira-code",
|
||||
theme_preference: "dark",
|
||||
});
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
terminal_font: "fira-code",
|
||||
theme_preference: "dark",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("changes font to fira code, then back to geist mono", async () => {
|
||||
@@ -71,11 +84,17 @@ describe("appearance page", () => {
|
||||
...MockUserOwner,
|
||||
terminal_font: "fira-code",
|
||||
theme_preference: "dark",
|
||||
theme_mode: "single",
|
||||
theme_light: "light",
|
||||
theme_dark: "dark",
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
...MockUserOwner,
|
||||
terminal_font: "geist-mono",
|
||||
theme_preference: "dark",
|
||||
theme_mode: "single",
|
||||
theme_light: "light",
|
||||
theme_dark: "dark",
|
||||
});
|
||||
|
||||
// when
|
||||
@@ -84,10 +103,12 @@ describe("appearance page", () => {
|
||||
|
||||
// then
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledTimes(1);
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledWith({
|
||||
terminal_font: "fira-code",
|
||||
theme_preference: "dark",
|
||||
});
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
terminal_font: "fira-code",
|
||||
theme_preference: "dark",
|
||||
}),
|
||||
);
|
||||
|
||||
// when
|
||||
const geistMono = await screen.findByText("Geist Mono");
|
||||
@@ -95,9 +116,12 @@ describe("appearance page", () => {
|
||||
|
||||
// then
|
||||
expect(API.updateAppearanceSettings).toHaveBeenCalledTimes(2);
|
||||
expect(API.updateAppearanceSettings).toHaveBeenNthCalledWith(2, {
|
||||
terminal_font: "geist-mono",
|
||||
theme_preference: "dark",
|
||||
});
|
||||
expect(API.updateAppearanceSettings).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
terminal_font: "geist-mono",
|
||||
theme_preference: "dark",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,6 +34,9 @@ const AppearancePage: FC = () => {
|
||||
error={updateAppearanceSettingsMutation.error}
|
||||
initialValues={{
|
||||
theme_preference: appearanceSettingsQuery.data.theme_preference,
|
||||
theme_mode: appearanceSettingsQuery.data.theme_mode,
|
||||
theme_light: appearanceSettingsQuery.data.theme_light,
|
||||
theme_dark: appearanceSettingsQuery.data.theme_dark,
|
||||
terminal_font: appearanceSettingsQuery.data.terminal_font,
|
||||
}}
|
||||
onSubmit={updateAppearanceSettingsMutation.mutateAsync}
|
||||
|
||||
@@ -562,6 +562,9 @@ export const SuspendedMockUser: TypesGen.User = {
|
||||
|
||||
export const MockUserAppearanceSettings: TypesGen.UserAppearanceSettings = {
|
||||
theme_preference: "dark",
|
||||
theme_mode: "single",
|
||||
theme_light: "light",
|
||||
theme_dark: "dark",
|
||||
terminal_font: "",
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user