feat: integrate Page/Dashboard editors into Console with design routes#788
feat: integrate Page/Dashboard editors into Console with design routes#788
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…h design routes, edit buttons, storybook stories, and tests Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…design page lazy imports Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细评估是不是完成了任务的所有要求? |
…esign routes from object path detection Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
经过仔细评估,以下是对 issue 所有要求的完成情况: ✅ 1. Console 路由与入口
✅ 2. 编辑与数据持久化
⏳ 3. Navigation 结构联动
✅ 4. Storybook & 测试
✅ 5. ROADMAP 同步
此外还修复了 总结: 核心功能(路由、编辑、持久化、快捷键、测试、Storybook)已全部完成。Navigation 结构联动(Section 3)由于需要与 NavigationDesigner 深度集成,建议作为独立 issue 跟进。 |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
❌ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
There was a problem hiding this comment.
Pull request overview
This PR integrates the PageCanvasEditor and DashboardEditor designer components from @object-ui/plugin-designer into the Console application, enabling full WYSIWYG editing of pages and dashboards. The integration follows the established lazy-loading pattern for route-based code splitting and provides a complete editing experience with auto-save, keyboard shortcuts, JSON export/import, and edit button entry points from read-only views.
Changes:
- Added two new design routes (
/design/page/:pageNameand/design/dashboard/:dashboardName) that wrap the editor components with save logic, keyboard shortcuts (Ctrl+S), and export/import handlers - Added Edit buttons to
PageViewandDashboardViewthat navigate to the corresponding design routes using relative paths - Created Storybook stories for both editor components with Default, WithComponents/WithWidgets, and ReadOnly variants
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
packages/plugin-designer/src/PageCanvasEditor.stories.tsx |
New Storybook stories for PageCanvasEditor with 3 variants (Default, WithComponents, ReadOnly) |
packages/plugin-designer/src/DashboardEditor.stories.tsx |
New Storybook stories for DashboardEditor with 3 variants (Default, WithWidgets, ReadOnly) |
apps/console/src/pages/PageDesignPage.tsx |
New page component wrapping PageCanvasEditor with auto-save, Ctrl+S shortcut, export/import handlers, and 404 handling |
apps/console/src/pages/DashboardDesignPage.tsx |
New page component wrapping DashboardEditor with auto-save, Ctrl+S shortcut, export/import handlers, and 404 handling |
apps/console/src/components/PageView.tsx |
Added Edit button in hover-revealed toolbar that navigates to design route |
apps/console/src/components/DashboardView.tsx |
Added Edit button in header that navigates to design route |
apps/console/src/__tests__/console-load-performance.test.tsx |
Updated MAX_CHUNK_COUNT from 20 to 22 to account for two new lazy-loaded design pages |
apps/console/src/__tests__/PageDesignPage.test.tsx |
New test file with 6 tests covering rendering, 404, back navigation, schema passthrough, onChange, and Ctrl+S save |
apps/console/src/__tests__/DashboardDesignPage.test.tsx |
New test file with 6 tests covering rendering, 404, back navigation, schema passthrough, onChange, and Ctrl+S save |
apps/console/src/App.tsx |
Added lazy imports for design pages, registered routes, and excluded 'design' from objectNameFromPath detection |
ROADMAP.md |
Updated console integration section with design page features, Ctrl+S shortcut, Storybook story count (78→80), and test counts |
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === 's') { | ||
| e.preventDefault(); | ||
| saveSchema(schemaRef.current).then((ok) => { | ||
| if (ok) toast.success('Page saved'); | ||
| }); | ||
| } | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [saveSchema]); |
There was a problem hiding this comment.
The keyboard event listener for Ctrl+S is attached to the global window object without checking if the user is focused on an input element or contenteditable area. This means pressing Ctrl+S while typing in a text field within the PageCanvasEditor will trigger a save, which could be disruptive to the user's workflow. Consider adding a check to prevent the shortcut from firing when the user is typing, similar to the pattern used in KeyboardShortcutsDialog.tsx where it checks if the target is an input, textarea, or contenteditable element.
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === 's') { | ||
| e.preventDefault(); | ||
| saveSchema(schemaRef.current).then((ok) => { | ||
| if (ok) toast.success('Dashboard saved'); | ||
| }); | ||
| } | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [saveSchema]); |
There was a problem hiding this comment.
The keyboard event listener for Ctrl+S is attached to the global window object without checking if the user is focused on an input element or contenteditable area. This means pressing Ctrl+S while typing in a text field within the DashboardEditor will trigger a save, which could be disruptive to the user's workflow. Consider adding a check to prevent the shortcut from firing when the user is typing, similar to the pattern used in KeyboardShortcutsDialog.tsx where it checks if the target is an input, textarea, or contenteditable element.
| <button | ||
| type="button" | ||
| onClick={() => navigate(`../design/page/${pageName}`)} | ||
| className="inline-flex items-center gap-1.5 rounded-md border border-input bg-background px-2.5 py-1.5 text-xs font-medium text-muted-foreground shadow-sm hover:bg-accent hover:text-accent-foreground" | ||
| data-testid="page-edit-button" | ||
| > | ||
| <Pencil className="h-3.5 w-3.5" /> | ||
| Edit | ||
| </button> |
There was a problem hiding this comment.
The Edit button lacks an aria-label attribute. While the button has visible text ("Edit") and an icon, adding an explicit aria-label would improve accessibility for screen reader users by providing clearer context, such as "Edit page" or "Edit page {pageName}". This is especially important since the button appears in a hover-revealed toolbar where context might not be immediately clear.
| a.href = url; | ||
| a.download = `${pageName || 'page'}.json`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); |
There was a problem hiding this comment.
The URL.revokeObjectURL(url) call happens immediately after the download is triggered with a.click(), which may cause the download to fail in some browsers if the browser hasn't started the download yet. Consider wrapping the revoke call in a setTimeout with a small delay (e.g., 100ms) to ensure the browser has time to begin the download before the URL is revoked. This is a common best practice for programmatic downloads to avoid race conditions.
| URL.revokeObjectURL(url); | |
| setTimeout(() => { | |
| URL.revokeObjectURL(url); | |
| }, 100); |
| a.href = url; | ||
| a.download = `${dashboardName || 'dashboard'}.json`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); |
There was a problem hiding this comment.
The URL.revokeObjectURL(url) call happens immediately after the download is triggered with a.click(), which may cause the download to fail in some browsers if the browser hasn't started the download yet. Consider wrapping the revoke call in a setTimeout with a small delay (e.g., 100ms) to ensure the browser has time to begin the download before the URL is revoked. This is a common best practice for programmatic downloads to avoid race conditions.
| URL.revokeObjectURL(url); | |
| setTimeout(() => { | |
| URL.revokeObjectURL(url); | |
| }, 100); |
| const page = pages?.find((p: any) => p.name === pageName); | ||
|
|
||
| const [schema, setSchema] = useState<PageSchema>( | ||
| () => (page as PageSchema) || { type: 'page', name: pageName ?? '', title: pageName ?? '', children: [] }, | ||
| ); | ||
| const schemaRef = useRef(schema); | ||
| schemaRef.current = schema; | ||
|
|
||
| const saveSchema = useCallback( | ||
| async (toSave: PageSchema) => { | ||
| try { | ||
| if (dataSource) { | ||
| await dataSource.update('sys_page', pageName!, toSave); | ||
| return true; | ||
| } | ||
| } catch { | ||
| // Save errors are non-blocking; user can retry via export | ||
| } | ||
| return false; | ||
| }, | ||
| [dataSource, pageName], | ||
| ); | ||
|
|
||
| const handleChange = useCallback( | ||
| async (updated: PageSchema) => { | ||
| setSchema(updated); | ||
| await saveSchema(updated); | ||
| }, | ||
| [saveSchema], | ||
| ); | ||
|
|
||
| // Ctrl+S / Cmd+S keyboard shortcut to explicitly save | ||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === 's') { | ||
| e.preventDefault(); | ||
| saveSchema(schemaRef.current).then((ok) => { | ||
| if (ok) toast.success('Page saved'); | ||
| }); | ||
| } | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [saveSchema]); | ||
|
|
||
| const handleExport = useCallback( | ||
| (exported: PageSchema) => { | ||
| const blob = new Blob([JSON.stringify(exported, null, 2)], { type: 'application/json' }); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| a.download = `${pageName || 'page'}.json`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); | ||
| toast.success('Page schema exported'); | ||
| }, | ||
| [pageName], | ||
| ); | ||
|
|
||
| const handleImport = useCallback( | ||
| (imported: PageSchema) => { | ||
| toast.success('Page schema imported'); | ||
| handleChange(imported); | ||
| }, | ||
| [handleChange], | ||
| ); | ||
|
|
||
| if (!page) { | ||
| return ( | ||
| <div className="h-full flex items-center justify-center text-muted-foreground"> | ||
| Page "{pageName}" not found | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The component initializes schema state with a fallback value even when the page is not found (line 27), but then returns early with a 404 message (lines 91-97). This means the state initialization, useCallback hooks, and useEffect hook all execute even when the page doesn't exist. While functionally harmless since the 404 UI is returned before rendering the editor, this pattern wastes computation. Consider moving the early return check before state initialization and hook declarations, or use a loading state pattern to defer hook execution until page existence is confirmed.
| const dashboard = dashboards?.find((d: any) => d.name === dashboardName); | ||
|
|
||
| const [schema, setSchema] = useState<DashboardSchema>( | ||
| () => | ||
| (dashboard as DashboardSchema) || { | ||
| type: 'dashboard', | ||
| name: dashboardName ?? '', | ||
| title: dashboardName ?? '', | ||
| columns: 2, | ||
| widgets: [], | ||
| }, | ||
| ); | ||
| const schemaRef = useRef(schema); | ||
| schemaRef.current = schema; | ||
|
|
||
| const saveSchema = useCallback( | ||
| async (toSave: DashboardSchema) => { | ||
| try { | ||
| if (dataSource) { | ||
| await dataSource.update('sys_dashboard', dashboardName!, toSave); | ||
| return true; | ||
| } | ||
| } catch { | ||
| // Save errors are non-blocking; user can retry via export | ||
| } | ||
| return false; | ||
| }, | ||
| [dataSource, dashboardName], | ||
| ); | ||
|
|
||
| const handleChange = useCallback( | ||
| async (updated: DashboardSchema) => { | ||
| setSchema(updated); | ||
| await saveSchema(updated); | ||
| }, | ||
| [saveSchema], | ||
| ); | ||
|
|
||
| // Ctrl+S / Cmd+S keyboard shortcut to explicitly save | ||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === 's') { | ||
| e.preventDefault(); | ||
| saveSchema(schemaRef.current).then((ok) => { | ||
| if (ok) toast.success('Dashboard saved'); | ||
| }); | ||
| } | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [saveSchema]); | ||
|
|
||
| const handleExport = useCallback( | ||
| (exported: DashboardSchema) => { | ||
| const blob = new Blob([JSON.stringify(exported, null, 2)], { type: 'application/json' }); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| a.download = `${dashboardName || 'dashboard'}.json`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); | ||
| toast.success('Dashboard schema exported'); | ||
| }, | ||
| [dashboardName], | ||
| ); | ||
|
|
||
| const handleImport = useCallback( | ||
| (imported: DashboardSchema) => { | ||
| toast.success('Dashboard schema imported'); | ||
| handleChange(imported); | ||
| }, | ||
| [handleChange], | ||
| ); | ||
|
|
||
| if (!dashboard) { | ||
| return ( | ||
| <div className="h-full flex items-center justify-center text-muted-foreground"> | ||
| Dashboard "{dashboardName}" not found | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The component initializes schema state with a fallback value even when the dashboard is not found (lines 26-35), but then returns early with a 404 message (lines 98-104). This means the state initialization, useCallback hooks, and useEffect hook all execute even when the dashboard doesn't exist. While functionally harmless since the 404 UI is returned before rendering the editor, this pattern wastes computation. Consider moving the early return check before state initialization and hook declarations, or use a loading state pattern to defer hook execution until dashboard existence is confirmed.
| const saveSchema = useCallback( | ||
| async (toSave: PageSchema) => { | ||
| try { | ||
| if (dataSource) { | ||
| await dataSource.update('sys_page', pageName!, toSave); | ||
| return true; | ||
| } | ||
| } catch { | ||
| // Save errors are non-blocking; user can retry via export | ||
| } | ||
| return false; | ||
| }, | ||
| [dataSource, pageName], | ||
| ); |
There was a problem hiding this comment.
The saveSchema function swallows all errors silently without logging or providing feedback to the user. When auto-save fails, users have no indication that their changes weren't persisted. Consider adding at minimum a console.error for debugging and optionally a toast.error for user notification, while still allowing retry via export as noted in the comment.
| const saveSchema = useCallback( | ||
| async (toSave: DashboardSchema) => { | ||
| try { | ||
| if (dataSource) { | ||
| await dataSource.update('sys_dashboard', dashboardName!, toSave); | ||
| return true; | ||
| } | ||
| } catch { | ||
| // Save errors are non-blocking; user can retry via export | ||
| } | ||
| return false; | ||
| }, | ||
| [dataSource, dashboardName], | ||
| ); |
There was a problem hiding this comment.
The saveSchema function swallows all errors silently without logging or providing feedback to the user. When auto-save fails, users have no indication that their changes weren't persisted. Consider adding at minimum a console.error for debugging and optionally a toast.error for user notification, while still allowing retry via export as noted in the comment.
| export function PageDesignPage() { | ||
| const navigate = useNavigate(); | ||
| const { pageName } = useParams<{ pageName: string }>(); | ||
| const dataSource = useAdapter(); | ||
| const { pages } = useMetadata(); | ||
|
|
||
| const page = pages?.find((p: any) => p.name === pageName); | ||
|
|
||
| const [schema, setSchema] = useState<PageSchema>( | ||
| () => (page as PageSchema) || { type: 'page', name: pageName ?? '', title: pageName ?? '', children: [] }, | ||
| ); | ||
| const schemaRef = useRef(schema); | ||
| schemaRef.current = schema; | ||
|
|
||
| const saveSchema = useCallback( | ||
| async (toSave: PageSchema) => { | ||
| try { | ||
| if (dataSource) { | ||
| await dataSource.update('sys_page', pageName!, toSave); | ||
| return true; | ||
| } | ||
| } catch { | ||
| // Save errors are non-blocking; user can retry via export | ||
| } | ||
| return false; | ||
| }, | ||
| [dataSource, pageName], | ||
| ); | ||
|
|
||
| const handleChange = useCallback( | ||
| async (updated: PageSchema) => { | ||
| setSchema(updated); | ||
| await saveSchema(updated); | ||
| }, | ||
| [saveSchema], | ||
| ); | ||
|
|
||
| // Ctrl+S / Cmd+S keyboard shortcut to explicitly save | ||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === 's') { | ||
| e.preventDefault(); | ||
| saveSchema(schemaRef.current).then((ok) => { | ||
| if (ok) toast.success('Page saved'); | ||
| }); | ||
| } | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [saveSchema]); | ||
|
|
||
| const handleExport = useCallback( | ||
| (exported: PageSchema) => { | ||
| const blob = new Blob([JSON.stringify(exported, null, 2)], { type: 'application/json' }); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| a.download = `${pageName || 'page'}.json`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); | ||
| toast.success('Page schema exported'); | ||
| }, | ||
| [pageName], | ||
| ); | ||
|
|
||
| const handleImport = useCallback( | ||
| (imported: PageSchema) => { | ||
| toast.success('Page schema imported'); | ||
| handleChange(imported); | ||
| }, | ||
| [handleChange], | ||
| ); | ||
|
|
||
| if (!page) { | ||
| return ( | ||
| <div className="h-full flex items-center justify-center text-muted-foreground"> | ||
| Page "{pageName}" not found | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className="h-full flex flex-col" data-testid="page-design-page"> | ||
| <div className="flex items-center gap-3 border-b px-4 py-3 sm:px-6"> | ||
| <button | ||
| type="button" | ||
| onClick={() => navigate(-1)} | ||
| className="rounded p-1.5 text-muted-foreground hover:text-foreground" | ||
| aria-label="Go back" | ||
| data-testid="page-design-back" | ||
| > | ||
| <ArrowLeft className="h-4 w-4" /> | ||
| </button> | ||
| <h1 className="text-lg font-semibold tracking-tight truncate"> | ||
| Edit Page: {(page as any).label || (page as any).title || pageName} | ||
| </h1> | ||
| </div> | ||
| <div className="flex-1 overflow-auto p-4 sm:p-6"> | ||
| <PageCanvasEditor | ||
| schema={schema} | ||
| onChange={handleChange} | ||
| onExport={handleExport} | ||
| onImport={handleImport} | ||
| /> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The PageDesignPage and DashboardDesignPage components contain significant code duplication. They share identical patterns for: state management (schema + schemaRef), save logic, keyboard shortcuts (Ctrl+S), export/import handlers, 404 rendering, header layout with back button, and editor wrapping structure. Consider extracting a generic DesignPageWrapper component or a custom hook (useDesignPageLogic) to reduce duplication and improve maintainability. The wrapper could accept the schema type, editor component, entity name, and metadata accessor as parameters.
Wire
PageCanvasEditorandDashboardEditorfromplugin-designerinto Console as full editing routes with auto-save, export/import, and edit button entry points from read-only views.Routes & Lazy Imports
/design/page/:pageName→PageDesignPagewrappingPageCanvasEditor/design/dashboard/:dashboardName→DashboardDesignPagewrappingDashboardEditorMAX_CHUNK_COUNTbumped 20→22designexcluded fromobjectNameFromPathdetection to prevent false object lookupsEdit Buttons on Views
PageViewandDashboardViewgain a ✏️ Edit button navigating to the corresponding design route via relative path (../design/page/:pageName)Design Pages
PageDesignPage/DashboardDesignPage: load schema fromuseMetadata(), init editor state, auto-saveonChangeviadataSource.update('sys_page'|'sys_dashboard', ...), JSON export downloads file, back button navigates-1useUndoRedo, Ctrl+Z/Ctrl+Y)Storybook
Designers/PageCanvasEditor— Default, WithComponents, ReadOnlyDesigners/DashboardEditor— Default, WithWidgets, ReadOnlyTests
PageDesignPage.test.tsxandDashboardDesignPage.test.tsx(render, 404, back nav, schema passthrough, onChange propagation, Ctrl+S keyboard save)ROADMAP
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.