From d6b7f7dfb64026e9ce5c766c3dc669f2d4093288 Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Sat, 5 Sep 2026 19:51:17 +0100 Subject: [PATCH] Make imported/linked CSS always a full theme replacement (ticket 020) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the "Use as theme" checkbox/is_theme distinction from NavMenu.tsx's CSS import and link forms — every imported/linked stylesheet is now always validated against the canonical --c-* variable set and becomes a selectable theme, same as today's is_theme:true path. Drops is_theme entirely from the backend (CssEntry, ImportCssRequest, LinkCssRequest, CssEntryDto) and from the frontend CssEntry type; an existing settings.json with a stored is_theme:false entry still deserializes fine (the field is simply unknown/ignored) and that entry becomes selectable like any other, so no migration step is needed. Also drops the now-dead always-on-overlay CSS injection path from useSettingsPanel.ts, and changes the CSS filename placeholder from "name.css" to "enter name". Implemented directly against main rather than merging the dispatched worktree branch for this ticket, since that branch's base predated ticket 012 (the custom-themes/imported-CSS consolidation) and its NavMenu structure no longer matched current main. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o --- src/api.rs | 10 +--- src/dto.rs | 14 ++--- src/settings.rs | 42 ++++--------- tickets/020-css-themes-full-replacement.md | 13 ---- webapp/src/components/NavMenu.tsx | 69 +++++++--------------- webapp/src/lib/api.ts | 10 ++-- webapp/src/lib/useSettingsPanel.ts | 39 +++--------- 7 files changed, 52 insertions(+), 145 deletions(-) delete mode 100644 tickets/020-css-themes-full-replacement.md diff --git a/src/api.rs b/src/api.rs index 2656ca9..3e4f280 100644 --- a/src/api.rs +++ b/src/api.rs @@ -392,13 +392,7 @@ async fn import_css( ) -> impl IntoResponse { let css_dir = state.css_dir(); let mut settings = state.settings.lock().unwrap(); - match settings.import_css( - &state.settings_path, - &css_dir, - body.filename, - &body.content, - body.is_theme, - ) { + match settings.import_css(&state.settings_path, &css_dir, body.filename, &body.content) { Ok(id) => { let entry = settings .css_entries @@ -417,7 +411,7 @@ async fn link_css( Json(body): Json, ) -> impl IntoResponse { let mut settings = state.settings.lock().unwrap(); - match settings.link_css(&state.settings_path, body.path_or_url.clone(), body.is_theme) { + match settings.link_css(&state.settings_path, body.path_or_url.clone()) { Ok(()) => { let entry = settings .css_entries diff --git a/src/dto.rs b/src/dto.rs index 478d169..d42be87 100644 --- a/src/dto.rs +++ b/src/dto.rs @@ -122,21 +122,18 @@ pub struct ErrorDto { pub error: String, } -/// `is_theme` marks this as a selectable colour-scheme theme rather than arbitrary CSS; defaults to false. +/// Every imported CSS entry is a selectable theme (ticket 020) — there's no separate +/// always-on-overlay mode to opt into. #[derive(Debug, Deserialize)] pub struct ImportCssRequest { pub filename: String, pub content: String, - #[serde(default)] - pub is_theme: bool, } /// A reference to an external CSS path/URL that ottyl doesn't copy or own. #[derive(Debug, Deserialize)] pub struct LinkCssRequest { pub path_or_url: String, - #[serde(default)] - pub is_theme: bool, } /// `id` is the real id for `Imported` entries or a stable id derived from the path/URL for `Linked` ones, so the frontend can key both kinds uniformly (only `Imported` entries are deletable/content-fetchable). @@ -146,7 +143,6 @@ pub struct CssEntryDto { pub kind: CssEntryKind, pub filename: Option, pub path_or_url: Option, - pub is_theme: bool, } #[derive(Debug, Clone, Copy, Serialize)] @@ -159,19 +155,17 @@ pub enum CssEntryKind { impl From<&CssEntry> for CssEntryDto { fn from(entry: &CssEntry) -> Self { match entry { - CssEntry::Imported { id, filename, is_theme } => CssEntryDto { + CssEntry::Imported { id, filename } => CssEntryDto { id: id.clone(), kind: CssEntryKind::Imported, filename: Some(filename.clone()), path_or_url: None, - is_theme: *is_theme, }, - CssEntry::Linked { path_or_url, is_theme } => CssEntryDto { + CssEntry::Linked { path_or_url } => CssEntryDto { id: crate::settings::css_entry_id(entry), kind: CssEntryKind::Linked, filename: None, path_or_url: Some(path_or_url.clone()), - is_theme: *is_theme, }, } } diff --git a/src/settings.rs b/src/settings.rs index e492379..7e1eee9 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -39,21 +39,16 @@ pub enum ColorScheme { System, } -/// A single CSS entry: content ottyl stores itself ("imported") or a reference to an external path/URL ("linked", not deletable via the API). `is_theme` marks it as a selectable colour-scheme theme rather than always-injected CSS. +/// A single CSS entry: content ottyl stores itself ("imported") or a reference to an external +/// path/URL ("linked", not deletable via the API). Every entry is always a selectable +/// colour-scheme theme (ticket 020 removed the "use as theme"/always-on-overlay distinction); +/// an entry persisted before that change with a stored `is_theme: false` still loads fine (the +/// field is simply ignored) and becomes selectable like any other entry. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "lowercase")] pub enum CssEntry { - Imported { - id: String, - filename: String, - #[serde(default)] - is_theme: bool, - }, - Linked { - path_or_url: String, - #[serde(default)] - is_theme: bool, - }, + Imported { id: String, filename: String }, + Linked { path_or_url: String }, } /// Tracks known profile names and which one is active; the profile files themselves live elsewhere. @@ -228,7 +223,6 @@ impl Settings { css_dir: &Path, filename: String, content: &str, - is_theme: bool, ) -> Result { fs::create_dir_all(css_dir)?; let id = generate_imported_css_id(&filename, content); @@ -236,23 +230,14 @@ impl Settings { self.css_entries.push(CssEntry::Imported { id: id.clone(), filename, - is_theme, }); self.save(settings_path)?; Ok(id) } /// Registers a reference to an external CSS path/URL without copying its content, and persists settings. - pub fn link_css( - &mut self, - settings_path: &Path, - path_or_url: String, - is_theme: bool, - ) -> Result<(), CssError> { - self.css_entries.push(CssEntry::Linked { - path_or_url, - is_theme, - }); + pub fn link_css(&mut self, settings_path: &Path, path_or_url: String) -> Result<(), CssError> { + self.css_entries.push(CssEntry::Linked { path_or_url }); self.save(settings_path)?; Ok(()) } @@ -340,11 +325,9 @@ mod tests { settings.css_entries.push(CssEntry::Imported { id: "abc123".to_string(), filename: "theme.css".to_string(), - is_theme: false, }); settings.css_entries.push(CssEntry::Linked { path_or_url: "https://example.com/style.css".to_string(), - is_theme: false, }); settings.profiles.names.push("default".to_string()); settings.profiles.active_profile = Some("default".to_string()); @@ -388,7 +371,6 @@ mod tests { &css_dir, "theme.css".to_string(), "body { color: red; }", - false, ) .unwrap(); @@ -418,7 +400,7 @@ mod tests { let mut settings = Settings::load(&settings_path); let id = settings - .import_css(&settings_path, &css_dir, "a.css".to_string(), "a{}", false) + .import_css(&settings_path, &css_dir, "a.css".to_string(), "a{}") .unwrap(); let css_file = css_dir.join(format!("{id}.css")); assert!(css_file.exists()); @@ -440,7 +422,7 @@ mod tests { let mut settings = Settings::load(&settings_path); settings - .link_css(&settings_path, "https://example.com/style.css".to_string(), false) + .link_css(&settings_path, "https://example.com/style.css".to_string()) .unwrap(); let linked_id = css_entry_id(&settings.css_entries[0]); @@ -469,7 +451,7 @@ mod tests { let mut settings = Settings::load(&settings_path); settings - .link_css(&settings_path, "https://example.com/style.css".to_string(), false) + .link_css(&settings_path, "https://example.com/style.css".to_string()) .unwrap(); let linked_id = css_entry_id(&settings.css_entries[0]); diff --git a/tickets/020-css-themes-full-replacement.md b/tickets/020-css-themes-full-replacement.md deleted file mode 100644 index 3c8b01d..0000000 --- a/tickets/020-css-themes-full-replacement.md +++ /dev/null @@ -1,13 +0,0 @@ -# Make imported/linked CSS always a full theme replacement - -From WISHLIST.md: colour schemes should be full CSS replacements — there should be no "use as theme" tickbox distinguishing a selectable alternative theme from an always-on overlay. Every imported/linked stylesheet should just work the same way as the built-in colour schemes: a selectable, full replacement. - -Currently `NavMenu.tsx`'s CSS section has an `is_theme`/"Use as theme" checkbox (`cssIsTheme`/`cssLinkIsTheme` state, `panel.importCssEntry(filename, content, isTheme)` / `panel.linkCssEntry(url, isTheme)`) that decides whether an entry becomes a selectable theme (validated against the canonical `--c-*` variable set, toggled via `panel.setCustomTheme`) or an always-on CSS overlay applied on top of the active colour scheme. This ticket removes that distinction: every imported/linked stylesheet is validated as a theme and becomes a selectable alternative to the built-in colour schemes, same as today's `is_theme: true` path. - -## Acceptance criteria -- The "Use as theme" checkboxes are removed from both the CSS import form and the CSS link form in `NavMenu.tsx`. -- Imported/linked CSS is always validated against the canonical `--c-*` theme variable set (`parseThemeCss`) and always becomes a selectable theme entry (today's `is_theme: true` behavior) — the "always-on overlay" mode is removed. -- The explanatory text "imported/linked stylesheets — check 'Use as theme' to make one a selectable alternative to the built-in colour schemes above instead of an always-on overlay" is deleted/rewritten to reflect the simpler model (every entry is a selectable theme). -- The CSS filename input's placeholder changes from `"name.css"` to `"enter name"`. -- Backend/`CssEntry`/`is_theme` plumbing (`src/settings.rs`, `src/api.rs`, `src/dto.rs`, `webapp/src/lib/api.ts`) is simplified accordingly if `is_theme` becomes always-true and dead as a variable — remove it if no longer meaningful, or confirm it's still needed for backward compatibility with existing saved entries. -- Existing saved CSS entries (including any persisted with `is_theme: false`) keep working — decide and document how they're treated (e.g. migrated to always selectable) rather than silently breaking. diff --git a/webapp/src/components/NavMenu.tsx b/webapp/src/components/NavMenu.tsx index f22f9e6..e9f3671 100644 --- a/webapp/src/components/NavMenu.tsx +++ b/webapp/src/components/NavMenu.tsx @@ -187,10 +187,8 @@ function SettingsPanel({ panel }: { panel: ReturnType } const [maxLogsInput, setMaxLogsInput] = useState('') const [cssFilename, setCssFilename] = useState('') const [cssContent, setCssContent] = useState('') - const [cssIsTheme, setCssIsTheme] = useState(false) const [cssUploadError, setCssUploadError] = useState(null) const [cssLinkUrl, setCssLinkUrl] = useState('') - const [cssLinkIsTheme, setCssLinkIsTheme] = useState(false) const [ntfyTopicInput, setNtfyTopicInput] = useState('') const [ntfyServerInput, setNtfyServerInput] = useState('') const [ntfyTestError, setNtfyTestError] = useState(null) @@ -254,33 +252,27 @@ function SettingsPanel({ panel }: { panel: ReturnType } setCssUploadError(null) } - // Single import path for both "configured/imported CSS" and "custom - // themes" (ticket 012) — `cssIsTheme` is the only thing that distinguishes - // them, so a theme upload is only additionally validated against the - // canonical `--c-*` variable set before it's sent. + // Every imported/linked stylesheet is always a selectable theme (ticket 020), so it's always + // validated against the canonical `--c-*` variable set before it's sent. async function handleCssImport(e: React.FormEvent) { e.preventDefault() setCssUploadError(null) if (!cssFilename.trim() || !cssContent.trim()) return - if (cssIsTheme) { - const { missing } = parseThemeCss(cssContent) - if (missing.length > 0) { - setCssUploadError(`Missing variable${missing.length > 1 ? 's' : ''}: ${missing.join(', ')}`) - return - } + const { missing } = parseThemeCss(cssContent) + if (missing.length > 0) { + setCssUploadError(`Missing variable${missing.length > 1 ? 's' : ''}: ${missing.join(', ')}`) + return } - await panel.importCssEntry(cssFilename.trim(), cssContent, cssIsTheme) + await panel.importCssEntry(cssFilename.trim(), cssContent) setCssFilename('') setCssContent('') - setCssIsTheme(false) } async function handleCssLink(e: React.FormEvent) { e.preventDefault() if (!cssLinkUrl.trim()) return - await panel.linkCssEntry(cssLinkUrl.trim(), cssLinkIsTheme) + await panel.linkCssEntry(cssLinkUrl.trim()) setCssLinkUrl('') - setCssLinkIsTheme(false) } function downloadBuiltinTheme(name: string) { @@ -391,8 +383,8 @@ function SettingsPanel({ panel }: { panel: ReturnType }

CSS

- imported/linked stylesheets — check "Use as theme" to make one a selectable alternative to the built-in - colour schemes above instead of an always-on overlay + imported/linked stylesheets — each one becomes a selectable alternative to the built-in colour schemes + above
{panel.cssEntries.length === 0 ? ( @@ -405,23 +397,16 @@ function SettingsPanel({ panel }: { panel: ReturnType } className="flex items-center justify-between rounded border border-border px-2 py-1 text-xs" >
- {entry.is_theme ? ( - - ) : ( -
{entry.filename ?? entry.path_or_url}
- )} -
- {entry.kind} - {entry.is_theme ? ' · theme' : ''} -
+ +
{entry.kind}