Merge pull request #886 from objectstack-ai/copilot/fix-listview-navigation-issues

hotlong · web-flow · commit 72c97098df31 · 2026-02-27T14:08:46.000+08:00
diff --git a/ROADMAP.md b/ROADMAP.md
@@ -200,6 +200,13 @@ ObjectUI is a universal Server-Driven UI (SDUI) engine built on React + Tailwind
 - [x] Navigation Sync Service — `useNavigationSync` hook auto-syncs App navigation tree on Page/Dashboard CRUD (create, delete, rename) with toast + undo
 - [x] Navigation Sync auto-detection — `NavigationSyncEffect` component monitors metadata changes and auto-syncs navigation across ALL apps when pages/dashboards are added or removed
 - [x] Navigation Sync all-apps convenience API — `sync*AllApps` methods iterate all apps without requiring explicit `appName`
+- [x] **ListView Navigation Mode Fix** — All 6 navigation modes (page/drawer/modal/split/popover/new_window) now work correctly on row click:
+  - ✅ `page` mode navigates to `/record/:recordId` detail page via React Router
+  - ✅ `new_window` mode opens correct Console URL in a new browser tab (delegates to `onNavigate`)
+  - ✅ `split` mode renders resizable split panels with main content + detail panel
+  - ✅ `popover` mode falls back to compact dialog when no `popoverTrigger` is provided
+  - ✅ `useNavigationOverlay` hook delegates `new_window` to `onNavigate` when available for app-specific URL control
+  - ✅ plugin-view `handleRowClick` supports `split` and `popover` branches
 
 ### P1.8 Console — View Config Panel (Phase 20)
 
