feat: design mode click-to-select widgets in dashboard preview#798
feat: design mode click-to-select widgets in dashboard preview#798
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add designMode, selectedWidgetId, onWidgetClick props to DashboardRenderer - Implement widget click-to-select with primary ring highlight (a11y, light/dark) - Add keyboard navigation (Arrow keys, Enter, Space, Escape, Tab) - Add selectedWidgetId/onWidgetSelect to DashboardEditor for external control - Sync selection between DashboardRenderer and DashboardEditor in DashboardView - Add 21 new tests (15 DashboardRenderer design mode, 6 DashboardEditor external) - Update ROADMAP.md with Phase 6 — Design Mode Preview Click-to-Select Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细评估是否已经实现了完整的设计功能 |
…nd selection sync - Fix stale closure in addWidget/removeWidget (missing setSelectedWidgetId dep) - Add 8 DashboardView integration tests verifying complete selection sync flow - Add 3 new DashboardEditor tests (live preview, add widget sync, remove widget sync) - Update ROADMAP with verified end-to-end capabilities (32 total new tests) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
经过仔细评估，发现了以下问题并已修复（commit 修复的 Bug：
补充的集成测试（+11 新测试）：
功能完整性评估（全部 ✅）：
共 159 测试全部通过（16 个文件）。 |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a “design mode” for dashboard preview selection so users can click widgets in the rendered dashboard preview to select/deselect them and keep selection in sync with the right-side editor panel.
Changes:
- Add design-mode selection props to
DashboardRendererand implement click + keyboard selection/deselection affordances. - Add external (controlled) widget selection props to
DashboardEditorand wire selection sync inDashboardView. - Add unit/integration tests covering renderer/editor/view selection sync, and update
ROADMAP.md.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-designer/src/tests/DashboardEditor.externalSelection.test.tsx | Adds tests for externally controlled widget selection in the editor. |
| packages/plugin-designer/src/DashboardEditor.tsx | Adds selectedWidgetId/onWidgetSelect props and controlled/uncontrolled selection behavior. |
| packages/plugin-dashboard/src/tests/DashboardRenderer.designMode.test.tsx | Adds tests for design-mode selection, keyboard nav, and deselection behaviors. |
| packages/plugin-dashboard/src/DashboardRenderer.tsx | Implements design-mode widget wrappers, selection styling, click-to-select/deselect, and keyboard navigation. |
| apps/console/src/components/DashboardView.tsx | Shares selectedWidgetId state between preview renderer and editor drawer to sync selection. |
| apps/console/src/tests/DashboardViewSelection.test.tsx | Integration tests for end-to-end selection sync between preview/editor and autosave behavior. |
| ROADMAP.md | Documents Phase 6 “Design Mode Preview Click-to-Select”. |
| const designModeProps = designMode ? { | ||
| 'data-testid': `dashboard-preview-widget-${widget.id}`, | ||
| 'data-widget-id': widget.id, | ||
| role: 'button' as const, | ||
| tabIndex: 0, |
There was a problem hiding this comment.
DashboardWidgetSchema.id is optional, but design mode props are built unconditionally from widget.id (test id, data-widget-id, role/tabIndex, handlers). If id is missing, the element becomes focusable and labeled but can’t be selected (handleWidgetClick returns when widgetId is falsy). Consider only applying the interactive design-mode props when widget.id is present (or ensuring widgets always have stable ids before enabling selection).
| <div ref={ref} className={cn("flex flex-col gap-4 px-4", className)} data-user-actions={userActionsAttr} onClick={handleBackgroundClick} {...props}> | ||
| {headerSection} |
There was a problem hiding this comment.
onClick={handleBackgroundClick} is set before {...props} on the mobile root container, so a consumer-provided onClick in props will override the background deselect handler and break click-to-deselect. Consider spreading props before attaching onClick, or composing the handlers so both run.
| }} | ||
| data-user-actions={userActionsAttr} | ||
| onClick={handleBackgroundClick} | ||
| {...props} | ||
| > |
There was a problem hiding this comment.
onClick={handleBackgroundClick} is set before {...props} on the desktop grid container, so a consumer-provided onClick in props will override the background deselect handler and break click-to-deselect. Consider spreading props before attaching onClick, or composing the handlers so both run.
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| onWidgetClick(widgetId ?? null); | ||
| } else if (e.key === 'ArrowRight' || e.key === 'ArrowDown') { | ||
| e.preventDefault(); | ||
| const next = index + 1 < widgets.length ? widgets[index + 1] : null; | ||
| if (next?.id) onWidgetClick(next.id); | ||
| } else if (e.key === 'ArrowLeft' || e.key === 'ArrowUp') { | ||
| e.preventDefault(); | ||
| const prev = index - 1 >= 0 ? widgets[index - 1] : null; |
There was a problem hiding this comment.
Arrow-key navigation uses schema.widgets[index ± 1], but index comes from the render loop and is relative to the list being mapped. In the mobile layout, widgets are mapped from filtered subsets (metrics vs others), so index won’t match the position in schema.widgets and navigation can jump to the wrong widget. Consider deriving the current index from widgetId (e.g., findIndex) before computing next/prev.
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onWidgetClick(widgetId ?? null); | |
| } else if (e.key === 'ArrowRight' || e.key === 'ArrowDown') { | |
| e.preventDefault(); | |
| const next = index + 1 < widgets.length ? widgets[index + 1] : null; | |
| if (next?.id) onWidgetClick(next.id); | |
| } else if (e.key === 'ArrowLeft' || e.key === 'ArrowUp') { | |
| e.preventDefault(); | |
| const prev = index - 1 >= 0 ? widgets[index - 1] : null; | |
| // Derive the position from widgetId to avoid relying on subset indices | |
| const currentIndex = | |
| widgetId != null ? widgets.findIndex((w) => w.id === widgetId) : -1; | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onWidgetClick(widgetId ?? null); | |
| } else if (e.key === 'ArrowRight' || e.key === 'ArrowDown') { | |
| e.preventDefault(); | |
| if (currentIndex === -1) return; | |
| const nextIndex = currentIndex + 1; | |
| const next = nextIndex < widgets.length ? widgets[nextIndex] : null; | |
| if (next?.id) onWidgetClick(next.id); | |
| } else if (e.key === 'ArrowLeft' || e.key === 'ArrowUp') { | |
| e.preventDefault(); | |
| if (currentIndex === -1) return; | |
| const prevIndex = currentIndex - 1; | |
| const prev = prevIndex >= 0 ? widgets[prevIndex] : null; |
| 'data-widget-id': widget.id, | ||
| role: 'button' as const, | ||
| tabIndex: 0, | ||
| 'aria-selected': isSelected, |
There was a problem hiding this comment.
Design mode renders widgets with role="button" but also sets aria-selected. aria-selected isn’t valid for the button role and may be ignored/flagged by a11y tooling. Consider using a semantic <button> element, or switch to aria-pressed, or adopt listbox/option roles if you want to keep aria-selected.
| 'aria-selected': isSelected, | |
| 'aria-pressed': isSelected, |
selectedWidgetId,onWidgetClick, anddesignModeprops toDashboardRendererDashboardRenderer(primary ring, light/dark theme, a11y)selectedWidgetIdandonWidgetSelectprops toDashboardEditorfor external controlDashboardViewto sync selection betweenDashboardRendererandDashboardEditoraddWidget/removeWidget(missingsetSelectedWidgetIdin useCallback deps)DashboardRendererdesign mode (widget selection, keyboard nav, a11y, deselection)DashboardEditorexternal selection control (including live preview path, add/remove sync)DashboardViewend-to-end selection sync (preview→editor→edit→preview→save, drawer close clears selection)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.