Reorganize settings: data-driven UI, two-tier theme, stats popup, ⌘, shortcut#253
Merged
StephaneDelcroix merged 5 commits intomainfrom Mar 3, 2026
Conversation
StephaneDelcroix
changed the title
Owner
|
Yayayayayayayayayayayayayayay |
…⌘, shortcut - Move Statistics section from Settings page to a dedicated StatisticsPopup component, accessible via a chart icon in the sidebar header (next to settings and tutorial icons) - Add VSCode-style category navigation sidebar to Settings page with Connection, CLI, UI, and Developer sections. Clicking a category scrolls to that section; IntersectionObserver highlights the active category on scroll. Hidden during search to avoid clutter. - Add ⌘, (Cmd+comma) / Ctrl+, global keyboard shortcut to open Settings, registered in index.html's keydown listener and wired to MainLayout via a DotNetObjectReference (_navRef) - Add SettingsReorganizationTests covering stats snapshot isolation, FormatDuration, default values, and category structure - Add 4 UI scenarios: Cmd+comma navigation, statistics popup open/close, category nav scrolling, and no-statistics-in-settings verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…I sections - Add SettingDescriptor model with typed descriptors for each setting - Add SettingsRegistry with declarative setting definitions, search, and category filtering - Add SettingEditor.razor generic renderer for Bool, String, Int, Enum, CardEnum, and Action setting types - Refactor CLI and UI sections in Settings.razor to render from registry descriptors instead of hardcoded HTML - Split theme into two-tier: Color Scheme (System/Dark/Light) and Style (PolyPilot/Solarized) - Make search bar full-width and left-aligned (VSCode-style) - Per-setting search filtering via SettingMatchesSearch - Add OnSettingChanged handler for side effects (theme apply, font size) - Remove Save() from registry SetValue lambdas (page handles saving) - Add 21 SettingsRegistryTests covering search, categories, visibility, theme split, font clamping, enum parsing, unique IDs - Add 2 UI scenarios for data-driven rendering and search filtering - All 1687 tests pass (1 pre-existing failure unrelated) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove IsVisible predicate that hid the style picker when theme was System. Both color scheme and style pickers should always be visible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
System is a color scheme (auto light/dark), not a style. Users can now combine System + Solarized to get auto-switching with Solarized palette. - Add UiTheme.SystemSolarized enum value - Add data-theme='system-solarized' CSS with prefers-color-scheme media query using Solarized light/dark variables - Update ApplyTheme, GetThemeLabel, GetThemePreviewClass for new value - Update MainLayout.razor theme switch - Update SettingsRegistry color scheme and style setters to preserve System when switching styles and preserve Solarized when switching color schemes - Add 4 tests for System+Solarized combinations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add @implements IDisposable and dispose _navRef in Dispose() to prevent the DotNetObjectReference<MainLayout> created for the ⌘, keyboard shortcut from leaking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
StephaneDelcroix
force-pushed
the
fix/the-settings-page-is-becoming-a-mixed-ba-20260302-0925
branch
from
4746991 to
d1cf0f1
Compare
StephaneDelcroix
commented
Mar 3, 2026
Collaborator
Author
StephaneDelcroix
left a comment
StephaneDelcroix
left a comment
There was a problem hiding this comment.
✅ Multi-model consensus review (5 models, post-fix re-review)
Previous Finding Status
| Finding | Status |
|---|---|
| 🟡 DotNetObjectReference<MainLayout> leaked | ✅ FIXED -- @implements IDisposable added, _navRef?.Dispose() in Dispose() |
New Findings (non-blocking)
🟡 MODERATE -- SettingsRegistry.cs:130 -- connection.reconnect Action descriptor has null OnAction (2/5 models)
The descriptor exists for search-keyword matching only. The real reconnect is a hardcoded SaveAndApply() button. If the Connection section is ever migrated to data-driven rendering, this would become a no-op button. Consider adding IsVisible = _ => false or wiring OnAction. Non-blocking today.
🟢 MINOR -- SettingEditor.razor -- SettingType.Custom descriptors silently invisible (2/5 models)
Custom descriptors (connection.sharing, developer.autoUpdate) are rendered by hardcoded UI blocks, not SettingEditor. By-design but fragile if refactored.
Verified as Safe
- ✅
SettingsRegistry.All = Build()static init -- uses#ifcompile-time directives, not runtime platform APIs - ✅
UiTheme.SystemSolarizedenum -- appended at ordinal 5, old values unchanged - ✅
settingsCtx = null!--RebuildSettingsContext()called before first render - ✅ Tests pass: 1690/1691 (1 pre-existing failure)
Action: ✅ Approve
DotNetObjectReference leak fixed. Settings refactor is clean. Data-driven SettingsRegistry is a solid foundation.
StephaneDelcroix
deleted the
fix/the-settings-page-is-becoming-a-mixed-ba-20260302-0925
branch
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
2 participants
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
Transforms the Settings page from hardcoded HTML sections into a VSCode-style data-driven settings architecture. Settings are declared as typed descriptors in a central registry and rendered dynamically by a generic editor component.
Also moves Statistics out of Settings into a sidebar popup, and adds ⌘, keyboard shortcut.
Changes
Data-driven settings infrastructure
SettingDescriptor.cs— Model describing a single setting (Id, Label, Description, Category, Type, options, getter/setter delegates, visibility predicate, search keywords)SettingsRegistry.cs— Declarative registry of all ~15 settings withSearch()andForCategory()methods. NoSave()in setters (page handles persistence)SettingEditor.razor— Generic renderer that dynamically renders the right control for eachSettingType: Bool (toggle), String (text input), Int (+/−), Enum (dropdown), CardEnum (visual cards), Action (button), Custom (page-rendered)Settings page refactored
SettingsRegistrydescriptors instead of hardcoded HTMLTwo-tier theme model
The theme is now two independent dimensions:
All 6 combinations work, including System + Solarized (new
UiTheme.SystemSolarizedenum value withprefers-color-schemeCSS media query).Statistics popup
Keyboard shortcut
Test coverage
mode-switch-scenarios.json