Browse Source

RHL-038 fix(quicknav): optimize branch fetching logic for admin/dev users and ensure valid selection

Code_Uwe 2 weeks ago
parent
commit
7acd3ce7ce
2 changed files with 48 additions and 33 deletions
  1. 25 27
      components/app-shell/AppShell.jsx
  2. 23 6
      components/app-shell/QuickNav.jsx

+ 25 - 27
components/app-shell/AppShell.jsx

@@ -2,39 +2,37 @@ import React from "react";
 import TopNav from "@/components/app-shell/TopNav";
 import SidebarPlaceholder from "@/components/app-shell/SidebarPlaceholder";
 
-/**
- * AppShell
- *
- * Purpose:
- * - Provide a stable frame for all protected pages:
- *   - Top navigation (always visible)
- *   - Optional sidebar area (desktop)
- *   - Main content area (route pages render here)
- *
- * Layout notes:
- * - The outer wrapper must be `flex flex-col` so the content region can use `flex-1`
- *   and fill the remaining viewport height below the TopNav.
- *
- * Test/runtime note:
- * - Our Vitest/Vite setup currently uses the "classic" JSX runtime in unit tests,
- *   which requires React to be in scope. Importing React here ensures tests work
- *   without introducing additional build tooling configuration.
- */
 export default function AppShell({ children }) {
 	return (
 		<div className="min-h-screen flex flex-col">
 			<TopNav />
 
-			{/* Content area below the top navigation */}
-			<div className="mx-auto flex w-full max-w-7xl flex-1 gap-4 px-4 py-4">
-				{/* Sidebar is reserved space for navigation/filter UI.
-            We hide it on small screens for now (responsive baseline). */}
-				<aside className="hidden w-64 shrink-0 md:block">
-					<SidebarPlaceholder />
-				</aside>
+			{/* 
+				Layout strategy (2xl+):
+				- Center column is exactly 50% width.
+				- Left/right gutters are flexible.
+				- Sidebar is placed in the left gutter and aligned to the right edge,
+				  so it “docks” to the centered content without consuming its width.
 
-				{/* Main content: all route pages render here */}
-				<main className="min-w-0 flex-1">{children}</main>
+				Below 2xl:
+				- Keep the app wide (single-column flow).
+				- Sidebar is hidden (it would otherwise reduce main content width).
+			*/}
+			<div className="flex-1 px-4 py-4">
+				<div className="mx-auto grid w-full gap-4 2xl:grid-cols-[1fr_minmax(0,50%)_1fr]">
+					<aside className="hidden 2xl:col-start-1 2xl:block 2xl:justify-self-end">
+						{/* 
+							Sidebar width policy:
+							- Fixed width keeps it stable and prevents “percentage jitter”.
+							- Adjust these widths if you want a bigger/smaller left rail.
+						*/}
+						<div className="w-96">
+							<SidebarPlaceholder />
+						</div>
+					</aside>
+
+					<main className="min-w-0 2xl:col-start-2">{children}</main>
+				</div>
 			</div>
 		</div>
 	);

+ 23 - 6
components/app-shell/QuickNav.jsx

@@ -66,6 +66,9 @@ export default function QuickNav() {
 	}, [isAuthenticated, isBranchUser, user?.branchId]);
 
 	React.useEffect(() => {
+		// B.4 fix:
+		// - Fetch the branch list once for admin/dev users (or when the user changes),
+		//   not on every selectedBranch change.
 		if (!isAdminDev) return;
 
 		let cancelled = false;
@@ -79,11 +82,6 @@ export default function QuickNav() {
 
 				const branches = Array.isArray(res?.branches) ? res.branches : [];
 				setBranchList({ status: BRANCH_LIST_STATE.READY, branches });
-
-				if (!selectedBranch && branches.length > 0) {
-					setSelectedBranch(branches[0]);
-					safeWriteLocalStorageBranch(STORAGE_KEY_LAST_BRANCH, branches[0]);
-				}
 			} catch (err) {
 				if (cancelled) return;
 
@@ -95,7 +93,26 @@ export default function QuickNav() {
 		return () => {
 			cancelled = true;
 		};
-	}, [isAdminDev, selectedBranch]);
+	}, [isAdminDev, user?.userId]);
+
+	React.useEffect(() => {
+		// After we have the branch list, ensure selectedBranch is valid and known.
+		// This effect does NOT trigger any refetches (only local state).
+		if (!isAdminDev) return;
+		if (branchList.status !== BRANCH_LIST_STATE.READY) return;
+
+		const branches = Array.isArray(branchList.branches)
+			? branchList.branches
+			: [];
+		if (branches.length === 0) return;
+
+		// If no selection yet (or selection is no longer in the list), choose a stable default.
+		if (!selectedBranch || !branches.includes(selectedBranch)) {
+			const next = branches[0];
+			setSelectedBranch(next);
+			safeWriteLocalStorageBranch(STORAGE_KEY_LAST_BRANCH, next);
+		}
+	}, [isAdminDev, branchList.status, branchList.branches, selectedBranch]);
 
 	React.useEffect(() => {
 		if (!isAdminDev) return;