feat(wikibase-schema-editor): create drag-and-drop context types and utilities #70
Conversation
…utilities Addresses Task 2 of #66
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change introduces a new drag-and-drop context composable ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VueComponent
participant useDragDropContext
participant DropZone
User->>VueComponent: Initiates drag on column
VueComponent->>useDragDropContext: startDrag(column)
useDragDropContext-->>VueComponent: Updates drag state
User->>DropZone: Drags over drop zone
DropZone->>useDragDropContext: enterDropZone(target)
useDragDropContext-->>DropZone: Updates hover state
User->>DropZone: Drops column
DropZone->>useDragDropContext: performDrop(column, target)
useDragDropContext->>useDragDropContext: validateDrop(column, target)
alt Valid drop
useDragDropContext-->>VueComponent: Updates state, feedback (success)
else Invalid drop
useDragDropContext-->>VueComponent: Updates state, feedback (error/suggestion)
end
User->>VueComponent: Ends drag
VueComponent->>useDragDropContext: endDrag()
useDragDropContext-->>VueComponent: Resets drag state
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
.kiro/steering/tech.md (1)
30-32: Clarify exceptions to the “arrow-functions-only” ruleMandating arrow functions everywhere may break:
- Hoisting use-cases (e.g. helper declared below its usage in the same module).
- Context-bound callbacks that rely on their own
this.- Vue macros that expect a plain
function(e.g.defineStore({ actions: { foo() { … } } })).Consider amending the guideline with explicit exceptions (e.g. Vue options API, callbacks requiring
this, test hooks, etc.) so the rule does not force awkward work-arounds.frontend/src/types/schema-mapping.ts (1)
1-1: Consolidated import is fine but keep alphabetic orderMinor nit: To stay consistent with other files, keep grouped/combined imports sorted:
-import type { ItemId, PropertyId } from '@backend/types/wikibase-schema' +import type { ItemId, PropertyId } from '@backend/types/wikibase-schema'(if other symbols are added later, please alphabetise).
frontend/src/types/drag-drop.ts (1)
53-59: DropTargetType still misses “reference-value”?If future drop zones distinguish between reference objects vs. their values, you may later need:
| 'reference-value'Flagging now in case the omission was accidental.
frontend/src/composables/__tests__/useDragDropContext.test.ts (1)
185-200: Consider improving async test reliability.The use of
setTimeoutfor testing async behavior could lead to flaky tests. Consider using more deterministic approaches like mocking the async behavior or using test utilities that wait for specific conditions.- // Wait for async operation to complete - await new Promise((resolve) => setTimeout(resolve, 150)) + // Consider using a more deterministic approach: + // await waitFor(() => expect(context.dragState.value).toBe('idle'))frontend/src/composables/useDragDropContext.ts (1)
324-327: Use optional chaining instead of non-null assertion.The non-null assertion on
event.dataTransfer!could cause runtime errors.onDragOver: (event: DragEvent) => { event.preventDefault() - event.dataTransfer!.dropEffect = 'copy' + if (event.dataTransfer) { + event.dataTransfer.dropEffect = 'copy' + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.kiro/specs/wikibase-schema-editor/tasks.md(1 hunks).kiro/steering/tech.md(1 hunks)frontend/.eslintrc-auto-import.json(2 hunks)frontend/auto-imports.d.ts(4 hunks)frontend/src/composables/__tests__/useDragDropContext.test.ts(1 hunks)frontend/src/composables/useDragDropContext.ts(1 hunks)frontend/src/types/__tests__/drag-drop-context.test.ts(1 hunks)frontend/src/types/__tests__/drag-drop.test.ts(27 hunks)frontend/src/types/__tests__/drop-target-validation.test.ts(1 hunks)frontend/src/types/__tests__/schema-mapping.test.ts(36 hunks)frontend/src/types/drag-drop.ts(2 hunks)frontend/src/types/schema-mapping.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
**/*.{html,ts,css}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
**/*.{html,js,ts,jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
🧬 Code Graph Analysis (6)
frontend/src/types/drag-drop.ts (2)
frontend/auto-imports.d.ts (5)
SchemaDragDropContext(357-357)Ref(345-345)ColumnInfo(360-360)DragState(357-357)DropTargetType(357-357)frontend/src/types/schema-mapping.ts (1)
ColumnInfo(101-107)
frontend/src/types/__tests__/drag-drop.test.ts (3)
frontend/auto-imports.d.ts (8)
Ref(345-345)ComputedRef(345-345)ColumnInfo(360-360)DropTarget(357-357)DragDropContext(357-357)DragEventData(357-357)DropEventData(357-357)DragVisualState(357-357)frontend/src/types/schema-mapping.ts (1)
ColumnInfo(101-107)frontend/src/types/drag-drop.ts (5)
DropTarget(44-51)DragDropContext(37-42)DragEventData(72-76)DropEventData(79-84)DragVisualState(87-93)
frontend/src/types/__tests__/schema-mapping.test.ts (2)
frontend/auto-imports.d.ts (13)
ColumnMapping(360-360)ValueMapping(360-360)QualifierSchemaMapping(360-360)ReferenceSchemaMapping(360-360)StatementSchemaMapping(360-360)ValidationError(360-360)ValidationResult(360-360)TransformationParameter(360-360)ValidationRule(360-360)SchemaMapping(360-360)TransformationFunction(360-360)ValueSchemaMapping(360-360)ValidatedSchemaMapping(360-360)frontend/src/types/schema-mapping.ts (13)
ColumnMapping(26-30)ValueMapping(69-73)QualifierSchemaMapping(75-78)ReferenceSchemaMapping(80-83)StatementSchemaMapping(54-61)ValidationError(110-116)ValidationResult(118-122)TransformationParameter(46-52)ValidationRule(133-133)SchemaMapping(136-140)TransformationFunction(39-44)ValueSchemaMapping(148-152)ValidatedSchemaMapping(155-158)
frontend/src/composables/__tests__/useDragDropContext.test.ts (3)
frontend/auto-imports.d.ts (3)
ColumnInfo(360-360)DropTarget(357-357)WikibaseDataType(360-360)frontend/src/types/schema-mapping.ts (2)
ColumnInfo(101-107)WikibaseDataType(88-98)frontend/src/types/drag-drop.ts (1)
DropTarget(44-51)
frontend/src/composables/useDragDropContext.ts (3)
frontend/auto-imports.d.ts (8)
SchemaDragDropContext(357-357)ColumnInfo(360-360)DropTarget(357-357)DragState(357-357)DragEventData(357-357)DropEventData(357-357)WikibaseDataType(360-360)DropZoneConfig(357-357)frontend/src/types/drag-drop.ts (6)
SchemaDragDropContext(5-18)DropTarget(44-51)DragState(62-62)DragEventData(72-76)DropEventData(79-84)DropZoneConfig(21-28)frontend/src/types/schema-mapping.ts (2)
ColumnInfo(101-107)WikibaseDataType(88-98)
frontend/src/types/__tests__/drag-drop-context.test.ts (2)
frontend/src/types/schema-mapping.ts (1)
ColumnInfo(101-107)frontend/src/types/drag-drop.ts (6)
DropTarget(44-51)DragState(62-62)SchemaDragDropContext(5-18)DragEventData(72-76)DropEventData(79-84)DragVisualState(87-93)
🔇 Additional comments (19)
.kiro/specs/wikibase-schema-editor/tasks.md (1)
9-14: Task checkbox update looks goodThe checklist correctly reflects the newly implemented drag-and-drop utilities and associated tests.
No action required.frontend/src/types/schema-mapping.ts (1)
88-96: Union type re-formatting OKOnly formatting changed; no semantic impact.
LGTM.frontend/src/types/drag-drop.ts (2)
1-3: Path alias check
@frontend/types/schema-mappingmust be resolvable intsconfig.paths.
If this alias isn’t already present, add it to avoid IDE lint errors.
9-18: UnuseddragStateimport?
dragStateis typed asRef<DragState>butDragStateis declared later in the same file.
That’s fine, yet make sure there is no separateDragStateexport causing shadowing.frontend/.eslintrc-auto-import.json (2)
76-76: ESLint auto-import entry added
createDropZoneConfigregistered – good.
233-233: ESLint auto-import entry added
useDragDropContextregistered – good.frontend/src/types/__tests__/drag-drop.test.ts (2)
14-16: LGTM! Import path improvements enhance maintainability.The switch to absolute imports using
@frontend/typesis a good practice that makes the codebase more maintainable and consistent with the project's structure.
22-23: LGTM! Consistent formatting improvements.The addition of trailing commas and parentheses around arrow function parameters improves code consistency and follows modern JavaScript/TypeScript best practices.
Also applies to: 29-30, 36-37, 48-49, 72-73
frontend/src/types/__tests__/schema-mapping.test.ts (2)
24-25: LGTM! Consistent import path improvements.The switch to absolute imports using
@frontend/types/schema-mappingmaintains consistency with the broader codebase refactoring and improves maintainability.
40-43: LGTM! Comprehensive formatting consistency.The formatting improvements including trailing commas, parentheses around arrow function parameters, and consistent spacing align with the coding standards mentioned in the PR objectives.
Also applies to: 67-68, 84-85, 121-122, 139-140
frontend/auto-imports.d.ts (2)
27-27: LGTM! Auto-generated declarations for new drag-drop functionality.The auto-generated declarations for
createDropZoneConfiganduseDragDropContextcorrectly follow the existing pattern and enable seamless auto-import usage in Vue components.Also applies to: 193-193
387-387: LGTM! Vue component custom properties properly declared.The corresponding Vue component custom properties are correctly declared, ensuring proper TypeScript support in Vue templates.
Also applies to: 544-544
frontend/src/composables/__tests__/useDragDropContext.test.ts (6)
1-5: LGTM! Proper test framework and import usage.The test correctly uses
bun:testas specified in the coding guidelines and imports the necessary types and functions using absolute paths.
10-26: LGTM! Well-structured test setup.The
beforeEachsetup properly initializes mock data with realistic values, ensuring consistent test conditions across all test cases.
28-93: LGTM! Comprehensive state management testing.The test suite thoroughly covers all aspects of the drag-drop context state management including initialization, drag start/end, and drop zone enter/leave scenarios.
95-183: LGTM! Thorough validation testing.The validation tests cover all important scenarios including data type compatibility, nullable column handling, property ID requirements, and label length constraints with appropriate error messages and suggestions.
218-286: LGTM! Excellent computed property testing.The tests for valid targets computation and computed properties provide comprehensive coverage of the reactive behavior, ensuring the composable correctly updates when state changes.
288-372: LGTM! Complete drop zone configuration testing.The
createDropZoneConfigtests thoroughly validate the helper function's ability to create proper drop zone configurations, handle events, and validate drop data correctly.frontend/src/types/__tests__/drag-drop-context.test.ts (1)
1-324: Comprehensive test coverage for drag-drop types!The test suite thoroughly validates all drag-drop related types and their expected behaviors. Good use of
bun:testas per the coding guidelines.
| // Helper functions for validation (these would be implemented in the actual utilities) | ||
| const validateDataTypeCompatibility = (column: ColumnInfo, target: DropTarget): boolean => { | ||
| const compatibilityMap: Record<string, WikibaseDataType[]> = { | ||
| VARCHAR: ['string', 'url', 'external-id'], | ||
| TEXT: ['string', 'monolingualtext'], | ||
| STRING: ['string', 'url', 'external-id'], | ||
| INTEGER: ['quantity'], | ||
| DECIMAL: ['quantity'], | ||
| NUMERIC: ['quantity'], | ||
| FLOAT: ['quantity'], | ||
| DATE: ['time'], | ||
| DATETIME: ['time'], | ||
| TIMESTAMP: ['time'], | ||
| BOOLEAN: [], | ||
| } | ||
|
|
||
| const compatibleTypes = compatibilityMap[column.dataType.toUpperCase()] || [] | ||
| return target.acceptedTypes.some((type) => compatibleTypes.includes(type)) | ||
| } | ||
|
|
||
| const validateTargetPath = (path: string): boolean => { | ||
| if (!path || path.trim() === '') return false | ||
|
|
||
| const validPatterns = [ | ||
| /^item\.terms\.(labels|descriptions)\.[a-z]{2,3}$/, | ||
| /^item\.terms\.aliases\.[a-z]{2,3}\[\d+\]$/, | ||
| /^item\.statements\[\d+\]\.value$/, | ||
| /^item\.statements\[\d+\]\.qualifiers\[\d+\]\.value$/, | ||
| /^item\.statements\[\d+\]\.references\[\d+\]\.value$/, | ||
| ] | ||
|
|
||
| return validPatterns.some((pattern) => pattern.test(path)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider importing actual validation functions instead of duplicating logic.
The helper functions validateDataTypeCompatibility and validateTargetPath duplicate logic that should exist in the actual implementation. This can lead to tests passing even if the actual implementation changes.
Import the actual functions from the composable:
+import { validateDataTypeCompatibility, validateTargetPath } from '@frontend/composables/useDragDropContext'
-// Helper functions for validation (these would be implemented in the actual utilities)
-const validateDataTypeCompatibility = (column: ColumnInfo, target: DropTarget): boolean => {
- const compatibilityMap: Record<string, WikibaseDataType[]> = {
- VARCHAR: ['string', 'url', 'external-id'],
- TEXT: ['string', 'monolingualtext'],
- STRING: ['string', 'url', 'external-id'],
- INTEGER: ['quantity'],
- DECIMAL: ['quantity'],
- NUMERIC: ['quantity'],
- FLOAT: ['quantity'],
- DATE: ['time'],
- DATETIME: ['time'],
- TIMESTAMP: ['time'],
- BOOLEAN: [],
- }
-
- const compatibleTypes = compatibilityMap[column.dataType.toUpperCase()] || []
- return target.acceptedTypes.some((type) => compatibleTypes.includes(type))
-}
-
-const validateTargetPath = (path: string): boolean => {
- if (!path || path.trim() === '') return false
-
- const validPatterns = [
- /^item\.terms\.(labels|descriptions)\.[a-z]{2,3}$/,
- /^item\.terms\.aliases\.[a-z]{2,3}\[\d+\]$/,
- /^item\.statements\[\d+\]\.value$/,
- /^item\.statements\[\d+\]\.qualifiers\[\d+\]\.value$/,
- /^item\.statements\[\d+\]\.references\[\d+\]\.value$/,
- ]
-
- return validPatterns.some((pattern) => pattern.test(path))
-}If these functions don't exist yet, add a TODO comment:
// TODO: Import these functions once implemented in the composable🤖 Prompt for AI Agents
In frontend/src/types/__tests__/drop-target-validation.test.ts around lines 369
to 401, the helper functions validateDataTypeCompatibility and
validateTargetPath duplicate logic that should be imported from the actual
implementation to ensure test accuracy. Replace these local helper functions by
importing the real validation functions from the composable module. If these
functions are not yet implemented in the composable, add a TODO comment
indicating to import them once available.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/composables/useDragDropContext.ts (1)
223-229: Consider replacing setTimeout with more explicit async simulation.While the current implementation works, consider making the async simulation more explicit or adding a configuration option for the delay in testing scenarios.
- await new Promise<void>((resolve) => { - // This could be replaced with actual async operations like: - // - API calls to save the mapping - // - Store actions to update the schema - // - Event emission to parent components - setTimeout(resolve, 100) - }) + await new Promise<void>((resolve) => { + // This could be replaced with actual async operations like: + // - API calls to save the mapping + // - Store actions to update the schema + // - Event emission to parent components + setTimeout(resolve, process.env.NODE_ENV === 'test' ? 0 : 100) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/composables/__tests__/useDragDropContext.test.ts(1 hunks)frontend/src/composables/useDragDropContext.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/composables/tests/useDragDropContext.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
**/*.{html,ts,css}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
**/*.{html,js,ts,jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
🔇 Additional comments (15)
frontend/src/composables/useDragDropContext.ts (15)
1-11: Import structure looks good and follows TypeScript best practices.The imports are well-organized with proper type-only imports where appropriate, and the type definitions are comprehensive for the drag-and-drop functionality.
13-33: Path parsing utilities are well-structured with clear intent.The helper functions for extracting target types, languages, and property IDs from paths are well-implemented with appropriate regex patterns and fallback handling.
35-55: Data type compatibility mapping is comprehensive and well-documented.The compatibility map covers all major SQL data types and their corresponding Wikibase types, including proper handling of serializable types like JSON and ARRAY.
60-83: Composable function signature and state management are well-designed.The composable follows Vue 3 composition API best practices with proper TypeScript typing and comprehensive reactive state management. The WeakMap for listener cleanup is a good approach to prevent memory leaks.
84-95: Computed properties provide efficient reactive validation.The computed properties for
validDropTargetsandisValidDropare efficiently implemented and will only recompute when their dependencies change.
98-121: Event listener management addresses previous memory leak concerns.The implementation properly handles event listener cleanup using WeakMap storage and removes existing listeners before adding new ones, which addresses the memory leak issue from previous reviews.
123-129: Clean drag state reset implementation.The
endDragfunction properly resets all drag-related state, ensuring no stale references remain.
131-154: Drop zone interaction handling with proper feedback.The enter/leave drop zone methods provide appropriate visual feedback and validation updates, enhancing user experience.
156-187: Comprehensive validation logic with detailed error messages.The validation function covers data type compatibility, nullability constraints, and target-specific validation with helpful error messages and suggestions.
189-257: Async drop handling with proper error management.The
performDropfunction correctly handles async operations with proper error handling and state management. The try-catch-finally structure ensures proper cleanup regardless of success or failure.
264-299: Smart suggestion generation enhances user experience.The suggestion system provides helpful guidance for data type conversions and alternative mapping targets, improving the overall user experience.
301-345: Target-specific validation with appropriate business rules.The validation logic correctly handles different target types with appropriate constraints (e.g., length limits for labels, property ID requirements for statements).
352-372: Clean composable return structure.The return object properly exposes all necessary reactive properties and methods with clear organization.
377-431: Well-structured HTML5 drag-and-drop configuration helper.The
createDropZoneConfigfunction provides a clean abstraction for HTML5 drag-and-drop events with proper error handling and validation.
422-429: Validate drop method handles JSON parsing gracefully.The validation method properly handles JSON parsing errors and returns appropriate boolean values.
1 participant
Addresses Task 2 of #66