Merge pull request #909 from objectstack-ai/copilot/fix-default-navigation-mode

Author: hotlong

ROADMAP.md

@@ -1129,6 +1129,17 @@ The `FlowDesigner` is a canvas-based flow editor that bridges the gap between th
 
 ## 🐛 Bug Fixes
 
+### Default Navigation Mode (Page) Clicks Have No Effect — Stale Closure (February 2026)
+
+**Root Cause:** Three compounding issues created a stale closure chain in `ObjectView.tsx`:
+1. `{ mode: 'page' }` fallback created a new object literal every render, causing `useNavigationOverlay`'s `handleClick` `useCallback` to be recreated with potentially stale deps.
+2. The `onNavigate` callback was an inline arrow function with unstable identity, so React could batch-skip the render where `handleClick` picks up the fresh closure.
+3. Cascade instability: `navOverlay` (from `useMemo`) got a new reference every render because its deps (`handleClick`, etc.) changed, propagating stale closures through `renderListView`.
+
+**Fix:** Memoized `detailNavigation` with `useMemo` (stable reference for the `{ mode: 'page' }` fallback) and extracted `onNavigate` into a `useCallback` (`handleNavOverlayNavigate`) with `[navigate, viewId]` deps. This ensures stable identities for both inputs to `useNavigationOverlay`, preventing stale closures.
+
+**Tests:** Added 2 stale closure prevention tests in `useNavigationOverlay.test.ts`: (1) verify `handleClick` uses latest `onNavigate` after re-render with new callback, (2) verify navigation works after config changes from undefined to explicit page mode. All 31 useNavigationOverlay tests and 739 console tests pass.
+
 ### ListView Grouping Config Not Taking Effect (February 2026)
 
 **Root Cause:** `viewComponentSchema` `useMemo` in `ListView.tsx` was missing `groupingConfig`, `rowColorConfig`, and `navigation.handleClick` in its dependency array. When users toggled grouping fields via the toolbar popover, the state changed but the memoized schema was not recomputed, so the child grid/kanban/gallery never received the updated grouping config.

apps/console/src/components/ObjectView.tsx

@@ -420,12 +420,15 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
 
     // Navigation overlay for record detail (supports drawer/modal/split/popover via config)
     // Priority: activeView.navigation > objectDef.navigation > default page
-    const detailNavigation: ViewNavigationConfig = activeView?.navigation ?? objectDef.navigation ?? { mode: 'page' };
+    // Memoize to avoid unstable object identity on every render (stale closure prevention)
+    const detailNavigation: ViewNavigationConfig = useMemo(
+        () => activeView?.navigation ?? objectDef.navigation ?? { mode: 'page' },
+        [activeView?.navigation, objectDef.navigation]
+    );
     const drawerRecordId = searchParams.get('recordId');
-    const navOverlay = useNavigationOverlay({
-        navigation: detailNavigation,
-        objectName: objectDef.name,
-        onNavigate: (recordId: string | number, action?: string) => {
+    // Memoize onNavigate to prevent stale closure in useNavigationOverlay's handleClick
+    const handleNavOverlayNavigate = useCallback(
+        (recordId: string | number, action?: string) => {
             if (action === 'new_window') {
                 // Open record detail in a new browser tab with Console-correct URL
                 const basePath = window.location.pathname.replace(/\/view\/.*$/, '');
@@ -443,6 +446,12 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
                 }
             }
         },
+        [navigate, viewId]
+    );
+    const navOverlay = useNavigationOverlay({
+        navigation: detailNavigation,
+        objectName: objectDef.name,
+        onNavigate: handleNavOverlayNavigate,
     });
     const handleDrawerClose = () => {
         navOverlay.close();

packages/react/src/__tests__/useNavigationOverlay.test.ts

@@ -507,4 +507,72 @@ describe('useNavigationOverlay', () => {
       expect(result.current.view).toBeUndefined();
     });
   });
+
+  // ============================================================
+  // Stale closure prevention
+  // ============================================================
+
+  describe('stale closure prevention', () => {
+    it('should use latest onNavigate after re-render with new callback', () => {
+      const onNavigateV1 = vi.fn();
+      const onNavigateV2 = vi.fn();
+
+      const { result, rerender } = renderHook(
+        ({ onNavigate }) =>
+          useNavigationOverlay({
+            navigation: { mode: 'page' },
+            objectName: 'contacts',
+            onNavigate,
+          }),
+        { initialProps: { onNavigate: onNavigateV1 } }
+      );
+
+      // First click uses v1
+      act(() => {
+        result.current.handleClick({ _id: '1' });
+      });
+      expect(onNavigateV1).toHaveBeenCalledWith('1', 'view');
+      expect(onNavigateV2).not.toHaveBeenCalled();
+
+      // Re-render with new onNavigate
+      rerender({ onNavigate: onNavigateV2 });
+
+      // Second click must use v2 (not stale v1)
+      act(() => {
+        result.current.handleClick({ _id: '2' });
+      });
+      expect(onNavigateV2).toHaveBeenCalledWith('2', 'view');
+      expect(onNavigateV1).toHaveBeenCalledTimes(1); // v1 was not called again
+    });
+
+    it('should use latest onNavigate after navigation config changes from undefined to page', () => {
+      const onNavigate = vi.fn();
+
+      const { result, rerender } = renderHook(
+        ({ navigation }) =>
+          useNavigationOverlay({
+            navigation,
+            objectName: 'contacts',
+            onNavigate,
+          }),
+        { initialProps: { navigation: undefined as NavigationConfig | undefined } }
+      );
+
+      // Click with no config — should still call onNavigate (default page behavior)
+      act(() => {
+        result.current.handleClick({ _id: '1' });
+      });
+      expect(onNavigate).toHaveBeenCalledWith('1', 'view');
+
+      // Re-render with explicit page config
+      rerender({ navigation: { mode: 'page' } });
+
+      // Click should still work (not stale)
+      act(() => {
+        result.current.handleClick({ _id: '2' });
+      });
+      expect(onNavigate).toHaveBeenCalledWith('2', 'view');
+      expect(onNavigate).toHaveBeenCalledTimes(2);
+    });
+  });
 });