-
Notifications
You must be signed in to change notification settings - Fork 2
Add cut/duplicate, tree navigation, and keyboard shortcuts to designer #45
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
02ecf78
Initial plan
Copilot 2ac7812
Add expand/collapse all, cut, and duplicate functionality to designer
Copilot 1addb98
Add move up/down functionality and update documentation
Copilot 2cea64b
Add keyboard navigation and shortcuts help dialog
Copilot c54506c
Fix duplicate function and add comprehensive keyboard shortcuts tests
Copilot 3679594
Fix code review issues: deep clone, expansion logic, unused imports, …
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
206 changes: 206 additions & 0 deletions
206
packages/designer/src/__tests__/keyboard-shortcuts.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| import { describe, it, expect, beforeEach } from 'vitest'; | ||
| import { render, fireEvent } from '@testing-library/react'; | ||
| import { DesignerProvider, useDesigner } from '../context/DesignerContext'; | ||
| import { SchemaNode } from '@object-ui/core'; | ||
| import React from 'react'; | ||
|
|
||
| // Test component to access designer context | ||
| const TestComponent = () => { | ||
| const { | ||
| schema, | ||
| selectedNodeId, | ||
| setSelectedNodeId, | ||
| copyNode, | ||
| cutNode, | ||
| duplicateNode, | ||
| pasteNode, | ||
| moveNodeUp, | ||
| moveNodeDown, | ||
| canPaste | ||
| } = useDesigner(); | ||
|
|
||
| return ( | ||
| <div> | ||
| <div data-testid="schema">{JSON.stringify(schema)}</div> | ||
| <div data-testid="selected">{selectedNodeId || 'none'}</div> | ||
| <div data-testid="can-paste">{canPaste ? 'yes' : 'no'}</div> | ||
| <button onClick={() => setSelectedNodeId('child-1')} data-testid="select-child-1">Select Child 1</button> | ||
| <button onClick={() => copyNode('child-1')} data-testid="copy">Copy</button> | ||
| <button onClick={() => cutNode('child-1')} data-testid="cut">Cut</button> | ||
| <button onClick={() => duplicateNode('child-1')} data-testid="duplicate">Duplicate</button> | ||
| <button onClick={() => pasteNode('root')} data-testid="paste">Paste</button> | ||
| <button onClick={() => moveNodeUp('child-2')} data-testid="move-up">Move Up</button> | ||
| <button onClick={() => moveNodeDown('child-1')} data-testid="move-down">Move Down</button> | ||
| <button onClick={() => moveNodeUp('child-1')} data-testid="move-up-first">Move Up First</button> | ||
| <button onClick={() => moveNodeDown('child-3')} data-testid="move-down-last">Move Down Last</button> | ||
| <button onClick={() => cutNode('nonexistent-id')} data-testid="cut-nonexistent">Cut Nonexistent</button> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| describe('Keyboard Shortcuts and Navigation', () => { | ||
| const initialSchema: SchemaNode = { | ||
| type: 'div', | ||
| id: 'root', | ||
| body: [ | ||
| { type: 'text', id: 'child-1', content: 'First' }, | ||
| { type: 'text', id: 'child-2', content: 'Second' }, | ||
| { type: 'text', id: 'child-3', content: 'Third' } | ||
| ] | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| // Reset any global state if needed | ||
| }); | ||
|
|
||
| it('should copy a node', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| expect(getByTestId('can-paste').textContent).toBe('no'); | ||
|
|
||
| fireEvent.click(getByTestId('copy')); | ||
|
|
||
| expect(getByTestId('can-paste').textContent).toBe('yes'); | ||
| }); | ||
|
|
||
| it('should cut a node and allow paste', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| expect(getByTestId('can-paste').textContent).toBe('no'); | ||
|
|
||
| fireEvent.click(getByTestId('cut')); | ||
|
|
||
| // Should be able to paste after cut | ||
| expect(getByTestId('can-paste').textContent).toBe('yes'); | ||
|
|
||
| // The schema should have the node removed | ||
| const schema = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| expect(schema.body).toHaveLength(2); // One node was cut | ||
| }); | ||
|
|
||
| it('should duplicate a node', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| const initialBody = JSON.parse(getByTestId('schema').textContent || '{}').body; | ||
| const initialLength = initialBody.length; | ||
|
|
||
| fireEvent.click(getByTestId('duplicate')); | ||
|
|
||
| // Schema should have an extra node | ||
| const schema = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| expect(schema.body.length).toBe(initialLength + 1); | ||
|
|
||
| // The duplicated node should have the same type as the original | ||
| expect(schema.body[1].type).toBe('text'); | ||
| expect(schema.body[1].content).toBe('First'); | ||
|
|
||
| // The duplicated node should be positioned right after the original (at index 1) | ||
| expect(schema.body[1].id).not.toBe('child-1'); // Should have a new ID | ||
| }); | ||
|
|
||
| it('should handle moving a node up when already at top', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| const schemaBeforeMove = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
|
|
||
| // Try to move the first node up (should do nothing) | ||
| fireEvent.click(getByTestId('move-up-first')); | ||
|
|
||
| const schemaAfterMove = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| // Schema should remain unchanged | ||
| expect(schemaAfterMove.body[0].id).toBe(schemaBeforeMove.body[0].id); | ||
| }); | ||
|
|
||
| it('should handle moving a node down when already at bottom', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| const schemaBeforeMove = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
|
|
||
| // Try to move the last node down (should do nothing) | ||
| fireEvent.click(getByTestId('move-down-last')); | ||
|
|
||
| const schemaAfterMove = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| // Schema should remain unchanged | ||
| expect(schemaAfterMove.body[2].id).toBe(schemaBeforeMove.body[2].id); | ||
| }); | ||
|
|
||
| it('should handle cutting a non-existent node gracefully', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| const initialLength = JSON.parse(getByTestId('schema').textContent || '{}').body.length; | ||
|
|
||
| // Try to cut a node that doesn't exist | ||
| fireEvent.click(getByTestId('cut-nonexistent')); | ||
|
|
||
| const schema = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| // Schema should remain unchanged | ||
| expect(schema.body.length).toBe(initialLength); | ||
| }); | ||
|
|
||
| it('should paste a copied node', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| fireEvent.click(getByTestId('copy')); | ||
| fireEvent.click(getByTestId('paste')); | ||
|
|
||
| // Schema should have an extra node | ||
| const schema = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| expect(schema.body.length).toBeGreaterThan(3); | ||
| }); | ||
|
|
||
| it('should move a node up', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| fireEvent.click(getByTestId('move-up')); | ||
|
|
||
| const schema = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| // child-2 should now be at index 0 after moving up | ||
| expect(schema.body[0].id).toBe('child-2'); | ||
| }); | ||
|
|
||
| it('should move a node down', () => { | ||
| const { getByTestId } = render( | ||
| <DesignerProvider initialSchema={initialSchema}> | ||
| <TestComponent /> | ||
| </DesignerProvider> | ||
| ); | ||
|
|
||
| fireEvent.click(getByTestId('move-down')); | ||
|
|
||
| const schema = JSON.parse(getByTestId('schema').textContent || '{}'); | ||
| // child-1 should now be at index 1 after moving down | ||
| expect(schema.body[1].id).toBe('child-1'); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 test file is missing test coverage for edge cases such as attempting to move a node up when it's already at the top position, moving down when at the bottom, or cutting/duplicating a node that doesn't exist. These boundary conditions should be tested to ensure the functions handle them gracefully.
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.
Added in commit 3679594. Implemented 3 new edge case tests: moving up when already at top, moving down when already at bottom, and cutting a non-existent node. All tests pass successfully.