Settings UI restructure + DatabaseToolsModal extraction with per-row upgrade progress and modal-aware banner cycle#553
Merged
Conversation
Reorder DatabaseEntryRow DOM so the trash button is the LAST child of .db-entry-row (was first child). Trash stays visually pinned to the left edge via CSS position:absolute; left:0 -- zero muscle-memory regression. The reorder fixes keyboard tab order so Tab traversal hits name button, toggle/upgrade action, then trash in the natural left-to-right order. Painting-order correctness requires z-index:1 on .db-entry-row-content: both elements at z-index:auto would fall into CSS 2.1 painting group 6 where tree order wins (trash later in DOM would obscure content at rest). z-index:1 promotes content to group 7 above trash's group 6. Background fill remains required to cover the trash strip when content is opaque. Add pending-toggle visual indicator: a 1px box-shadow ring around the toggle (control-anchored, survives slide-reveal animation) plus an SR aria-label suffix "(pending toggle, unsaved)". Indicator suppressed on ActionKind.DisabledToggle so disabled controls don't get a misleading announcement. Selectors use ::deep to pierce the BooleanSelect component CSS isolation boundary. Bump row padding from .15rem to .25rem .5rem .25rem 0 for breathing room. Tests: UI 191 (+4 new), Runtime 1003 unchanged. Pre-impl 4-slot panel unanimous READY at R3. Single-agent gpt-5.5 post-impl review READY after the ::deep fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… centralized all aria parameters into base class
…age tab with Save/Apply pattern, snapshot-based reload detection, ClassificationFailed retry, and BackupExists restore
jschick04
force-pushed
the
jschick/modal-coordinator-pr9
branch
from
c91e725 to
142b20f
Compare
This was referenced May 29, 2026
…yRow click lambdas, and routed AttentionBanner action to Databases modal
…l reloads open log views
…modal-wide IsUpgradeBlocked to the recovery row
… clipping, and layered inline alert overlay above sticky save strip
…oordinated open/close swap to eliminate flicker
…eProgress cancel faults, and awaited OnRemove EventCallback
…rt-triggered upgrades block restore
…of clearing the entire set
… per-file removal failures
…onfirmation, and trimmed misleading test name
NikTilton
approved these changes
May 29, 2026
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
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.
Restructures Settings, extracts database management into
DatabaseToolsModal, surfaces per-row upgrade progress, reworks the banner cycle to remain usable while a modal is open, and redesigns the Remove flow with multi-select + cancel-then-remove.Closes #525.
Closes #526.
Commit summary (~17 commits)
Foundation (pre-split):
IAnnouncementService+AnnouncerHostfor SR completion announcementsDatabaseToolsModalManage tab with Save/Apply pattern, snapshot-based reload detection, ClassificationFailed retry, BackupExists restoreCommit A (polish):
AttentionBanneraction:Open Settings→Open Databases, flippedIMenuActionService.OpenDatabaseToolsAsync()fromTask→Task<bool>:focus-visiblerings no longer clippedCommit B (per-row upgrade progress, partial #525):
ManageDatabasesUpgradeProgressBanner; surface upgrade progress (UpgradePhase verb + batch position + Cancel) per row inDatabaseEntryRowCommit C (closes #526 Bug 2): Suppress
AttentionBannerwhileDatabaseToolsModalis the active modalCommit D (closes #526 Bug 1): Lift banner cycle into
ModalChromevia singletonIBannerCycleStateServicewith coordinated open/close swap to eliminate flickerCommit E (fully closes #525): Permanent redesign of Remove flow — always-visible per-row Remove + multi-select via
<div role=\"checkbox\">+ bottom bulk strip + inline confirmation modal with cancel-then-remove during in-flight upgradeCommit F: Extracted reusable
<Checkbox>component (visually-hidden native input pattern matchingToggle) + removed pending blue box-shadowRound 1 + 2 + 3 review fixes (in response to Copilot bot findings):
IAnnouncementServiceInvokeAsyncin 5 Razor click lambdasIsRestoreBlockeddual-signal guard (Coordinator + per-row UpgradeProgress)BannerCycleStateServicethread-safety viaLock _stateLock(UI clicks vs background upgrade thread)OnCancelClickfault isolation (try/catch +TraceLogger.Warning)OnRemoveClickawaitsOnRemove.InvokeAsync()Clear())TraceLogger+ announcementoutcome.Removed(notConfirmed) for success accountingIsUpgradeBlockedpassed unmodified (no per-row narrowing)History
Originally split into 4 stacked PRs (#553, #554, #555, #556) to fit the Copilot reviewer's diff-size limit. Each PR underwent 4-slot pre-impl + post-impl panels (Opus 4.8, GPT-5.5, GPT-5.4, Sonnet 4.6) before re-collapse. All bot findings addressed across 3 review rounds.
Tests