Replace fancier-settings with a TypeScript settings engine#283
Merged
bartekplus merged 13 commits intomasterfrom Mar 8, 2026
Merged
Replace fancier-settings with a TypeScript settings engine#283bartekplus merged 13 commits intomasterfrom
bartekplus merged 13 commits intomasterfrom
Conversation
Promotes the vendored fancier-settings JS library to first-party TypeScript. Splits into a generic framework (src/ui/settings-engine/) and app-specific code (src/ui/options/), then deletes the old src/third_party/fancier-settings/ directory entirely. Key changes: - New generic framework: store, i18n, controls, layout, SettingsEngine - Store and storage backends moved to src/core/application/storage/ to respect architecture boundaries - UX improvements: tab fade-in, focus rings, grammar rule keyboard nav, modal focus trap, inline "Saved ✓" feedback, color hex label - All 17 e2e smoke tests and 1187 unit tests pass Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add tests/settingsEngine.controls.test.ts with 37 unit tests covering all control types (CheckboxControl, SliderControl, TextControl, TextareaControl, SelectControl, RadioControl, ButtonControl, DescriptionControl, ValueOnlyControl, BaseControl.destroy) - Fix SliderControl tooltip to use textContent instead of innerText so it works correctly in JSDOM and detached DOM environments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… cleanup - SliderControl: compute left% from value/min/max to position tooltip above the thumb instead of staying fixed at the left edge - RuleToggleCardsControl: set aria-checked on initial card creation and sync it in updateStateUI() so screen readers see the correct state - ListBoxControl: collapse store_persist/storeWithFeedback/_storeInternal chain into direct this.store.set() calls Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SettingsEngine: extract instantiateControl() to eliminate ~80 lines of duplicated switch statement between createControl and modal nesting - FieldControl: remove dead _clearHandle assignment and redundant null check in showSavedIndicator's setTimeout callback - Controls (Checkbox, Slider, Select, Textarea, ListBoxMultiSelect, RuleToggleCards): remove redundant name-undefined guards before persistToStorage() — the base class already handles this - TextControl: collapse duplicated if/else (input.type set in both branches) to a single assignment + ternary for className; remove redundant name check before params.store check - SliderControl: extract formatValue() private method to eliminate duplicated displayModifier ternary; convert updateDisplay closure to private method for consistency - RuleToggleCardsControl: replace nested ternary for rawOptions with if/else chain per project conventions - LocalStorageBackend: convert verbose Promise constructor + try/catch to plain async methods; use startsWith() over substring comparison Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Root cause: tab: field in manifest used i18n.get("advanced_tab") (the
translated label) as both the tab key and the hash anchor, while popup
deep-links always use the stable key "advanced_tab". Comparing
location.hash against a translated string never matched.
Fix:
- types.ts: add TabConfig { id, label } + tabs[] to ManifestDefinition
- settingsManifest.ts: change all tab: i18n.get("xxx") to tab: "xxx"
(62 entries); add explicit tabs[] array mapping id to translated label
- SettingsEngine: build tabLabelMap from manifest.tabs before iterating
settings; key tabs by stable id; set tabA.href="#id" and call
history.replaceState on click; defer hash activation until after all
tabs are created; add hashchange listener for back/forward navigation
- index.ts: export TabConfig from public barrel
- ruleToggleCards.test.ts: update assertion to stable key "grammar_tab"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bartekplus
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fancier-settingsimplementation with the in-repo TypeScriptsettings-engine, including the settings page wiring, manifest, i18n, and styles@core/application/storageand route settings consumers to that single storage ownerTesting