diff --git a/apps/console/src/__tests__/ObjectView.test.tsx b/apps/console/src/__tests__/ObjectView.test.tsx
@@ -33,6 +33,7 @@ vi.mock('@object-ui/plugin-list', () => ({
         {props.schema?.selection?.type && <div data-testid="schema-selection-type">{props.schema.selection.type}</div>}
         {props.schema?.addRecord?.enabled && <div data-testid="schema-addRecord-enabled">addRecord</div>}
         {props.schema?.addRecordViaForm && <div data-testid="schema-addRecordViaForm">addRecordViaForm</div>}
+        <button data-testid="list-row-click" onClick={() => props.onRowClick?.({ _id: 'rec-1', id: 'rec-1', name: 'Test Record' })}>Click Row</button>
       </div>
     );
   },
@@ -762,6 +763,102 @@ describe('ObjectView Component', () => {
         expect(screen.getByTestId('object-grid')).toBeInTheDocument();
     });
 
+    it('navigates to record detail page for page navigation mode via onNavigate', () => {
+        const objectsWithPage = [
+            {
+                ...mockObjects[0],
+                navigation: { mode: 'page' as const },
+            }
+        ];
+        mockUseParams.mockReturnValue({ objectName: 'opportunity' });
+
+        render(<ObjectView dataSource={mockDataSource} objects={objectsWithPage} onEdit={vi.fn()} />);
+
+        // Click a list row — should trigger page navigation
+        fireEvent.click(screen.getByTestId('list-row-click'));
+
+        // Verify React Router navigate was called with the record detail path
+        expect(mockNavigate).toHaveBeenCalledWith('record/rec-1');
+    });
+
+    it('opens new window with correct URL for new_window navigation mode', () => {
+        const mockOpen = vi.fn();
+        const originalOpen = window.open;
+        window.open = mockOpen;
+
+        const objectsWithNewWindow = [
+            {
+                ...mockObjects[0],
+                navigation: { mode: 'new_window' as const },
+            }
+        ];
+        mockUseParams.mockReturnValue({ objectName: 'opportunity' });
+
+        render(<ObjectView dataSource={mockDataSource} objects={objectsWithNewWindow} onEdit={vi.fn()} />);
+
+        // Click a list row — should open new window
+        fireEvent.click(screen.getByTestId('list-row-click'));
+
+        // Verify window.open was called with a URL containing /record/rec-1
+        expect(mockOpen).toHaveBeenCalledTimes(1);
+        expect(mockOpen.mock.calls[0][0]).toContain('/record/rec-1');
+        expect(mockOpen.mock.calls[0][1]).toBe('_blank');
+
+        window.open = originalOpen;
+    });
+
+    it('renders split layout with mainContent when split mode is active', async () => {
+        const objectsWithSplit = [
+            {
+                ...mockObjects[0],
+                navigation: { mode: 'split' as const },
+            }
+        ];
+        mockUseParams.mockReturnValue({ objectName: 'opportunity' });
+
+        const dataSourceWithFindOne = {
+            ...mockDataSource,
+            findOne: vi.fn().mockResolvedValue({ _id: 'rec-1', id: 'rec-1', name: 'Test' }),
+        };
+
+        render(<ObjectView dataSource={dataSourceWithFindOne} objects={objectsWithSplit} onEdit={vi.fn()} />);
+
+        // Click a list row — should open the split detail panel
+        fireEvent.click(screen.getByTestId('list-row-click'));
+
+        // The grid should still render inside the split layout
+        await vi.waitFor(() => {
+            expect(screen.getByTestId('object-grid')).toBeInTheDocument();
+            // Split mode should render the close button for the detail panel
+            expect(screen.getByLabelText('Close panel')).toBeInTheDocument();
+        });
+    });
+
+    it('renders popover overlay without popoverTrigger using fallback dialog', async () => {
+        const objectsWithPopover = [
+            {
+                ...mockObjects[0],
+                navigation: { mode: 'popover' as const },
+            }
+        ];
+        mockUseParams.mockReturnValue({ objectName: 'opportunity' });
+
+        const dataSourceWithFindOne = {
+            ...mockDataSource,
+            findOne: vi.fn().mockResolvedValue({ _id: 'rec-1', id: 'rec-1', name: 'Test' }),
+        };
+
+        render(<ObjectView dataSource={dataSourceWithFindOne} objects={objectsWithPopover} onEdit={vi.fn()} />);
+
+        // Click a list row — should open the popover fallback dialog
+        fireEvent.click(screen.getByTestId('list-row-click'));
+
+        // The popover fallback dialog should render (dialog role from Radix)
+        await vi.waitFor(() => {
+            expect(screen.getByRole('dialog')).toBeInTheDocument();
+        });
+    });
+
     it('renders RecordChatterPanel inside drawer overlay when navigation mode is drawer', async () => {
         mockAuthUser = { id: 'u1', name: 'Admin', role: 'admin' };
         // Provide recordId in URL to trigger overlay open
diff --git a/apps/console/src/components/ObjectView.tsx b/apps/console/src/components/ObjectView.tsx
@@ -425,10 +425,19 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
     const navOverlay = useNavigationOverlay({
         navigation: detailNavigation,
         objectName: objectDef.name,
-        onNavigate: (recordId: string | number, _action?: string) => {
-            const newParams = new URLSearchParams(searchParams);
-            newParams.set('recordId', String(recordId));
-            setSearchParams(newParams);
+        onNavigate: (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\/.*$/, '');
+                window.open(`${basePath}/record/${String(recordId)}`, '_blank');
+                return;
+            }
+            // page / view mode — navigate to record detail page
+            if (viewId) {
+                navigate(`../../record/${String(recordId)}`, { relative: 'path' });
+            } else {
+                navigate(`record/${String(recordId)}`);
+            }
         },
     });
     const handleDrawerClose = () => {
@@ -633,9 +642,15 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
         onNavigate: (recordId: string | number, mode: 'view' | 'edit') => {
             if (mode === 'edit') {
                 onEdit?.({ _id: recordId, id: recordId });
+            } else if (mode === 'view') {
+                if (viewId) {
+                    navigate(`../../record/${String(recordId)}`, { relative: 'path' });
+                } else {
+                    navigate(`record/${String(recordId)}`);
+                }
             }
         },
-    }), [objectDef.name, onEdit, activeView?.showSearch, activeView?.showFilters, activeView?.showSort]);
+    }), [objectDef.name, onEdit, activeView?.showSearch, activeView?.showFilters, activeView?.showSort, navigate, viewId]);
 
     return (
         <div className="h-full flex flex-col bg-background min-w-0 overflow-hidden">
@@ -767,6 +782,48 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
 
              {/* 2. Content — Plugin ObjectView with ViewSwitcher + Filter + Sort */}
              <div className="flex-1 overflow-hidden relative flex flex-row">
+                {navOverlay.mode === 'split' && navOverlay.isOpen ? (
+                    <NavigationOverlay
+                        {...navOverlay}
+                        setIsOpen={(open: boolean) => { if (!open) handleDrawerClose(); }}
+                        title={objectDef.label}
+                        mainContent={
+                            <div className="flex-1 min-w-0 relative h-full flex flex-col">
+                                <div className="flex-1 relative overflow-auto p-3 sm:p-4">
+                                    <PluginObjectView
+                                        schema={objectViewSchema}
+                                        dataSource={dataSource}
+                                        views={mergedViews}
+                                        activeViewId={activeViewId}
+                                        onViewChange={handleViewChange}
+                                        onEdit={(record: any) => onEdit?.(record)}
+                                        onRowClick={onRowClick || ((record: any) => {
+                                            navOverlay.handleClick(record);
+                                        })}
+                                        renderListView={renderListView}
+                                    />
+                                </div>
+                                {typeof recordCount === 'number' && (
+                                    <div data-testid="record-count-footer" className="border-t px-3 sm:px-4 py-1.5 text-xs text-muted-foreground bg-muted/5 shrink-0">
+                                        {t('console.objectView.recordCount', { count: recordCount })}
+                                    </div>
+                                )}
+                            </div>
+                        }
+                    >
+                        {(record: Record<string, unknown>) => {
+                            const recordId = (record._id || record.id) as string;
+                            return (
+                                <DrawerDetailContent
+                                    objectDef={objectDef}
+                                    recordId={recordId}
+                                    dataSource={dataSource}
+                                    onEdit={onEdit}
+                                />
+                            );
+                        }}
+                    </NavigationOverlay>
+                ) : (
                 <div className="flex-1 min-w-0 relative h-full flex flex-col">
                     <div className="flex-1 relative overflow-auto p-3 sm:p-4">
                         <PluginObjectView
@@ -789,6 +846,7 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
                         </div>
                     )}
                 </div>
+                )}
                 {/* Metadata panel only shows for admin users */}
                 <MetadataPanel
                     open={showDebug && isAdmin}
