From 53985dc630b0c39ea2d20f9d807af1b2d44c1d95 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Tue, 11 Aug 2026 13:49:48 +0200 Subject: [PATCH] :bug: Fix setting dark theme on onboarding (#11212) * :bug: Fix setting dark theme on onboarding * :tada: Add test --- backend/src/app/rpc/commands/auth.clj | 2 +- .../register/verify-token-email-verified.json | 28 ++++++++++++ frontend/playwright/ui/pages/RegisterPage.js | 36 +++++++++++++++ .../ui/specs/email-verification.spec.js | 45 +++++++++++++++++++ frontend/src/app/main/data/profile.cljs | 40 +++++++++-------- 5 files changed, 131 insertions(+), 20 deletions(-) create mode 100644 frontend/playwright/data/register/verify-token-email-verified.json create mode 100644 frontend/playwright/ui/specs/email-verification.spec.js diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index 07460633c4..78d2ac45c4 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -367,7 +367,7 @@ email (str/lower email) fullname (d/normalize-string (:fullname params)) locale (d/normalize-string locale) - theme (d/normalize-string theme) + theme (some-> theme d/normalize-string not-empty) photo-id (some->> (or (:oidc/picture props) (:google/picture props) diff --git a/frontend/playwright/data/register/verify-token-email-verified.json b/frontend/playwright/data/register/verify-token-email-verified.json new file mode 100644 index 0000000000..347bc4057c --- /dev/null +++ b/frontend/playwright/data/register/verify-token-email-verified.json @@ -0,0 +1,28 @@ +{ + "~:iss": "~:verify-email", + "~:profile-id": "~uc7ce0794-0992-8105-8004-38e630f29a9b", + "~:profile": { + "~:id": "~uc7ce0794-0992-8105-8004-38e630f29a9b", + "~:email": "foo@example.com", + "~:fullname": "Princesa Leia", + "~:auth-backend": "penpot", + "~:is-active": true, + "~:is-demo": false, + "~:is-muted": false, + "~:is-blocked": false, + "~:theme": "", + "~:default-team-id": "~uc7ce0794-0992-8105-8004-38e630f40f6d", + "~:default-project-id": "~uc7ce0794-0992-8105-8004-38e630f7920b", + "~:created-at": "~m1713533116365", + "~:modified-at": "~m1713533116365", + "~:props": { + "~:nudge": { + "~:big": 10, + "~:small": 1 + }, + "~:v2-info-shown": true, + "~:viewed-tutorial?": false, + "~:viewed-walkthrough?": false + } + } +} diff --git a/frontend/playwright/ui/pages/RegisterPage.js b/frontend/playwright/ui/pages/RegisterPage.js index 8d3633e678..ef43f56469 100644 --- a/frontend/playwright/ui/pages/RegisterPage.js +++ b/frontend/playwright/ui/pages/RegisterPage.js @@ -29,6 +29,42 @@ export class RegisterPage extends BasePage { ); } + /** + * Mocks a successful email-verification token exchange (the link the + * user clicks from the verification email) and every RPC the dashboard + * needs to render right after landing on it, so the flow can be + * exercised end-to-end without a real backend. + */ + async setupEmailVerificationSuccess() { + await this.mockConfigFlags(["disable-onboarding"]); + await this.mockRPC( + "verify-token", + "register/verify-token-email-verified.json", + ); + await this.mockRPCs({ + "get-teams": "logged-in-user/get-teams-default.json", + "get-font-variants?team-id=*": + "logged-in-user/get-font-variants-empty.json", + "get-projects?team-id=*": "logged-in-user/get-projects-default.json", + "get-team-members?team-id=*": + "logged-in-user/get-team-members-your-penpot.json", + "get-team-users?team-id=*": + "logged-in-user/get-team-users-single-user.json", + "get-unread-comment-threads?team-id=*": + "logged-in-user/get-team-users-single-user.json", + "get-team-recent-files?team-id=*": + "logged-in-user/get-team-recent-files-empty.json", + "get-profiles-for-file-comments": + "logged-in-user/get-profiles-for-file-comments-empty.json", + "get-builtin-templates": + "logged-in-user/get-built-in-templates-empty.json", + }); + } + + async goToVerifyToken(token = "verify-email-token") { + await this.page.goto(`/#/auth/verify-token?token=${token}`); + } + static async init(page) { await BasePage.init(page); } diff --git a/frontend/playwright/ui/specs/email-verification.spec.js b/frontend/playwright/ui/specs/email-verification.spec.js new file mode 100644 index 0000000000..6be76653ce --- /dev/null +++ b/frontend/playwright/ui/specs/email-verification.spec.js @@ -0,0 +1,45 @@ +import { test, expect } from "@playwright/test"; +import { RegisterPage } from "../pages/RegisterPage"; + +// Regression test for the bug where a freshly verified account (whose +// profile never had a theme persisted) ended up with an empty string as +// its theme instead of falling back to the dark default: the workspace +// switched to light mode and Settings > UI Theme showed a blank field. + +test.beforeEach(async ({ page }) => { + await RegisterPage.initWithLoggedOutUser(page); +}); + +test.describe("Email verification", () => { + test("Newly verified account defaults to the dark theme", async ({ + page, + }) => { + const registerPage = new RegisterPage(page); + await registerPage.setupEmailVerificationSuccess(); + + await registerPage.goToVerifyToken(); + await page.waitForURL("**/dashboard/**"); + + // `default` is the body class applied for dark theme, `light` for + // light theme (see app.util.theme/set-color-scheme). + await expect(page.locator("body")).toHaveClass(/default/); + await expect(page.locator("body")).not.toHaveClass(/light/); + }); + + test("Settings > UI Theme shows Penpot Dark (default) selected, not blank", async ({ + page, + }) => { + const registerPage = new RegisterPage(page); + await registerPage.setupEmailVerificationSuccess(); + + await registerPage.goToVerifyToken(); + await page.waitForURL("**/dashboard/**"); + + await page.goto("/#/settings/options"); + + // The language select is the first combobox on the page, the theme + // select is the second one. + const themeSelect = page.getByRole("combobox").nth(1); + await expect(themeSelect).toHaveText("Penpot Dark (default)"); + }); +}); diff --git a/frontend/src/app/main/data/profile.cljs b/frontend/src/app/main/data/profile.cljs index 93a948c119..3f1237c2b7 100644 --- a/frontend/src/app/main/data/profile.cljs +++ b/frontend/src/app/main/data/profile.cljs @@ -43,28 +43,30 @@ (defn set-profile "Initialize profile state, only logged-in profile data should be passed to this event" - [{:keys [id] :as profile}] - (ptk/reify ::set-profile - IDeref - (-deref [_] profile) + [profile] + (let [profile (update profile :theme not-empty) + id (:id profile)] + (ptk/reify ::set-profile + IDeref + (-deref [_] profile) - ptk/UpdateEvent - (update [_ state] - (-> state - (assoc :profile-id id) - (assoc :profile profile))) + ptk/UpdateEvent + (update [_ state] + (-> state + (assoc :profile-id id) + (assoc :profile profile))) - ptk/WatchEvent - (watch [_ state _] - (let [profile (:profile state)] - (->> (rx/from (i18n/set-locale (:lang profile))) - (rx/ignore)))) + ptk/WatchEvent + (watch [_ state _] + (let [profile (:profile state)] + (->> (rx/from (i18n/set-locale (:lang profile))) + (rx/ignore)))) - ptk/EffectEvent - (effect [_ state _] - (let [profile (:profile state)] - (swap! storage/user assoc :profile profile) - (plugins.register/init))))) + ptk/EffectEvent + (effect [_ state _] + (let [profile (:profile state)] + (swap! storage/user assoc :profile profile) + (plugins.register/init)))))) (def profile-fetched? (ptk/type? ::profile-fetched))