-
Notifications
You must be signed in to change notification settings - Fork 1
Remove breadcrumb and Add Filter button; simplify ViewTabBar to read-only; restructure toolbar layout #981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ae5d020
881f3d4
f7df05a
b5b99b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,11 +167,11 @@ describe('ObjectView Component', () => { | |
|
|
||
| render(<ObjectView dataSource={mockDataSource} objects={mockObjects} onEdit={vi.fn()} />); | ||
|
|
||
| // Check Header (appears in breadcrumb and h1) | ||
| // Check Header (h1 only, breadcrumb removed) | ||
| const headers = screen.getAllByText('Opportunity'); | ||
| expect(headers.length).toBeGreaterThanOrEqual(1); | ||
|
|
||
| // Check Tabs exist (also appears in breadcrumb) | ||
| // Check view tabs are rendered (without drag/add features) | ||
| const allOppTexts = screen.getAllByText('All Opportunities'); | ||
| expect(allOppTexts.length).toBeGreaterThanOrEqual(1); | ||
| expect(screen.getByText('Pipeline')).toBeInTheDocument(); | ||
|
|
@@ -284,14 +284,14 @@ describe('ObjectView Component', () => { | |
| expect(screen.getByTestId('view-config-panel')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows breadcrumb with object and view name', () => { | ||
| it('does not show breadcrumb in ObjectView (removed to avoid duplication with AppHeader)', () => { | ||
| mockUseParams.mockReturnValue({ objectName: 'opportunity' }); | ||
|
|
||
| render(<ObjectView dataSource={mockDataSource} objects={mockObjects} onEdit={vi.fn()} />); | ||
|
|
||
| // Breadcrumb should show object label and active view label (may appear in tabs too) | ||
| const allOppTexts = screen.getAllByText('All Opportunities'); | ||
| expect(allOppTexts.length).toBeGreaterThanOrEqual(2); // breadcrumb + tab | ||
| // Breadcrumb removed — "All Opportunities" should not appear in the header area | ||
| // The h1 title should still show the object label | ||
| expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Opportunity'); | ||
| }); | ||
|
|
||
| it('shows object description when present', () => { | ||
|
|
@@ -548,10 +548,9 @@ describe('ObjectView Component', () => { | |
| const titleInput = await screen.findByDisplayValue('All Opportunities'); | ||
| fireEvent.change(titleInput, { target: { value: 'Live Preview Test' } }); | ||
|
|
||
| // The breadcrumb should update immediately (live preview) since it reads from activeView | ||
| // The label change propagates via mergedViews (live preview) | ||
| await vi.waitFor(() => { | ||
| const breadcrumbItems = screen.getAllByText('Live Preview Test'); | ||
| expect(breadcrumbItems.length).toBeGreaterThanOrEqual(1); | ||
| expect(titleInput).toHaveValue('Live Preview Test'); | ||
| }); | ||
|
Comment on lines
+551
to
554
|
||
| }); | ||
|
|
||
|
|
@@ -572,10 +571,9 @@ describe('ObjectView Component', () => { | |
| const titleInput = await screen.findByDisplayValue('All Opportunities'); | ||
| fireEvent.change(titleInput, { target: { value: 'Changed Live' } }); | ||
|
|
||
| // The breadcrumb updates immediately (live preview) — this verifies that | ||
| // viewDraft → activeView data flow propagates config changes without save. | ||
| // The label change propagates via live preview | ||
| await vi.waitFor(() => { | ||
| expect(screen.getByText('Changed Live')).toBeInTheDocument(); | ||
| expect(titleInput).toHaveValue('Changed Live'); | ||
| }); | ||
|
|
||
| // Grid persists after config change (no remount) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,10 +90,10 @@ describe('Console View Switching Integration', () => { | |
| ); | ||
| }; | ||
|
|
||
| it('renders all view tabs', () => { | ||
| it('renders view tabs without drag or add features', () => { | ||
| renderObjectView(); | ||
|
|
||
| // "All Tasks" appears in breadcrumb and tab, so use getAllByText | ||
| // ViewTabBar should be present for switching views | ||
| const allTasksElements = screen.getAllByText('All Tasks'); | ||
|
Comment on lines
+93
to
97
|
||
| expect(allTasksElements.length).toBeGreaterThanOrEqual(1); | ||
| expect(screen.getByText('Board')).toBeInTheDocument(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,13 +15,13 @@ import { ObjectChart } from '@object-ui/plugin-charts'; | |||||||||||||||||
| import { ListView } from '@object-ui/plugin-list'; | ||||||||||||||||||
| import { DetailView, RecordChatterPanel } from '@object-ui/plugin-detail'; | ||||||||||||||||||
| import { ObjectView as PluginObjectView, ViewTabBar } from '@object-ui/plugin-view'; | ||||||||||||||||||
| import type { ViewTabItem, AvailableViewType } from '@object-ui/plugin-view'; | ||||||||||||||||||
| import type { ViewTabItem } from '@object-ui/plugin-view'; | ||||||||||||||||||
| // Import plugins for side-effects (registration) | ||||||||||||||||||
| import '@object-ui/plugin-grid'; | ||||||||||||||||||
| import '@object-ui/plugin-kanban'; | ||||||||||||||||||
| import '@object-ui/plugin-calendar'; | ||||||||||||||||||
| import { Button, Empty, EmptyTitle, EmptyDescription, NavigationOverlay, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, DropdownMenuSeparator } from '@object-ui/components'; | ||||||||||||||||||
| import { Plus, Table as TableIcon, Settings2, Wrench, KanbanSquare, Calendar, LayoutGrid, Activity, GanttChart, MapPin, BarChart3, ChevronRight } from 'lucide-react'; | ||||||||||||||||||
| import { Plus, Table as TableIcon, Settings2, Wrench, KanbanSquare, Calendar, LayoutGrid, Activity, GanttChart, MapPin, BarChart3 } from 'lucide-react'; | ||||||||||||||||||
| import type { ListViewSchema, ViewNavigationConfig, FeedItem } from '@object-ui/types'; | ||||||||||||||||||
| import { MetadataToggle, MetadataPanel, useMetadataInspector } from './MetadataInspector'; | ||||||||||||||||||
| import { ViewConfigPanel } from './ViewConfigPanel'; | ||||||||||||||||||
|
|
@@ -44,18 +44,6 @@ const VIEW_TYPE_ICONS: Record<string, ComponentType<{ className?: string }>> = { | |||||||||||||||||
| chart: BarChart3, | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** Available view types for quick-switch palette */ | ||||||||||||||||||
| const AVAILABLE_VIEW_TYPES: AvailableViewType[] = [ | ||||||||||||||||||
| { type: 'grid', label: 'Grid', description: 'Spreadsheet-style rows and columns' }, | ||||||||||||||||||
| { type: 'kanban', label: 'Kanban', description: 'Drag cards between columns' }, | ||||||||||||||||||
| { type: 'calendar', label: 'Calendar', description: 'View records on a calendar' }, | ||||||||||||||||||
| { type: 'gallery', label: 'Gallery', description: 'Visual card layout' }, | ||||||||||||||||||
| { type: 'timeline', label: 'Timeline', description: 'Chronological event view' }, | ||||||||||||||||||
| { type: 'gantt', label: 'Gantt', description: 'Project timeline with dependencies' }, | ||||||||||||||||||
| { type: 'map', label: 'Map', description: 'Geographic location view' }, | ||||||||||||||||||
| { type: 'chart', label: 'Chart', description: 'Data visualization' }, | ||||||||||||||||||
| ]; | ||||||||||||||||||
|
|
||||||||||||||||||
| const FALLBACK_USER = { id: 'current-user', name: 'Demo User' }; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
|
|
@@ -697,12 +685,6 @@ export function ObjectView({ dataSource, objects, onEdit }: any) { | |||||||||||||||||
| <TableIcon className="h-4 w-4 text-primary" /> | ||||||||||||||||||
| </div> | ||||||||||||||||||
| <div className="min-w-0"> | ||||||||||||||||||
| {/* Breadcrumb: Object > View */} | ||||||||||||||||||
| <div className="flex items-center gap-1 text-xs text-muted-foreground mb-0.5"> | ||||||||||||||||||
| <span className="truncate">{objectLabel(objectDef)}</span> | ||||||||||||||||||
| <ChevronRight className="h-3 w-3 shrink-0" /> | ||||||||||||||||||
| <span className="truncate font-medium text-foreground">{activeView?.label || t('console.objectView.allRecords')}</span> | ||||||||||||||||||
| </div> | ||||||||||||||||||
| <h1 className="text-base sm:text-lg font-semibold tracking-tight text-foreground truncate">{objectLabel(objectDef)}</h1> | ||||||||||||||||||
| {objectDef.description && ( | ||||||||||||||||||
| <p className="text-xs text-muted-foreground truncate hidden sm:block max-w-md">{objectDesc(objectDef)}</p> | ||||||||||||||||||
|
|
@@ -763,7 +745,7 @@ export function ObjectView({ dataSource, objects, onEdit }: any) { | |||||||||||||||||
| </div> | ||||||||||||||||||
| </div> | ||||||||||||||||||
|
|
||||||||||||||||||
| {/* View Tabs — Airtable-style named-view switcher with management UX */} | ||||||||||||||||||
| {/* View Tabs — read-only view switcher (design features like drag/add are in ViewConfigPanel) */} | ||||||||||||||||||
| {views.length > 1 && ( | ||||||||||||||||||
| <div className="border-b px-3 sm:px-4 bg-background overflow-x-auto shrink-0"> | ||||||||||||||||||
| <ViewTabBar | ||||||||||||||||||
|
|
@@ -777,39 +759,7 @@ export function ObjectView({ dataSource, objects, onEdit }: any) { | |||||||||||||||||
| activeViewId={activeViewId} | ||||||||||||||||||
| onViewChange={handleViewChange} | ||||||||||||||||||
| viewTypeIcons={VIEW_TYPE_ICONS} | ||||||||||||||||||
| config={{ ...objectDef.viewTabBar, reorderable: isAdmin ? true : objectDef.viewTabBar?.reorderable }} | ||||||||||||||||||
| onAddView={isAdmin ? () => { setViewConfigPanelMode('create'); setShowViewConfigPanel(true); } : undefined} | ||||||||||||||||||
| onRenameView={(id: any, newName: any) => { | ||||||||||||||||||
| // Rename is wired for future backend integration | ||||||||||||||||||
| console.info('[ViewTabBar] Rename view:', id, newName); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onDuplicateView={(id: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Duplicate view:', id); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onDeleteView={(id: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Delete view:', id); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onSetDefaultView={(id: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Set default view:', id); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onShareView={(id: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Share view:', id); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onPinView={(id: any, pinned: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Pin view:', id, pinned); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onReorderViews={(viewIds: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Reorder views:', viewIds); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onChangeViewType={(id: any, newType: any) => { | ||||||||||||||||||
| console.info('[ViewTabBar] Change view type:', id, newType); | ||||||||||||||||||
| }} | ||||||||||||||||||
| onConfigView={isAdmin ? (id: any) => { | ||||||||||||||||||
| handleViewChange(id); | ||||||||||||||||||
| setViewConfigPanelMode('edit'); | ||||||||||||||||||
| setShowViewConfigPanel(true); | ||||||||||||||||||
| } : undefined} | ||||||||||||||||||
| availableViewTypes={AVAILABLE_VIEW_TYPES} | ||||||||||||||||||
| config={{ reorderable: false }} | ||||||||||||||||||
|
||||||||||||||||||
| config={{ reorderable: false }} | |
| config={{ | |
| ...(objectDef as any).viewTabBar, | |
| contextMenu: false, | |
| inlineRename: false, | |
| showAddButton: false, | |
| reorderable: false, | |
| }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test name/intent says the breadcrumb was removed, but the assertion only checks that the H1 still renders; it doesn't actually verify the breadcrumb is absent. Add an explicit negative assertion (e.g. only one occurrence of the active view label, or absence of a breadcrumb-specific element) so the test will fail if the breadcrumb UI regresses.