diff --git a/CHANGELOG.md b/CHANGELOG.md index b865a32..d6f07bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Each browser keeps its own theme, so a phone and a desktop can differ. A browser that has not + chosen one yet starts from the theme your account had. - The Modern theme takes its colours from the new logo: its navy, the blue of its bars and the orange of its needle. - The logo follows the page: the dark version in a dark theme, the light one in a light theme. diff --git a/src/db.rs b/src/db.rs index c871f8a..44a758b 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1517,7 +1517,11 @@ impl Db { Ok(()) } - /// The theme this person chose, and light, dark or auto; None for either until they choose. + /// The theme this person chose when it was kept on the account, and light, dark or auto; None + /// for either if they never did. Only read now, to give a browser with no theme cookie its + /// first one (issue #69). + // ponytail: users.theme and theme_mode are never written any more; drop them once every + // browser in use has its own cookie. pub async fn theme(&self, user_id: i64) -> Result<(Option, Option)> { Ok(users::Entity::find_by_id(user_id) .one(&self.orm) @@ -1526,18 +1530,6 @@ impl Db { .unwrap_or_default()) } - pub async fn set_theme(&self, user_id: i64, theme: &str, mode: &str) -> Result<()> { - self.update_user( - user_id, - users::ActiveModel { - theme: Set(Some(theme.to_owned())), - theme_mode: Set(Some(mode.to_owned())), - ..Default::default() - }, - ) - .await - } - /// The user behind a session cookie, if it is still live. Idle sessions expire after /// `max_idle_secs`; touching `seen` is what keeps a session in daily use alive. pub async fn session_user(&self, token: &str, max_idle_secs: i64) -> Result> { diff --git a/src/web.rs b/src/web.rs index e1bc51a..f00dbe8 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1,6 +1,7 @@ //! Web front end. Runs inside the daemon so it reads SQLite and the event bus directly. use anyhow::{Context, Result}; +use axum::http::HeaderMap; use axum::{ Json, Router, extract::{Path, Query, Request, State}, @@ -252,7 +253,11 @@ impl axum::extract::FromRequestParts for crate::db::User { } fn cookie(req: &Request, name: &str) -> Option { - req.headers() + headers_cookie(req.headers(), name) +} + +fn headers_cookie(headers: &HeaderMap, name: &str) -> Option { + headers .get(header::COOKIE) .and_then(|v| v.to_str().ok()) .and_then(|c| { @@ -335,38 +340,25 @@ async fn me( ) -> Json { let url = state.ctx.cfg().web.sign_out_url.clone(); let sign_out = (by_proxy && !url.is_empty()).then_some(url); - let (theme, mode) = state.ctx.db.theme(user.id).await.unwrap_or_default(); let blocked = state.ctx.db.blocklist(user.id, "").await.unwrap_or_default(); Json(serde_json::json!({ - "name": user.name, "admin": user.is_admin, "sign_out": sign_out, "theme": theme, "mode": mode, - "blocked": blocked, + "name": user.name, "admin": user.is_admin, "sign_out": sign_out, "blocked": blocked, })) } #[derive(Deserialize)] struct MePatch { - theme: Option, - mode: Option, /// Words that hide an item in every feed you read. blocked: Option>, } -/// Saves the theme to the account, so it follows the person rather than the browser, and the -/// block list for every feed. +/// Saves the block list for every feed. The theme is not the account's: each browser keeps its +/// own, in a cookie (issue #69). async fn patch_me( State(state): State, user: crate::db::User, Json(body): Json, ) -> Result { - if body.theme.is_some() || body.mode.is_some() { - let (theme, mode) = (body.theme.unwrap_or_default(), body.mode.unwrap_or_default()); - // The page's script knows the themes; this only makes sure what is kept is safe to write - // into the page's tag, which is where index() puts it. - if !theme_ok(&theme, &mode) { - return Err(ApiError::bad_request("not a theme")); - } - state.ctx.db.set_theme(user.id, &theme, &mode).await?; - } if let Some(words) = body.blocked { state.ctx.db.set_blocklist(user.id, "", &clean_words(words)?).await?; } @@ -530,13 +522,26 @@ async fn app_css() -> impl IntoResponse { ) } +/// The cookie the page keeps its theme in, `.`, written by theme.ts. +const THEME_COOKIE: &str = "ipx_theme"; + +/// The theme this browser chose, from its cookie: per device, so a phone and a desktop signed in +/// as the same person can each have their own (issue #69). A browser without one yet gets the +/// theme the account kept from before, which theme.ts then writes into the cookie. +async fn page_theme(state: &WebState, user: &crate::db::User, headers: &HeaderMap) -> (Option, Option) { + if let Some((t, m)) = headers_cookie(headers, THEME_COOKIE).as_deref().and_then(|v| v.split_once('.')) { + return (Some(t.to_owned()), Some(m.to_owned())); + } + state.ctx.db.theme(user.id).await.unwrap_or_default() +} + /// The admin page and its script go to admins only: not just hidden from everyone else, never /// sent. Anyone else asking for the page is sent back to the app. -async fn admin_page(State(state): State, user: crate::db::User) -> Response { +async fn admin_page(State(state): State, user: crate::db::User, headers: HeaderMap) -> Response { if !user.is_admin { return Redirect::to("/").into_response(); } - let theme = state.ctx.db.theme(user.id).await.unwrap_or_default(); + let theme = page_theme(&state, &user, &headers).await; let page = with_theme(include_str!(concat!(env!("OUT_DIR"), "/admin.html")), theme); ([(header::CACHE_CONTROL, PAGE_CACHE)], Html(page)).into_response() } @@ -636,8 +641,8 @@ const VERSION_SLOT: &str = "iPX {version}"; /// The page, with the log button left out for anyone but an admin. Hiding it from the page's /// script instead showed it for a moment on every load, until /api/me answered. -async fn index(State(state): State, user: crate::db::User) -> impl IntoResponse { - let theme = state.ctx.db.theme(user.id).await.unwrap_or_default(); +async fn index(State(state): State, user: crate::db::User, headers: HeaderMap) -> impl IntoResponse { + let theme = page_theme(&state, &user, &headers).await; ([(header::CACHE_CONTROL, PAGE_CACHE)], Html(page_for(user.is_admin, theme))) } diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index f6dbda6..13218fd 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -63,13 +63,9 @@ test('Settings picks a theme and, where it has both, light, dark or Auto', async await page.locator('#stheme').selectOption('paper'); await expect(page.locator('#smode')).toBeHidden(); await expect.poll(bg).toBe('rgb(242, 238, 222)'); // #F2EEDE - // The save that says Classic: the ones before it may still be answering. - const saved = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH' - && r.request().postDataJSON().theme === 'classic'); await page.locator('#stheme').selectOption('classic'); await expect(page.locator('#smode')).toBeHidden(); - await saved; // kept on the account, not the browser - await page.reload(); + await page.reload(); // kept in this browser's cookie await expect.poll(root).toEqual(['classic', 'light']); // The 2004 Mac app set its type in Lucida Grande. expect(await page.evaluate(() => getComputedStyle(document.body).fontFamily)).toContain('Lucida Grande'); @@ -83,46 +79,40 @@ test('Settings picks a theme and, where it has both, light, dark or Auto', async await expect.poll(bg).toBe('rgb(46, 52, 64)'); // nord0 }); -test('the theme is kept on the account, and follows it to another browser', async ({ page, browser }) => { +test('each browser keeps its own theme, in a cookie, not on the account', async ({ page, browser }) => { + // Glass on a phone, Dracula on a desktop, signed in as the same person (issue #69). + let patched = false; + page.on('request', r => { if (r.url().endsWith('/api/me') && r.method() === 'PATCH') patched = true; }); await page.locator('#prefs').click(); - const saved = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH'); await page.locator('#stheme').selectOption('flatremix'); - expect((await saved).status()).toBe(204); - const saved2 = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH'); await page.locator('#smode').selectOption('light'); - await saved2; + const cookie = (await page.context().cookies()).find(c => c.name === 'ipx_theme'); + expect(cookie?.value).toBe('flatremix.light'); + expect(patched, 'nothing sent to the account').toBe(false); - // Another browser: nothing in its localStorage, and the page still arrives in the theme, - // written onto by the server rather than set once the script has run. + // This browser: the server reads the cookie and draws the page in it from the first frame. + const res = await page.reload(); + expect(await res.text()).toContain('data-theme=flatremix data-choice=light data-mode=light'); + + // Another browser, the same account: not this one's theme. const other = await browser.newContext(); const p2 = await other.newPage(); - const res = await p2.goto(`/?token=${TOKEN}`); - expect(await res.text()).toContain('data-theme=flatremix data-choice=light data-mode=light'); - expect(await p2.evaluate(() => [document.documentElement.dataset.theme, document.documentElement.dataset.mode])) - .toEqual(['flatremix', 'light']); + const res2 = await p2.goto(`/?token=${TOKEN}`); + expect(await res2.text()).not.toContain('data-theme=flatremix'); await other.close(); }); -test('a theme this browser kept before themes were on the account goes up to it once', async ({ browser }) => { - // A new account, made by the proxy header on first sight, so it has no theme of its own yet. +test('a theme this browser kept in localStorage becomes its cookie once', async ({ browser }) => { const who = `theme-${Date.now()}@example.com`; const ctx = await browser.newContext({ extraHTTPHeaders: { 'X-Test-User': who } }); // From before light and dark: ipx.theme alone, 'light' meaning Modern, light. await ctx.addInitScript(() => { localStorage.setItem('ipx.theme', 'light'); localStorage.removeItem('ipx.mode'); }); const page = await ctx.newPage(); - const saved = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH'); await page.goto('/'); expect(await page.evaluate(() => [document.documentElement.dataset.theme, document.documentElement.dataset.mode])) .toEqual(['modern', 'light']); - expect((await saved).request().postDataJSON()).toEqual({ theme: 'modern', mode: 'light' }); + expect((await ctx.cookies()).find(c => c.name === 'ipx_theme')?.value).toBe('modern.light'); await ctx.close(); - - // Anywhere else now, it comes from the account. - const fresh = await browser.newContext({ extraHTTPHeaders: { 'X-Test-User': who } }); - const p2 = await fresh.newPage(); - const res = await p2.goto('/'); - expect(await res.text()).toContain('data-theme=modern data-choice=light'); - await fresh.close(); }); test('the admin page saves the global schedule', async ({ page }) => { diff --git a/web/src/theme.ts b/web/src/theme.ts index 2f00a57..d56fd0c 100644 --- a/web/src/theme.ts +++ b/web/src/theme.ts @@ -1,7 +1,8 @@ /* ---------------- theme ---------------- */ -// A theme, and for those that come in both, light, dark or Auto, chosen in Settings and kept on -// the account, so it follows you to another browser or computer. The server writes it onto the -// page's tag (data-theme, data-choice) so the page is drawn in it from the start. The +// A theme, and for those that come in both, light, dark or Auto, chosen in Settings and kept in a +// cookie, so each device has its own: Glass on a phone, Dracula on a desktop (issue #69). The +// server reads it and writes it onto the page's tag (data-theme, data-choice) so the page +// is drawn in it from the start. The // page gets data-mode, light or dark, which is all the CSS reads: Auto is worked out here, from // the system, so no palette is written twice. const THEMES: Record = { @@ -25,7 +26,7 @@ const OLD_THEMES: Record = {dark: ['modern', 'dark'], const systemDark = window.matchMedia?.('(prefers-color-scheme: dark)'); const theme = {name: 'modern', mode: 'dark'}; -/// `save` for a choice made in Settings, which goes to the account; not for applying one. +/// `save` for a choice made in Settings, which goes to this browser's cookie; not for applying one. function setTheme(name = theme.name, mode = theme.mode, save = false){ theme.name = THEMES[name] ? name : 'modern'; theme.mode = MODES[mode] ? mode : 'dark'; @@ -43,21 +44,18 @@ function setTheme(name = theme.name, mode = theme.mode, save = false){ if(save) saveTheme(); } -/// One save at a time, each sending the choice as it stands when it goes. Sent as they came, -/// several at once, a quick run through the list could reach the server out of order and -/// leave the account on a theme passed on the way. -let themeSaving = Promise.resolve(); +/// Kept a year, for the whole site: the admin page is drawn in it too. function saveTheme(){ - themeSaving = themeSaving - .then(() => api('/api/me', {method: 'PATCH', body: JSON.stringify({theme: theme.name, mode: theme.mode})})) - .catch(e => toast(`Your theme was not saved: ${e.message}`, true)); + document.cookie = `ipx_theme=${theme.name}.${theme.mode}; Path=/; Max-Age=31536000; SameSite=Lax`; } systemDark?.addEventListener?.('change', () => { if(theme.mode === 'auto') setTheme(); }); (() => { const root = document.documentElement; - if(root.dataset.choice) return setTheme(root.dataset.theme, root.dataset.choice); - // Nothing on the account yet. A theme this browser kept, from before themes were kept on the - // account, goes up to it once, so nobody has to choose again. + // Without a cookie, the server sent the theme the account kept from before; this browser takes + // it as its own, once, so nobody has to choose again. + if(root.dataset.choice) return setTheme(root.dataset.theme, root.dataset.choice, !/(^|; )ipx_theme=/.test(document.cookie)); + // Nothing on the account either. A theme this browser kept in localStorage, from before that, + // becomes its cookie, once. let name: string | null = null, mode: string | null = null; try{ name = localStorage.getItem('ipx.theme'); mode = localStorage.getItem('ipx.mode'); }catch{} if(OLD_THEMES[name]) [name, mode] = OLD_THEMES[name];