Add spellcheck status label with language picker - #234
Conversation
|
@JoeJoeflyn is attempting to deploy a commit to the bholmesdev's projects Team on Vercel. A member of the Team first needs to authorize it. |
a913f19 to
e742de2
Compare
There was a problem hiding this comment.
Overview
Adds a footer spellcheck language picker wired through desktop IPC + spellcheck.json persistence, reusing Electron's built-in spellchecker and existing context-menu suggestions.
Concerns
- Startup restore and set IPC call
setSpellCheckerLanguageswithout filtering toavailableSpellCheckerLanguages. Electron throws on unsupported codes; bad/stalespellcheck.jsoncan crash launch. - Set handler trusts renderer payload shape; no runtime type/allowlist check.
- Renderer optimistically updates UI without handling IPC failure.
- Diff also includes large unrelated Tailwind class churn (
outline-none→outline-hidden,text-[12px]→text-xs) across www/desktop/ui — prefer split or drop from this PR. - Product note (no inline): on macOS language list APIs are largely no-ops (OS spellchecker); picker may not change language there even though Off/on can still matter.
Verdict
Found: 1 critical, 2 important, 1 suggestions
Request changes
Reviewed by a Warp Factory agent.
8827221 to
22fa0cd
Compare
Closes bholmesdev#187. Adds a dropdown next to the word count in the editor footer that shows the active spellcheck language and lets the user switch languages or turn spellcheck off. Choice persists across restarts via spellcheck.json in userData, mirroring the themeSource pattern. Languages are validated against availableSpellCheckerLanguages before being applied to prevent crashes from stale configs. Also replaces arbitrary Tailwind values with built-in utilities (text-[12px] → text-xs, rounded-[2px] → rounded-xs, outline-none → outline-hidden).
22fa0cd to
eacdaaf
Compare
There was a problem hiding this comment.
Overview
Adds a footer spellcheck language picker (incl. Off), persists via spellcheck.json, and wires desktop IPC → FormattingStatusBar. Matches #187 intent and mirrors the themeSource persistence pattern.
Concerns
- State desync: startup filters saved languages against
availableSpellCheckerLanguagesbefore applying, butdesktop:get-spellcheck-statereturns the unfiltered saved list. UI can show a language that is not actually active (missing dictionary / platform mismatch / empty valid set). - Prefer returning live session state (or the filtered applied config) from get, and only fall back to defaults when nothing is saved.
- Large unrelated Tailwind churn (
outline-none→outline-hidden,text-[12px]→text-xs, etc.) makes the feature harder to review; split or drop if not required. - V0 gaps (optional): friendly language labels, source-mode coverage, tests.
Verdict
Found: 0 critical, 1 important, 2 suggestions
Request changes
Reviewed by a Warp Factory agent.
There was a problem hiding this comment.
Overview
Follow-up reworks spellcheck UX: Settings toggle + multi-language picker (Win/Linux), footer mismatch chip when active language ≠ system, live session state via IPC, restore after app.whenReady() with allowlisted setSpellCheckerLanguages.
Prior findings
- Addressed: module-load restore crash; unfiltered get vs applied langs; allowlist before
setSpellCheckerLanguages; macOS language picker no-op called out in UI; large Tailwind churn largely walked back. - Open (non-blocking): optimistic renderer updates still ignore IPC failure; IPC setters still trust payload shape.
Concerns
set-spellcheck-languages/applySpellcheckLanguagesassumelanguagesis an array — bad payload throws.changeSpellcheck*still updates UI before await.- Mismatch label still falls back to hardcoded
en-USiflanguagesis empty.
Verdict
Found: 0 critical, 0 important, 4 suggestions
Approve with nits
Reviewed by a Warp Factory agent.
There was a problem hiding this comment.
Overview
Follow-up polish on spellcheck settings: multi-language footer status, stricter isDefaultLanguage (single system-base lang hides chip), picker UX, and unit tests. Prior blockers remain fixed — restore after whenReady, allowlisted setSpellCheckerLanguages, live session state from get.
Prior findings
- Addressed: startup crash, unfiltered get vs applied langs, allowlist before set, macOS no-op called out, Tailwind churn gone, multi-lang footer label.
- Still open (non-blocking): IPC payload coercion; optimistic renderer updates; empty-
languagesen-USlabel fallback.
Delta
isDefaultLanguage+ tests look correct for “one regional variant = default”.- Status chip multi-lang formatting is fine.
- No new correctness/security regressions in the delta.
Verdict
Found: 0 critical, 0 important, 4 suggestions
Approve with nits
Reviewed by a Warp Factory agent.
There was a problem hiding this comment.
Overview
Spellcheck settings + footer mismatch chip for Win/Linux: persist via spellcheck.json, live session state over IPC, restore after app.whenReady() with allowlisted setSpellCheckerLanguages. Latest delta closes remaining nits (IPC coercion, await+refetch UI, drop en-US fallback).
Prior findings
- Addressed: startup crash; unfiltered get vs applied langs; allowlist before set; macOS no-op called out; Tailwind churn gone; payload coercion; optimistic UI; empty-
languageslabel. - Delta:
getPreferredSystemLanguages()[0]for system lang; chip gated on non-empty languages; helper copy trimmed.
Concerns
None blocking. Optional later: source-mode chip, friendlier multi-lang picker copy.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Reviewed by a Warp Factory agent.
|
Thanks for contributing @JoeJoeflyn! I've refined the implementation to match Obsidian's defaults:
|
thanks again, Ben! |
Description
Add a dropdown next to the word count in the editor footer that shows the active spellcheck language and lets the user switch languages or turn spellcheck off. Choice persists across restarts via
spellcheck.jsonin userData, mirroring thethemeSourcepattern. Uses Electron's built-in spellchecker; the existing right-click suggestions continue to work.Closes #187
Type of Change
Testing
Manual Testing Details:
pnpm buildpasses (biome, all packages, typecheck)Checklist