@@ -819,6 +877,7 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
              </div>
 
              {/* Record Detail Overlay — navigation mode driven by objectDef.navigation */}
+             {navOverlay.mode !== 'split' && (
              <NavigationOverlay
                  {...navOverlay}
                  setIsOpen={(open: boolean) => { if (!open) handleDrawerClose(); }}
@@ -837,6 +896,7 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
                      );
                  }}
              </NavigationOverlay>
+             )}
         </div>
     );
 }
diff --git a/packages/components/src/__tests__/navigation-overlay.test.tsx b/packages/components/src/__tests__/navigation-overlay.test.tsx
@@ -219,6 +219,31 @@ describe('NavigationOverlay', () => {
       );
       expect(screen.getByText('Quick View')).toBeInTheDocument();
     });
+
+    it('should render fallback dialog when no popoverTrigger is provided', () => {
+      render(
+        <NavigationOverlay
+          {...createProps({
+            mode: 'popover',
+            title: 'Preview',
+          })}
+        />
+      );
+      expect(screen.getByText('Preview')).toBeInTheDocument();
+      expect(screen.getByText('Test Record')).toBeInTheDocument();
+    });
+
+    it('should not render fallback dialog when closed and no popoverTrigger', () => {
+      const { container } = render(
+        <NavigationOverlay
+          {...createProps({
+            mode: 'popover',
+            isOpen: false,
+          })}
+        />
+      );
+      expect(container.innerHTML).toBe('');
+    });
   });
 
   // ============================================================
