-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Console UX Phase 2 — skeletons, toasts, keyboard shortcuts, responsive sidebar, breadcrumb nav, recent items, file upload #437
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
97b8ac3
d12f03e
611ffba
d690500
a9898a2
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 | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { BrowserRouter, Routes, Route, Navigate, useNavigate, useLocation, useSe | |||||||||||||||||||||
| import { useState, useEffect } from 'react'; | ||||||||||||||||||||||
| import { ObjectForm } from '@object-ui/plugin-form'; | ||||||||||||||||||||||
| import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription, Empty, EmptyTitle } from '@object-ui/components'; | ||||||||||||||||||||||
| import { toast } from 'sonner'; | ||||||||||||||||||||||
| import { SchemaRendererProvider } from '@object-ui/react'; | ||||||||||||||||||||||
| import { ObjectStackAdapter } from './dataSource'; | ||||||||||||||||||||||
| import type { ConnectionState } from './dataSource'; | ||||||||||||||||||||||
|
|
@@ -20,6 +21,8 @@ import { PageView } from './components/PageView'; | |||||||||||||||||||||
| import { ReportView } from './components/ReportView'; | ||||||||||||||||||||||
| import { ExpressionProvider } from './context/ExpressionProvider'; | ||||||||||||||||||||||
| import { ConditionalAuthWrapper } from './components/ConditionalAuthWrapper'; | ||||||||||||||||||||||
| import { KeyboardShortcutsDialog } from './components/KeyboardShortcutsDialog'; | ||||||||||||||||||||||
| import { useRecentItems } from './hooks/useRecentItems'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Auth Pages | ||||||||||||||||||||||
| import { LoginPage } from './pages/LoginPage'; | ||||||||||||||||||||||
|
|
@@ -35,6 +38,7 @@ import { ProfilePage } from './pages/system/ProfilePage'; | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { useParams } from 'react-router-dom'; | ||||||||||||||||||||||
| import { ThemeProvider } from './components/theme-provider'; | ||||||||||||||||||||||
| import { ConsoleToaster } from './components/ConsoleToaster'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export function AppContent() { | ||||||||||||||||||||||
| const [dataSource, setDataSource] = useState<ObjectStackAdapter | null>(null); | ||||||||||||||||||||||
|
|
@@ -55,6 +59,7 @@ export function AppContent() { | |||||||||||||||||||||
| const [isDialogOpen, setIsDialogOpen] = useState(false); | ||||||||||||||||||||||
| const [editingRecord, setEditingRecord] = useState<any>(null); | ||||||||||||||||||||||
| const [refreshKey, setRefreshKey] = useState(0); | ||||||||||||||||||||||
| const { addRecentItem } = useRecentItems(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Branding is now applied by AppShell via ConsoleLayout | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -116,6 +121,47 @@ export function AppContent() { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| const currentObjectDef = allObjects.find((o: any) => o.name === objectNameFromPath); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Track recent items on route change | ||||||||||||||||||||||
| // Only depend on location.pathname — the sole external trigger. | ||||||||||||||||||||||
| // All other values (activeApp, allObjects, cleanParts) are derived from | ||||||||||||||||||||||
| // stable module-level config and the current pathname, so they don't need | ||||||||||||||||||||||
| // to be in the dependency array (and including array refs would loop). | ||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||
| if (!activeApp) return; | ||||||||||||||||||||||
| const parts = location.pathname.split('/').filter(Boolean); | ||||||||||||||||||||||
| let objName = parts[2]; | ||||||||||||||||||||||
| if (objName === 'view' || objName === 'record' || objName === 'page' || objName === 'dashboard') { | ||||||||||||||||||||||
| objName = ''; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| const basePath = `/apps/${activeApp.name}`; | ||||||||||||||||||||||
| const objects = appConfig.objects || []; | ||||||||||||||||||||||
| if (objName) { | ||||||||||||||||||||||
| const obj = objects.find((o: any) => o.name === objName); | ||||||||||||||||||||||
| if (obj) { | ||||||||||||||||||||||
| addRecentItem({ | ||||||||||||||||||||||
| id: `object:${obj.name}`, | ||||||||||||||||||||||
| label: obj.label || obj.name, | ||||||||||||||||||||||
| href: `${basePath}/${obj.name}`, | ||||||||||||||||||||||
| type: 'object', | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } else if (parts[2] === 'dashboard' && parts[3]) { | ||||||||||||||||||||||
| addRecentItem({ | ||||||||||||||||||||||
| id: `dashboard:${parts[3]}`, | ||||||||||||||||||||||
| label: parts[3].replace(/[-_]/g, ' ').replace(/\b\w/g, (c: string) => c.toUpperCase()), | ||||||||||||||||||||||
| href: `${basePath}/dashboard/${parts[3]}`, | ||||||||||||||||||||||
| type: 'dashboard', | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } else if (parts[2] === 'report' && parts[3]) { | ||||||||||||||||||||||
| addRecentItem({ | ||||||||||||||||||||||
| id: `report:${parts[3]}`, | ||||||||||||||||||||||
| label: parts[3].replace(/[-_]/g, ' ').replace(/\b\w/g, (c: string) => c.toUpperCase()), | ||||||||||||||||||||||
| href: `${basePath}/report/${parts[3]}`, | ||||||||||||||||||||||
| type: 'report', | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }, [location.pathname, addRecentItem]); // eslint-disable-line react-hooks/exhaustive-deps | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const handleEdit = (record: any) => { | ||||||||||||||||||||||
| setEditingRecord(record); | ||||||||||||||||||||||
| setIsDialogOpen(true); | ||||||||||||||||||||||
|
|
@@ -166,6 +212,7 @@ export function AppContent() { | |||||||||||||||||||||
| objects={allObjects} | ||||||||||||||||||||||
| onAppChange={handleAppChange} | ||||||||||||||||||||||
| /> | ||||||||||||||||||||||
| <KeyboardShortcutsDialog /> | ||||||||||||||||||||||
| <SchemaRendererProvider dataSource={dataSource || {}}> | ||||||||||||||||||||||
| <ErrorBoundary> | ||||||||||||||||||||||
| <Routes> | ||||||||||||||||||||||
|
|
@@ -242,7 +289,15 @@ export function AppContent() { | |||||||||||||||||||||
| ? currentObjectDef.fields.map((f: any) => typeof f === 'string' ? f : f.name) | ||||||||||||||||||||||
| : Object.keys(currentObjectDef.fields)) | ||||||||||||||||||||||
| : [], | ||||||||||||||||||||||
| onSuccess: () => { setIsDialogOpen(false); setRefreshKey(k => k + 1); }, | ||||||||||||||||||||||
| onSuccess: () => { | ||||||||||||||||||||||
| setIsDialogOpen(false); | ||||||||||||||||||||||
| setRefreshKey(k => k + 1); | ||||||||||||||||||||||
| toast.success( | ||||||||||||||||||||||
| editingRecord | ||||||||||||||||||||||
| ? `${currentObjectDef?.label} updated successfully` | ||||||||||||||||||||||
| : `${currentObjectDef?.label} created successfully` | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| onCancel: () => setIsDialogOpen(false), | ||||||||||||||||||||||
| showSubmit: true, | ||||||||||||||||||||||
| showCancel: true, | ||||||||||||||||||||||
|
|
@@ -292,6 +347,7 @@ function RootRedirect() { | |||||||||||||||||||||
| export function App() { | ||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
| <ThemeProvider defaultTheme="system" storageKey="object-ui-theme"> | ||||||||||||||||||||||
| <ConsoleToaster position="bottom-right" /> | ||||||||||||||||||||||
|
Comment on lines
347
to
+350
|
||||||||||||||||||||||
| export function App() { | |
| return ( | |
| <ThemeProvider defaultTheme="system" storageKey="object-ui-theme"> | |
| <ConsoleToaster position="bottom-right" /> | |
| const consoleToastPosition = (appConfig as any)?.toastPosition ?? 'bottom-right'; | |
| export function App() { | |
| return ( | |
| <ThemeProvider defaultTheme="system" storageKey="object-ui-theme"> | |
| <ConsoleToaster position={consoleToastPosition} /> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| /** | ||
| * Tests for KeyboardShortcutsDialog component | ||
| */ | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import '@testing-library/jest-dom'; | ||
| import { KeyboardShortcutsDialog } from '../components/KeyboardShortcutsDialog'; | ||
|
|
||
| // Mock @object-ui/components Dialog | ||
| vi.mock('@object-ui/components', () => ({ | ||
| Dialog: ({ open, children }: any) => open ? <div data-testid="dialog">{children}</div> : null, | ||
| DialogContent: ({ children }: any) => <div data-testid="dialog-content">{children}</div>, | ||
| DialogHeader: ({ children }: any) => <div>{children}</div>, | ||
| DialogTitle: ({ children }: any) => <h2>{children}</h2>, | ||
| DialogDescription: ({ children }: any) => <p>{children}</p>, | ||
| })); | ||
|
|
||
| describe('KeyboardShortcutsDialog', () => { | ||
| it('renders without errors', () => { | ||
| const { container } = render(<KeyboardShortcutsDialog />); | ||
| // Dialog should be closed initially | ||
| expect(container.querySelector('[data-testid="dialog"]')).toBeNull(); | ||
| }); | ||
|
|
||
| it('opens when ? key is pressed', () => { | ||
| render(<KeyboardShortcutsDialog />); | ||
|
|
||
| fireEvent.keyDown(document, { key: '?' }); | ||
|
|
||
| expect(screen.getByTestId('dialog')).toBeInTheDocument(); | ||
| expect(screen.getByText('Keyboard Shortcuts')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows shortcut categories', () => { | ||
| render(<KeyboardShortcutsDialog />); | ||
| fireEvent.keyDown(document, { key: '?' }); | ||
|
|
||
| expect(screen.getByText('General')).toBeInTheDocument(); | ||
| expect(screen.getByText('Navigation')).toBeInTheDocument(); | ||
| expect(screen.getByText('Data Views')).toBeInTheDocument(); | ||
| expect(screen.getByText('Preferences')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('does not open when ? is pressed in an input', () => { | ||
| const { container } = render( | ||
| <div> | ||
| <input data-testid="input" /> | ||
| <KeyboardShortcutsDialog /> | ||
| </div> | ||
| ); | ||
|
|
||
| const input = screen.getByTestId('input'); | ||
| fireEvent.keyDown(input, { key: '?' }); | ||
|
|
||
| expect(container.querySelector('[data-testid="dialog"]')).toBeNull(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| /** | ||
| * Tests for useRecentItems hook | ||
| */ | ||
| import { describe, it, expect, beforeEach, vi } from 'vitest'; | ||
| import { renderHook, act } from '@testing-library/react'; | ||
| import { useRecentItems } from '../hooks/useRecentItems'; | ||
|
|
||
| // Mock localStorage | ||
| const localStorageMock = (() => { | ||
| let store: Record<string, string> = {}; | ||
| return { | ||
| getItem: vi.fn((key: string) => store[key] ?? null), | ||
| setItem: vi.fn((key: string, value: string) => { store[key] = value; }), | ||
| removeItem: vi.fn((key: string) => { delete store[key]; }), | ||
| clear: vi.fn(() => { store = {}; }), | ||
| }; | ||
| })(); | ||
|
|
||
| Object.defineProperty(window, 'localStorage', { value: localStorageMock }); | ||
|
|
||
| describe('useRecentItems', () => { | ||
| beforeEach(() => { | ||
| localStorageMock.clear(); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('starts with empty items when localStorage is empty', () => { | ||
| const { result } = renderHook(() => useRecentItems()); | ||
| expect(result.current.recentItems).toEqual([]); | ||
| }); | ||
|
|
||
| it('adds a recent item', () => { | ||
| const { result } = renderHook(() => useRecentItems()); | ||
|
|
||
| act(() => { | ||
| result.current.addRecentItem({ | ||
| id: 'object:contact', | ||
| label: 'Contacts', | ||
| href: '/apps/crm/contact', | ||
| type: 'object', | ||
| }); | ||
| }); | ||
|
|
||
| expect(result.current.recentItems).toHaveLength(1); | ||
| expect(result.current.recentItems[0].id).toBe('object:contact'); | ||
| expect(result.current.recentItems[0].label).toBe('Contacts'); | ||
| expect(result.current.recentItems[0].visitedAt).toBeDefined(); | ||
| }); | ||
|
|
||
| it('deduplicates items by id', () => { | ||
| const { result } = renderHook(() => useRecentItems()); | ||
|
|
||
| act(() => { | ||
| result.current.addRecentItem({ | ||
| id: 'object:contact', | ||
| label: 'Contacts', | ||
| href: '/apps/crm/contact', | ||
| type: 'object', | ||
| }); | ||
| }); | ||
|
|
||
| act(() => { | ||
| result.current.addRecentItem({ | ||
| id: 'object:contact', | ||
| label: 'Contacts Updated', | ||
| href: '/apps/crm/contact', | ||
| type: 'object', | ||
| }); | ||
| }); | ||
|
|
||
| expect(result.current.recentItems).toHaveLength(1); | ||
| expect(result.current.recentItems[0].label).toBe('Contacts Updated'); | ||
| }); | ||
|
|
||
| it('limits to max 8 items', () => { | ||
| const { result } = renderHook(() => useRecentItems()); | ||
|
|
||
| for (let i = 0; i < 10; i++) { | ||
| act(() => { | ||
| result.current.addRecentItem({ | ||
| id: `object:item-${i}`, | ||
| label: `Item ${i}`, | ||
| href: `/apps/crm/item-${i}`, | ||
| type: 'object', | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| expect(result.current.recentItems.length).toBeLessThanOrEqual(8); | ||
| }); | ||
|
|
||
| it('clears all items', () => { | ||
| const { result } = renderHook(() => useRecentItems()); | ||
|
|
||
| act(() => { | ||
| result.current.addRecentItem({ | ||
| id: 'object:contact', | ||
| label: 'Contacts', | ||
| href: '/apps/crm/contact', | ||
| type: 'object', | ||
| }); | ||
| }); | ||
|
|
||
| act(() => { | ||
| result.current.clearRecentItems(); | ||
| }); | ||
|
|
||
| expect(result.current.recentItems).toEqual([]); | ||
| }); | ||
|
|
||
| it('persists items to localStorage', () => { | ||
| const { result } = renderHook(() => useRecentItems()); | ||
|
|
||
| act(() => { | ||
| result.current.addRecentItem({ | ||
| id: 'object:contact', | ||
| label: 'Contacts', | ||
| href: '/apps/crm/contact', | ||
| type: 'object', | ||
| }); | ||
| }); | ||
|
|
||
| expect(localStorageMock.setItem).toHaveBeenCalledWith( | ||
| 'objectui-recent-items', | ||
| expect.any(String), | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| /** | ||
| * Tests for skeleton loading components | ||
| */ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { render } from '@testing-library/react'; | ||
| import '@testing-library/jest-dom'; | ||
| import { SkeletonGrid } from '../components/skeletons/SkeletonGrid'; | ||
| import { SkeletonDashboard } from '../components/skeletons/SkeletonDashboard'; | ||
| import { SkeletonDetail } from '../components/skeletons/SkeletonDetail'; | ||
|
|
||
| // Mock @object-ui/components Skeleton | ||
| vi.mock('@object-ui/components', () => ({ | ||
| Skeleton: ({ className, ...props }: any) => ( | ||
| <div data-testid="skeleton" className={className} {...props} /> | ||
| ), | ||
| })); | ||
|
|
||
| describe('SkeletonGrid', () => { | ||
| it('renders with default props', () => { | ||
| const { container } = render(<SkeletonGrid />); | ||
| const skeletons = container.querySelectorAll('[data-testid="skeleton"]'); | ||
| // Header (5) + 8 rows x 5 cols (40) + toolbar (4) + pagination (4) = 53 | ||
| expect(skeletons.length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it('renders correct number of rows', () => { | ||
| const { container } = render(<SkeletonGrid rows={3} columns={2} />); | ||
| // Should have skeletons for 3 rows x 2 columns in the table body | ||
| const rowContainers = container.querySelectorAll('.border-b'); | ||
| expect(rowContainers.length).toBeGreaterThanOrEqual(3); | ||
| }); | ||
| }); | ||
|
|
||
| describe('SkeletonDashboard', () => { | ||
| it('renders with default props', () => { | ||
| const { container } = render(<SkeletonDashboard />); | ||
| const skeletons = container.querySelectorAll('[data-testid="skeleton"]'); | ||
| expect(skeletons.length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it('renders correct number of widget cards', () => { | ||
| const { container } = render(<SkeletonDashboard cards={3} />); | ||
| // 3 widget cards, each with 3 skeletons + stats row (4 cards x 3 each) + header (2) | ||
| const skeletons = container.querySelectorAll('[data-testid="skeleton"]'); | ||
| expect(skeletons.length).toBeGreaterThan(0); | ||
| }); | ||
| }); | ||
|
|
||
| describe('SkeletonDetail', () => { | ||
| it('renders with default props', () => { | ||
| const { container } = render(<SkeletonDetail />); | ||
| const skeletons = container.querySelectorAll('[data-testid="skeleton"]'); | ||
| expect(skeletons.length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it('renders correct number of field rows', () => { | ||
| const { container } = render(<SkeletonDetail fields={4} columns={1} />); | ||
| const skeletons = container.querySelectorAll('[data-testid="skeleton"]'); | ||
| expect(skeletons.length).toBeGreaterThan(0); | ||
| }); | ||
| }); | ||
|
Comment on lines
+18
to
+61
|
||
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.
The eslint-disable comment may be masking a real issue. The effect uses
activeApp(line 130, 136) which is derived from theappNameroute parameter (line 57), butactiveAppis not in the dependency array. If the app changes (e.g., navigating from /apps/crm to /apps/sales), the effect won't re-run because onlylocation.pathnameandaddRecentItemare dependencies. Consider either addingactiveAppto the dependency array or usingappNamefrom the location directly inside the effect.