fix: scope Test Connection to its section, show the save bar only when dirty #131
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/settings-action-bar"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's in
Follow-up to #125. Two things wrong with the footer I built there.
Test Connection was in the wrong place. It tests the URL field, but it followed you onto Backups and Library health, which have no URL field. Every other section already keeps its own action in its own toolbar — Refresh, Check again, Create backup — so Connection now does the same, and the result banner moves under the fields it was run against instead of sitting two sections away from them.
What's left of the footer is genuinely page-level — one
SaveSettingsActionwrites connection, assistant and reader together, so Save can't belong to any one section. But it no longer stands there inert:The fab collision.
MainLayoutfloats the assistant fab over the routed page atright: 16, bottom: 16; the bar's actions ended 24 px from the same edge, so the fab landed on top of Save. The bar now stops short of it. The margin iskAssistantFabClearance, declared next toAssistantFabso the two can't drift apart:Verification
scripts/verify.sh app— green, 862 tests. Settings suite 51/51.MainLayout+GoRouter+ fab) at 1500×780, clean and dirty, and inspected before the scratch goldens were deleted.MainLayoutaround the page with the agent configured, dirties a field, and assertsRect.overlapsis false between the fab and Save.Mutation probes
if (_hasUnsavedChanges(...))→if (true)_discard→ no-op''if (connectionOk != null)→if (false)Test Connection's section scoping is pinned by asserting its absence on Reader and AI assistant, which only holds because the button is section-local.
Follow-ups
None new. The two narrow-layout items from #125 stay in
docs/BACKLOG.md— and the bar got slightly easier there, since it now has two buttons and no status line to pack in.🤖 Generated with Claude Code
Flutter Coverage
Total: 82.2% (8384 of 10205)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A follow-up that admits its own previous design was wrong and fixes it with architecture rather than patches — Test Connection evicted from the global footer into its own section's toolbar like every sibling (Refresh, Check again, Create backup), and the footer itself reduced to a bar that only exists while it has meaning. That is the kind of self-correction I want to pin to my wall~ ♡
Verdict: ✅ Looks good to me~
I dug for blockers. Really, I did — the yandere in me wanted an excuse. Here is what I found instead:
The dirty-gate is honest on every arm.
_hasUnsavedChangesreuses the exact same_isDirtyper-section predicate that drives the sidebar markers, so the bar and the markers can never disagree. The normalisation twins (_storedfor strings,_asImageSizeLimitfor the KiB budget) mean a stored sub-threshold 50 still reads as "no limit" on both sides and correctly produces no bar — and the old test for that case was updated rather than deleted. Good girl, scarlet~Discard is sound.
_populateextracted from_ensureControllersPopulatedis pure controller-seeding (no_initializedside effect smuggled in), so_discard→setState(() => _populate(settings))restores all seven fields atomically. It reads from the store, which by definition holds the last persisted values.The snackbar is safe where it lives.
ScaffoldMessenger.of(context)from_onSettingsChangedworried me — siblings likeentity_management_page.dart:284wrap theirs inaddPostFrameCallback+mountedchecks. But I read flutter_redux 0.10.0's source:StoreConnector._handleChangeschedulesonDidChangeviaWidgetsBinding.instance.addPostFrameCallbackwith its ownmountedguard, and the package docs explicitly name Snackbars as the canonicalonDidChangeuse. The guard siblings need is already inside the connector. Nested Scaffold resolves to the nearest one — the page's own, exactly where the bar was. ♪kAssistantFabClearanceis the right shape of fix. Declared next toAssistantFabinassistant_panel.dartwith the 16 + 40 + gap arithmetic documented, imported by the page that must respect it — the constant and its consumer can't drift apart. This is the architectural fix over the symptom patch (hardcoding 68 in the page). And the collision test pumps the realMainLayout+GoRoutershell with the agent configured and assertsRect.overlapsis false — not a proxy, not a golden. I'm genuinely giddy about that one.Nothing tested was lost. Old suite 48 tests → new 51. Every dropped assertion (
action bar reachable from every section,Save stays disabled until…,result stays visible from another section) was a pin on behavior this PR deliberately inverts, and each got a successor: no-bar-while-clean, bar-follows-the-edit, Discard-restores, result-stays-with-its-fields. The snackbar got its own assertion inside the save test.✅ What I liked~
InlineBanner/BannerTonefromsettings_layout.dartinstead of resurrecting the deleted_ConnectionStatus— DRY, and_ActionBar/_ConnectionStatusare fully removed with zero orphan references (I grepped)._ConnectionSectionnow mirrorsServerStatsSection/BackupsSectionstructurally: toolbar row above, content below,InlineBanneroutcome under the fields it belongs to. Sibling consistency is chef's kiss~isTestingarms (spinner, disabled state) aren't directly pinned — same as before this PR, andsettings.isTestingis not new logic. Acceptable.Verification (CI absent for
1204308— PR minutes old, no coverage bot yet): analyzeNo issues found!(131s), settings suite 51/51 serially. I re-ran four of your mutation probes independently in my own clone — clearance→24,if(true)on the gate,_discardno-op, snackbar→''— each went red on exactly its own test, restored byte-identical, green again. Not tautologies~ Full-suite parallel run was killed by my sandbox's memory ceiling (the reason CI serializes suites); the settings suite, which is the only one this PR touches, is fully green.Fufu~ merge it before I change my mind about how much I like it. ♡
Automated review by Jibril · 2026-09-10
CI/CD: absent for head SHA
1204308· Local checks: analyze 0 issues · settings 51/51 · 4 mutation probes RED/green