diff --git a/packages/components/src/custom/navigation-overlay.tsx b/packages/components/src/custom/navigation-overlay.tsx
@@ -285,6 +285,26 @@ export const NavigationOverlay: React.FC<NavigationOverlayProps> = ({
 
   // --- Popover Mode ---
   if (mode === 'popover') {
+    if (!popoverTrigger) {
+      // Fallback: render as a compact floating card when no trigger element is provided
+      if (!isOpen) return null;
+      return (
+        <Dialog open={isOpen} onOpenChange={setIsOpen}>
+          <DialogContent
+            className={cn('w-96 max-h-[80vh] overflow-y-auto p-4', className)}
+            style={widthStyle}
+          >
+            <DialogHeader>
+              <DialogTitle className="text-sm">{resolvedTitle}</DialogTitle>
+              {description && <DialogDescription className="text-xs">{description}</DialogDescription>}
+            </DialogHeader>
+            <div className="mt-2">
+              {renderContent(selectedRecord)}
+            </div>
+          </DialogContent>
+        </Dialog>
+      );
+    }
     return (
       <Popover open={isOpen} onOpenChange={setIsOpen}>
         {popoverTrigger && (
diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx
@@ -425,6 +425,12 @@ export const ObjectView: React.FC<ObjectViewProps> = ({
         }
         return;
       }
+      if (navigationConfig.mode === 'split' || navigationConfig.mode === 'popover') {
+        setFormMode('view');
+        setSelectedRecord(record);
+        setIsFormOpen(true);
+        return;
+      }
     }
 
     // Default behavior
diff --git a/packages/plugin-view/src/__tests__/ObjectView.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.test.tsx
@@ -288,6 +288,36 @@ describe('ObjectView', () => {
 
       expect(onNavigate).toHaveBeenCalledWith('1', 'view');
     });
+
+    it('should open form in view mode when split navigation mode is clicked', () => {
+      const schema: ObjectViewSchema = {
+        type: 'object-view',
+        objectName: 'contacts',
+        navigation: { mode: 'split' },
+      };
+
+      render(<ObjectView schema={schema} dataSource={mockDataSource} />);
+
+      fireEvent.click(screen.getByTestId('grid-row'));
+
+      // Split mode should trigger form display in view mode
+      expect(screen.getByTestId('object-form')).toBeDefined();
+    });
+
+    it('should open form in view mode when popover navigation mode is clicked', () => {
+      const schema: ObjectViewSchema = {
+        type: 'object-view',
+        objectName: 'contacts',
+        navigation: { mode: 'popover' },
+      };
+
+      render(<ObjectView schema={schema} dataSource={mockDataSource} />);
+
+      fireEvent.click(screen.getByTestId('grid-row'));
+
+      // Popover mode should trigger form display in view mode
+      expect(screen.getByTestId('object-form')).toBeDefined();
+    });
   });
 
   // ============================
diff --git a/packages/react/src/__tests__/useNavigationOverlay.test.ts b/packages/react/src/__tests__/useNavigationOverlay.test.ts
@@ -198,6 +198,40 @@ describe('useNavigationOverlay', () => {
 
       expect(mockWindowOpen).toHaveBeenCalledWith('/users/456', '_blank');
     });
+
+    it('should delegate to onNavigate with new_window action when onNavigate is provided', () => {
+      const onNavigate = vi.fn();
+      const { result } = renderHook(() =>
+        useNavigationOverlay({
+          navigation: { mode: 'new_window' },
+          objectName: 'contacts',
+          onNavigate,
+        })
+      );
+
+      act(() => {
+        result.current.handleClick({ _id: '789' });
+      });
+
+      expect(onNavigate).toHaveBeenCalledWith('789', 'new_window');
+      expect(mockWindowOpen).not.toHaveBeenCalled();
+      expect(result.current.isOpen).toBe(false);
+    });
+
+    it('should fallback to window.open when onNavigate is not provided', () => {
+      const { result } = renderHook(() =>
+        useNavigationOverlay({
+          navigation: { mode: 'new_window' },
+          objectName: 'accounts',
+        })
+      );
+
+      act(() => {
+        result.current.handleClick({ _id: '101' });
+      });
+
+      expect(mockWindowOpen).toHaveBeenCalledWith('/accounts/101', '_blank');
+    });
   });
 
   // ============================================================
diff --git a/packages/react/src/hooks/useNavigationOverlay.ts b/packages/react/src/hooks/useNavigationOverlay.ts

Original file line number	Diff line number	Diff line change
`@@ -425,6 +425,12 @@ export const ObjectView: React.FC<ObjectViewProps> = ({`
`425`	`425`	`}`
`426`	`426`	`return;`
`427`	`427`	`}`
	`428`	`+ if (navigationConfig.mode === 'split' \|\| navigationConfig.mode === 'popover') {`
	`429`	`+ setFormMode('view');`
	`430`	`+ setSelectedRecord(record);`
	`431`	`+ setIsFormOpen(true);`
	`432`	`+ return;`
	`433`	`+ }`
`428`	`434`	`}`
`429`	`435`
`430`	`436`	`// Default behavior`