Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add useDesignerHistory hook (command pattern wrapper) - ViewDesigner: Ctrl+S/Cmd+S save shortcut, field type selector, width validation (min/max) - DataModelDesigner: inline entity label editing, field type selector dropdown - Wire useDesignerHistory to both designers - Add comprehensive tests (63 total, 33 new) - Update ROADMAP.md with completed P1.1 items Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…lify width validation, improve test assertions Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implements the P1.1 “Designer Interaction” roadmap items for ViewDesigner and DataModelDesigner by adding shared undo/redo wiring (useDesignerHistory), new UX interactions (keyboard save, inline label editing, type selectors), plus expanded test coverage and a roadmap update.
Changes:
- Added
useDesignerHistoryhook as a designer-specific wrapper arounduseUndoRedo, and exported it from public entrypoints. - Enhanced
ViewDesignerwith Ctrl/Cmd+S save handling, a read-only field type display, and column width validation rules. - Enhanced
DataModelDesignerwith inline entity label editing and a field type dropdown, plus new/updated tests and ROADMAP.md checklist updates.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-designer/src/index.tsx | Re-export useDesignerHistory from the package entrypoint. |
| packages/plugin-designer/src/hooks/useDesignerHistory.ts | New hook wrapping useUndoRedo with designer-specific naming/types. |
| packages/plugin-designer/src/hooks/index.ts | Adds barrel exports for useDesignerHistory + types. |
| packages/plugin-designer/src/tests/useDesignerHistory.test.ts | New tests validating undo/redo wrapper behavior. |
| packages/plugin-designer/src/tests/ViewDesigner.test.tsx | New tests for Ctrl+S, field type display, and width validation behavior. |
| packages/plugin-designer/src/tests/DataModelDesigner.test.tsx | New test suite covering inline label editing, type dropdown, and undo/redo. |
| packages/plugin-designer/src/ViewDesigner.tsx | Wires useDesignerHistory, adds Ctrl/Cmd+S, adds field type display, and updates width handling. |
| packages/plugin-designer/src/DataModelDesigner.tsx | Wires useDesignerHistory, adds inline entity label editing and a field type dropdown. |
| ROADMAP.md | Marks most P1.1 checklist items complete and updates the “Designer Interaction” status text. |
| columns: columns.map((c, i) => (i === idx ? { ...c, width: val === 'auto' ? 'auto' : undefined } : c)), | ||
| }); | ||
| return; | ||
| } | ||
| if (/^\d+$/.test(val)) { | ||
| const num = Number(val); | ||
| const clamped = Math.max(COLUMN_WIDTH_MIN, Math.min(COLUMN_WIDTH_MAX, num)); | ||
| pushState({ | ||
| columns: columns.map((c, i) => (i === idx ? { ...c, width: clamped } : c)), | ||
| }); | ||
| return; | ||
| } | ||
| // Reject non-numeric, non-auto values | ||
| }} |
There was a problem hiding this comment.
Column width is clamped on every keystroke. Because the input is controlled, typing a value that starts below the min (e.g., trying to enter "100" by typing "1") will immediately clamp to 50 and makes it impossible to enter many valid multi-digit widths. Consider keeping a local string state for the width input and only validating/clamping on blur or on save (and/or allow intermediate values < min while editing).
| columns: columns.map((c, i) => (i === idx ? { ...c, width: val === 'auto' ? 'auto' : undefined } : c)), | |
| }); | |
| return; | |
| } | |
| if (/^\d+$/.test(val)) { | |
| const num = Number(val); | |
| const clamped = Math.max(COLUMN_WIDTH_MIN, Math.min(COLUMN_WIDTH_MAX, num)); | |
| pushState({ | |
| columns: columns.map((c, i) => (i === idx ? { ...c, width: clamped } : c)), | |
| }); | |
| return; | |
| } | |
| // Reject non-numeric, non-auto values | |
| }} | |
| columns: columns.map((c, i) => | |
| i === idx ? { ...c, width: val === 'auto' ? 'auto' : undefined } : c, | |
| ), | |
| }); | |
| return; | |
| } | |
| if (/^\d+$/.test(val)) { | |
| const num = Number(val); | |
| // Allow intermediate numeric values without clamping on each keystroke. | |
| pushState({ | |
| columns: columns.map((c, i) => (i === idx ? { ...c, width: num } : c)), | |
| }); | |
| return; | |
| } | |
| // Reject non-numeric, non-auto values | |
| }} | |
| onBlur={() => { | |
| if (readOnly) return; | |
| const idx = selectedColumnIndex; | |
| const current = columns[idx]?.width; | |
| if (typeof current === 'number') { | |
| const clamped = Math.max(COLUMN_WIDTH_MIN, Math.min(COLUMN_WIDTH_MAX, current)); | |
| if (clamped !== current) { | |
| pushState({ | |
| columns: columns.map((c, i) => (i === idx ? { ...c, width: clamped } : c)), | |
| }); | |
| } | |
| } | |
| }} |
| if (ctrl && e.key === 's' && !readOnly) { | ||
| e.preventDefault(); | ||
| handleSave(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Ctrl/Cmd+S handling checks e.key === 's' only. With Caps Lock or some layouts, KeyboardEvent.key can be uppercase ('S'), so the shortcut may not fire reliably. Consider normalizing via e.key.toLowerCase() === 's' (or checking code === 'KeyS') before triggering save.
| }} | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="text-xs text-muted-foreground ml-auto bg-transparent border-none focus:ring-1 focus:ring-primary rounded cursor-pointer p-0" | ||
| data-testid={`field-type-${entity.id}-${fieldIndex}`} |
There was a problem hiding this comment.
The field type has no associated label (it’s visually inline), so screen readers won’t have an accessible name for the control. Add an aria-label/aria-labelledby (e.g., include the field name) so the dropdown is discoverable and understandable via assistive tech. data-testid={`field-type-${entity.id}-${fieldIndex}`} aria-label={`Field type for ${field.name}`}
Implements the P1.1 Designer Interaction roadmap items for ViewDesigner and DataModelDesigner, completing 9/10 checklist items (dnd-kit column reorder deferred).
Shared Infrastructure
useDesignerHistoryhook — command-pattern wrapper overuseUndoRedo, wired into both designersViewDesigner
onSaveDESIGNER_FIELD_TYPESconstant (18 types) displayed as read-only selector in column properties panel"auto", rejects non-numeric/non-auto inputDataModelDesigner
<span>display; read-only mode falls back to textTests
33 new tests across 3 files (63 total in plugin-designer):
useDesignerHistory(7),DataModelDesigner(19), ViewDesigner new features (7).ROADMAP.md
Updated P1.1 checklist — remaining item: column drag-to-reorder via
@dnd-kit/core.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.