Merge Settings Mirror + Push mirror into one entry (Fix #627) #628

Merged

crueber merged 1 commit from fix/issue-627 into main

2026-09-16 18:06:42 +00:00

crueber commented

2026-09-16 18:04:24 +00:00

Owner

Merges the Settings sidebar Mirror + Push mirror entries into ONE entry rendering TWO containers.

settingsNav.js: drops the pushmirror row; resolveSettingsTab("pushmirror") returns "mirror" (explicit alias so old #pushmirror deep links land on the merged entry instead of falling back to General).
Settings.jsx: the mirror branch mounts MirrorTab (pull) then PushMirrorTab (push), old sidebar order, both byte-identical; pushmirror Show branch deleted.
Sweep: no other pushmirror-as-tab references remain (SDK/API/data keys untouched — pushmirror.* surface and mirror-push badge unchanged).
Docs: FIXED (Forgejo #627) amendment in docs/go/12_web_ui.md, same commit (law 12).

Verification: targeted suites 34/34 pass; full-minus-smoke 1619/1619 pass (smoke needs live server, pre-existing); vite build + esbuild SDK green, web/dist/.keep restored; go vet ./internal/... clean. Regroup only — same containers, both themes + 390px hold by construction.

Merges the Settings sidebar Mirror + Push mirror entries into ONE entry rendering TWO containers. - settingsNav.js: drops the pushmirror row; resolveSettingsTab("pushmirror") returns "mirror" (explicit alias so old #pushmirror deep links land on the merged entry instead of falling back to General). - Settings.jsx: the mirror branch mounts MirrorTab (pull) then PushMirrorTab (push), old sidebar order, both byte-identical; pushmirror Show branch deleted. - Sweep: no other pushmirror-as-tab references remain (SDK/API/data keys untouched — pushmirror.* surface and mirror-push badge unchanged). - Docs: FIXED (Forgejo #627) amendment in docs/go/12_web_ui.md, same commit (law 12). Verification: targeted suites 34/34 pass; full-minus-smoke 1619/1619 pass (smoke needs live server, pre-existing); vite build + esbuild SDK green, web/dist/.keep restored; go vet ./internal/... clean. Regroup only — same containers, both themes + 390px hold by construction.

crueber added 1 commit

2026-09-16 18:04:25 +00:00

Merge Settings Mirror + Push mirror into one sidebar entry (Fix #627 ) e7d1ac3a4a

web/src/lib/settingsNav.js drops the pushmirror row (single Mirror entry);
resolveSettingsTab("pushmirror") returns "mirror" as an explicit alias so
old #pushmirror deep links land on the merged entry. Settings.jsx mirror
branch mounts MirrorTab then PushMirrorTab (old sidebar order), pushmirror
branch deleted. Tests: settings-nav extended + new
settings-mirror-merge-627 (registry + render-structure + alias + law-12) +
pushmirror nav pin updated. Docs: 12_web_ui.md FIXED (Forgejo #627).

crueber commented

2026-09-16 18:06:15 +00:00

Author

Owner

Independent review — APPROVED (no fix commit; no defects found).

Acceptance (issue #627) — all met:

One Mirror nav row: SETTINGS_GROUP (settingsNav.js:17-27) holds exactly one {id:"mirror"} entry at index 2 (old Mirror slot, old sidebar order); pushmirror row deleted. Sidebar renders via (Settings.jsx:1846) so one row is structural, not cosmetic.
Both containers unchanged: mirror branch (Settings.jsx:1877) mounts — props byte-identical to the two pre-change call sites (main:1877-1878). Pull-then-push order preserves old sidebar order. PushMirrorTab/MirrorTab bodies untouched (only the branch line changed).
Deep links: resolveSettingsTab("pushmirror") returns "mirror" via explicit if (id === "pushmirror") (settingsNav.js:48) — strict equality, not fuzzy. settingsTabIdFromHash delegates to resolve (line 59), so #pushmirror and #mirror both land on mirror. Verified live: resolve('pushmirror')=mirror, hash('#pushmirror')=mirror, resolve('pushmirrors')=null, resolve('bogus')=null → General fallback intact.
Registry tests pin it: settings-nav.test.js (single-entry + alias + near-miss rejection incl. pushmirrors/mirrorpush/Mirror) and new settings-mirror-merge-627.test.js (single entry, explicit-alias source pin, render-order pin, no-pushmirror-branch pin, law-12 pin).

Checklist:

Alias explicitness: yes — 'id === "pushmirror"' strict check; unknown ids still return null (pre-existing fallback tests untouched, all passing).
No orphaned code: pushmirror branch deleted; remaining 'pushmirror' refs are legitimate (useData keys, lib/pushmirror imports, repo.pushmirror.* SDK calls, alias + comments). No dead tab-branch code.
No fetch regression: useData keys unchanged — 'mirror:${full}' (572) and 'pushmirror:${full}' (796); both tabs mount under one branch so both fetches still fire.
pushmirror.test.js pin update justified: old assertion (separate sidebar entry) contradicted the merge; new assertion (no entry + alias lands on mirror) matches acceptance. Rest of file untouched.
settings-nav fallback tests intact: all 8 pre-existing tests preserved verbatim; 2 new #627 tests appended.
Law 12: FIXED (Forgejo #627) amendment in docs/go/12_web_ui.md, same commit.
Law 1: no new deps/CSS — changed files are 2 src + 3 test + 1 doc only; package.json/lock/ui.css untouched.
Fail pre-fix: confirmed main carries both rows (mirror:15 + pushmirror:16 in main's settingsNav.js), so the new single-entry/no-pushmirror pins fail on main by construction.

Tests: targeted 22/22 pass (settings-nav + merge-627 + pushmirror). Full unit glob 1621/1622 — sole failure is smoke.test.js 'built SPA shell served at / and /setup' (/setup 403 vs 200), untouched by this PR (last touched in D-WEB-6) and disclosed in the PR body as pre-existing live-server-dependent. No rework needed.

Independent review — APPROVED (no fix commit; no defects found). Acceptance (issue #627) — all met: - One Mirror nav row: SETTINGS_GROUP (settingsNav.js:17-27) holds exactly one {id:"mirror"} entry at index 2 (old Mirror slot, old sidebar order); pushmirror row deleted. Sidebar renders via <For each={SETTINGS_GROUP}> (Settings.jsx:1846) so one row is structural, not cosmetic. - Both containers unchanged: mirror <Show> branch (Settings.jsx:1877) mounts <MirrorTab ctx={ctx} repo={repo}/><PushMirrorTab ctx={ctx} repo={repo}/> — props byte-identical to the two pre-change call sites (main:1877-1878). Pull-then-push order preserves old sidebar order. PushMirrorTab/MirrorTab bodies untouched (only the branch line changed). - Deep links: resolveSettingsTab("pushmirror") returns "mirror" via explicit if (id === "pushmirror") (settingsNav.js:48) — strict equality, not fuzzy. settingsTabIdFromHash delegates to resolve (line 59), so #pushmirror and #mirror both land on mirror. Verified live: resolve('pushmirror')=mirror, hash('#pushmirror')=mirror, resolve('pushmirrors')=null, resolve('bogus')=null → General fallback intact. - Registry tests pin it: settings-nav.test.js (single-entry + alias + near-miss rejection incl. pushmirrors/mirrorpush/Mirror) and new settings-mirror-merge-627.test.js (single entry, explicit-alias source pin, render-order pin, no-pushmirror-branch pin, law-12 pin). Checklist: - Alias explicitness: yes — 'id === "pushmirror"' strict check; unknown ids still return null (pre-existing fallback tests untouched, all passing). - No orphaned code: pushmirror <Show> branch deleted; remaining 'pushmirror' refs are legitimate (useData keys, lib/pushmirror imports, repo.pushmirror.* SDK calls, alias + comments). No dead tab-branch code. - No fetch regression: useData keys unchanged — 'mirror:${full}' (572) and 'pushmirror:${full}' (796); both tabs mount under one branch so both fetches still fire. - pushmirror.test.js pin update justified: old assertion (separate sidebar entry) contradicted the merge; new assertion (no entry + alias lands on mirror) matches acceptance. Rest of file untouched. - settings-nav fallback tests intact: all 8 pre-existing tests preserved verbatim; 2 new #627 tests appended. - Law 12: FIXED (Forgejo #627) amendment in docs/go/12_web_ui.md, same commit. - Law 1: no new deps/CSS — changed files are 2 src + 3 test + 1 doc only; package.json/lock/ui.css untouched. - Fail pre-fix: confirmed main carries both rows (mirror:15 + pushmirror:16 in main's settingsNav.js), so the new single-entry/no-pushmirror pins fail on main by construction. Tests: targeted 22/22 pass (settings-nav + merge-627 + pushmirror). Full unit glob 1621/1622 — sole failure is smoke.test.js 'built SPA shell served at / and /setup' (/setup 403 vs 200), untouched by this PR (last touched in D-WEB-6) and disclosed in the PR body as pre-existing live-server-dependent. No rework needed.

crueber merged commit a008d114cb into main

2026-09-16 18:06:42 +00:00

crueber referenced this pull request from a commit

2026-09-16 18:06:43 +00:00

Merge pull request 'Merge Settings Mirror + Push mirror into one entry (Fix #627)' (#628) from fix/issue-627 into main

crueber referenced this pull request

2026-09-16 18:06:43 +00:00

Settings: merge Mirror and Push mirror sidebar entries into one section with two containers #627