refactor(wikibase-schema-editor): consolidate drop zone functionality #134
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughRemoves centralized drag-and-drop composables, related tests, and global auto-import/type entries. Deletes SchemaDropZone and adds a reusable DropZone component. Updates TermSection and ValueMappingEditor to use DropZone with local validators/handlers. Adds DropZone global typing and minor docs cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DropZone
participant DragDropStore
participant Compatibility as DataTypeCompatibility
participant Consumer as TermSection/ValueMappingEditor
participant SchemaStore
User->>DropZone: Drag over / enter with column
DropZone->>DragDropStore: Read dragged column
DropZone->>Compatibility: Check isDataTypeCompatible(acceptedTypes)
Compatibility-->>DropZone: Valid / Invalid
User-->>DropZone: Drop
DropZone->>DropZone: Parse dataTransfer column JSON
DropZone->>Consumer: emit "column-dropped" (ColumnInfo)
Consumer->>SchemaStore: Update mappings (term/value context)
User->>DropZone: Click clear
DropZone->>Consumer: emit "clear-selection"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
🔭 Outside diff range comments (1)
frontend/components.d.ts (1)
43-43: Remove stale SchemaDropZone global type to avoid broken importsThe generated file frontend/components.d.ts still references SchemaDropZone.vue, which is not present in the repository — this will break TypeScript with a "cannot find module" error. Remove the stale entry or regenerate the file.
- Location to fix:
- frontend/components.d.ts — line ~43 (SchemaDropZone entry)
Apply this diff:
- SchemaDropZone: typeof import('./src/features/wikibase-schema/components/SchemaDropZone.vue')['default']If components.d.ts is generated by unplugin-vue-components: regenerate the declarations and commit the result (and ensure the generation step runs in CI). Otherwise remove the line manually as shown above.
🧹 Nitpick comments (4)
frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue (1)
102-134: Simplify acceptedTypes and make it responsive to propertyDataTypeThe two branches return the same list. Either keep the broad default intentionally, or improve UX by narrowing when propertyDataType is known so the DropZone can provide accurate drag feedback. Minimal change below narrows when available, else falls back to your current broad set.
Apply this diff:
-// Get accepted types for the drop zone based on property data type -const acceptedTypes = computed((): WikibaseDataType[] => { - if (!props.propertyDataType) { - // Default to all types if no property data type specified - return [ - 'string', - 'url', - 'external-id', - 'wikibase-item', - 'wikibase-property', - 'commonsMedia', - 'globe-coordinate', - 'quantity', - 'time', - 'monolingualtext', - ] - } - - // For specific property data types, we can be more restrictive - // but for now, accept all types and let the validation handle compatibility - return [ - 'string', - 'url', - 'external-id', - 'wikibase-item', - 'wikibase-property', - 'commonsMedia', - 'globe-coordinate', - 'quantity', - 'time', - 'monolingualtext', - ] -}) +// Get accepted types for the drop zone based on property data type +const acceptedTypes = computed((): WikibaseDataType[] => { + return props.propertyDataType + ? [props.propertyDataType] + : [ + 'string', + 'url', + 'external-id', + 'wikibase-item', + 'wikibase-property', + 'commonsMedia', + 'globe-coordinate', + 'quantity', + 'time', + 'monolingualtext', + ] +})frontend/src/features/wikibase-schema/components/DropZone.vue (2)
114-121: Validate the dropped payload rather than the store snapshotOn drop you validate using isValidForDrop (backed by dragDropStore.draggedColumn), but you parse the actual payload into columnInfo. These can drift (e.g., stale store, external drags), causing incorrect acceptance. Validate the parsed payload directly.
Apply this diff:
- try { - const columnInfo = JSON.parse(columnData) as ColumnInfo - const validation = isValidForDrop.value - - // Only proceed if drop validation passes - if (validation.isValid) { - emit('column-dropped', columnInfo) - } + try { + const columnInfo = JSON.parse(columnData) as ColumnInfo + const validation = validateColumnForTarget(columnInfo) + + // Only proceed if drop validation passes + if (validation.isValid) { + emit('column-dropped', columnInfo) + }
131-176: Minor a11y enhancement: add aria attributes and keyboard focusConsider making the drop zone keyboard-focusable and conveying disabled state for assistive tech.
Apply this diff:
<div :data-testid="testId" :class="[ 'grow flex flex-row items-center justify-center border-2 border-dashed border-gray-400 rounded-lg text-center transition-colors', dropZoneClasses, { 'opacity-50 cursor-not-allowed': disabled }, ]" + :aria-disabled="disabled ? 'true' : 'false'" + role="region" + tabindex="0" @dragover="handleDragOver" @dragenter="handleDragEnter" @dragleave="handleDragLeave" @drop="handleDrop" >frontend/src/features/wikibase-schema/components/TermSection.vue (1)
2-13: Prefer explicit interfaces for props per project conventionPer retrieved repo learnings, props/emits should use explicit TypeScript interfaces. Switching from an inline type literal to an interface improves consistency and reusability.
Apply this diff:
-// Props -const props = defineProps<{ - title: string - termType: 'label' | 'description' | 'alias' - icon: string - placeholder: string - testId: string - validationPath: string - values: Label | Alias - sectionIndex: number -}>() +// Props +interface TermSectionProps { + title: string + termType: 'label' | 'description' | 'alias' + icon: string + placeholder: string + testId: string + validationPath: string + values: Label | Alias + sectionIndex: number +} +const props = defineProps<TermSectionProps>()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
frontend/.eslintrc-auto-import.json(0 hunks)frontend/auto-imports.d.ts(0 hunks)frontend/components.d.ts(1 hunks)frontend/src/features/wikibase-schema/components/DropZone.vue(1 hunks)frontend/src/features/wikibase-schema/components/SchemaDropZone.vue(0 hunks)frontend/src/features/wikibase-schema/components/TermSection.vue(5 hunks)frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue(2 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useDragDropValidationIntegration.test.ts(0 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useStatementDropZone.test.ts(0 hunks)frontend/src/features/wikibase-schema/composables/useStatementDropZone.ts(0 hunks)frontend/src/features/wikibase-schema/composables/useTermsEditor.ts(0 hunks)frontend/src/shared/composables/__tests__/useDragDropContext.test.ts(0 hunks)frontend/src/shared/composables/__tests__/useDragDropHandlers.test.ts(0 hunks)frontend/src/shared/composables/useDragDropContext.ts(0 hunks)frontend/src/shared/composables/useDragDropHandlers.ts(0 hunks)frontend/src/shared/composables/useDropZoneStyling.ts(0 hunks)guidelines/reference/ELYSIA_EDEN.md(0 hunks)
💤 Files with no reviewable changes (13)
- frontend/.eslintrc-auto-import.json
- frontend/src/features/wikibase-schema/composables/useTermsEditor.ts
- frontend/src/features/wikibase-schema/composables/tests/useStatementDropZone.test.ts
- frontend/src/features/wikibase-schema/composables/tests/useDragDropValidationIntegration.test.ts
- frontend/src/features/wikibase-schema/components/SchemaDropZone.vue
- guidelines/reference/ELYSIA_EDEN.md
- frontend/src/features/wikibase-schema/composables/useStatementDropZone.ts
- frontend/src/shared/composables/tests/useDragDropContext.test.ts
- frontend/auto-imports.d.ts
- frontend/src/shared/composables/tests/useDragDropHandlers.test.ts
- frontend/src/shared/composables/useDropZoneStyling.ts
- frontend/src/shared/composables/useDragDropHandlers.ts
- frontend/src/shared/composables/useDragDropContext.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript or JavaScript files
Do not use dotenv; Bun automatically loads .env files
UseBun.serve()for HTTP servers and WebSockets instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofws
PreferBun.fileovernode:fs's readFile/writeFile
UseBun.$(e.g.,Bun.$ls``) instead of execa for running shell commands
Files:
frontend/components.d.ts
**/*.{html,ts,tsx,css}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuildfor building HTML, TypeScript, or CSS files
Files:
frontend/components.d.ts
🧠 Learnings (1)
📚 Learning: 2025-07-20T14:13:24.706Z
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Props and emits must use explicit TypeScript interfaces
Applied to files:
frontend/src/features/wikibase-schema/components/TermSection.vue
🪛 GitHub Actions: Typecheck code
frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue
[error] 138-138: TypeScript error TS6133: 'columnInfo' is declared but its value is never read.
🔇 Additional comments (6)
frontend/components.d.ts (1)
25-25: Global DropZone registration looks goodRegistering DropZone in GlobalComponents is correct and aligns with the new reusable component.
frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue (2)
96-101: Column drop handler is clean and respects disabled/type guardsThe guard and delegation to existing selection flow are good. Works well with the new DropZone API.
187-209: Good DropZone integration and state wiringProps are correctly bound for UX (placeholder updates, selectedColumn reflection, validator, and event handling). This should be a smooth replacement for the previous inline DnD handling.
frontend/src/features/wikibase-schema/components/DropZone.vue (1)
61-76: Clear, responsive visual feedback classesThe class computation cleanly distinguishes hover, valid, and invalid states with the store-backed drag context. Looks good.
frontend/src/features/wikibase-schema/components/TermSection.vue (2)
84-125: Validator enforces compatibility, length, and duplicates effectivelyThis is a solid, layered validation approach for term fields. It should give clear feedback and prevent common mistakes.
126-140: Drop handler correctly maps to store per term typeThe mapping object is minimal and correct. Integration with the store methods looks consistent across term types.
frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
frontend/src/features/wikibase-schema/components/DropZone.vue (4)
3-14: Adopt explicit TS interfaces for props (codebase convention)Per the team’s guideline to use explicit interfaces for props in src/**/*.vue, extract a named interface for props.
-const props = defineProps<{ - icon: string - placeholder: string - testId: string - acceptedTypes: WikibaseDataType[] - disabled?: boolean - validator?: (columnInfo: ColumnInfo) => boolean - selectedColumn?: { - name: string - dataType: string - } -}>() +interface DropZoneProps { + icon: string + placeholder: string + testId: string + acceptedTypes: WikibaseDataType[] + disabled?: boolean + validator?: (columnInfo: ColumnInfo) => boolean + selectedColumn?: { + name: string + dataType: string + } +} +const props = defineProps<DropZoneProps>()Using retrieved learnings: props and emits should use explicit TypeScript interfaces in src/**/*.vue.
17-20: Type emits with an explicit interface (consistency with props convention)Mirror the explicit interface style for emits as well to keep typing consistent across the codebase.
-const emit = defineEmits<{ - 'column-dropped': [columnInfo: ColumnInfo] - 'clear-selection': [] -}>() +interface DropZoneEmits { + (e: 'column-dropped', columnInfo: ColumnInfo): void + (e: 'clear-selection'): void +} +const emit = defineEmits<DropZoneEmits>()Using retrieved learnings: props and emits should use explicit TypeScript interfaces in src/**/*.vue.
95-113: Harden drop handler: check MIME type and stop propagationAdd a guard for the custom MIME type and stop propagation to avoid nested drop targets from handling the same event. This reduces accidental drops and noisy error logging.
const handleDrop = (event: DragEvent): void => { if (props.disabled) return - event.preventDefault() + event.preventDefault() + event.stopPropagation() - const columnData = event.dataTransfer?.getData('application/x-column-data') + // Ensure our custom payload is present before reading + const hasColumnData = event.dataTransfer?.types?.includes('application/x-column-data') + if (!hasColumnData) { + isOverDropZone.value = false + return + } + const columnData = event.dataTransfer?.getData('application/x-column-data') if (!columnData) { isOverDropZone.value = false return } try { const columnInfo = JSON.parse(columnData) as ColumnInfo // Only proceed if drop validation passes if (validateColumnForTarget(columnInfo)) { emit('column-dropped', columnInfo) } } catch (error) { console.error('Failed to parse column data:', error) } finally { isOverDropZone.value = false } }
116-162: Optional: add basic accessibility affordancesConsider adding role and aria-disabled to convey state to assistive tech; optionally add tabindex and keyboard handling if you plan to support keyboard-driven “drop” interactions.
Example (no diff as placement may vary):
- Add role="button" and :aria-disabled="disabled"
- Optionally, tabindex="0" for focusability when interactive
frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue (1)
102-119: Optional: centralize allowed Wikibase typesTo keep things DRY, consider exporting a shared constant (e.g., ALL_WIKIBASE_TYPES) from a types or constants module and reusing it here.
frontend/src/features/wikibase-schema/components/TermSection.vue (2)
2-13: Use explicit TS interfaces for props (team convention)Switch from inline defineProps to a named interface for consistency and clearer reuse.
-// Props -const props = defineProps<{ - title: string - termType: 'label' | 'description' | 'alias' - icon: string - placeholder: string - testId: string - validationPath: string - values: Label | Alias - sectionIndex: number -}>() +// Props +interface TermSectionProps { + title: string + termType: 'label' | 'description' | 'alias' + icon: string + placeholder: string + testId: string + validationPath: string + values: Label | Alias + sectionIndex: number +} +const props = defineProps<TermSectionProps>()Using retrieved learnings: props and emits should use explicit TypeScript interfaces in src/**/*.vue.
82-113: Strengthen validator robustness for sampleValuesGuard against non-string or missing sample values to prevent runtime issues, while keeping current behavior.
-const validateColumnDrop = (columnInfo: ColumnInfo) => { +const validateColumnDrop = (columnInfo: ColumnInfo) => { // 1. Data type compatibility - only string types for schema fields if (!isDataTypeCompatible(columnInfo.dataType, ['string'])) { return false } // 2. Length constraints for labels and aliases if (props.termType === 'label' || props.termType === 'alias') { const maxLength = props.termType === 'label' ? 250 : 100 - const hasLongValues = columnInfo.sampleValues?.some((val) => val.length > maxLength) + const samples = Array.isArray((columnInfo as any).sampleValues) + ? (columnInfo as any).sampleValues + : [] + const hasLongValues = samples.some((val) => typeof val === 'string' && val.length > maxLength) if (hasLongValues) { return false } } // 3. Check for duplicate aliases if (props.termType === 'alias') { const existingAliases = schemaStore.aliases[selectedLanguage.value] || [] const isDuplicate = existingAliases.some( (alias) => alias.columnName === columnInfo.name && alias.dataType === columnInfo.dataType, ) if (isDuplicate) { return false } } return true }Optional enhancement: if selectedLanguage defaults to a code not present in getAcceptedLanguages(), consider defaulting to the first available language on mount to avoid confusion for users switching locales.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
frontend/auto-imports.d.ts(0 hunks)frontend/components.d.ts(1 hunks)frontend/src/features/wikibase-schema/components/DropZone.vue(1 hunks)frontend/src/features/wikibase-schema/components/TermSection.vue(5 hunks)frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue(2 hunks)
💤 Files with no reviewable changes (1)
- frontend/auto-imports.d.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript or JavaScript files
Do not use dotenv; Bun automatically loads .env files
UseBun.serve()for HTTP servers and WebSockets instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofws
PreferBun.fileovernode:fs's readFile/writeFile
UseBun.$(e.g.,Bun.$ls``) instead of execa for running shell commands
Files:
frontend/components.d.ts
**/*.{html,ts,tsx,css}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuildfor building HTML, TypeScript, or CSS files
Files:
frontend/components.d.ts
🧠 Learnings (2)
📚 Learning: 2025-07-20T14:13:24.706Z
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use PrimeVue as the UI library in all Vue components
Applied to files:
frontend/components.d.ts
📚 Learning: 2025-07-20T14:13:24.706Z
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Props and emits must use explicit TypeScript interfaces
Applied to files:
frontend/src/features/wikibase-schema/components/TermSection.vue
🔇 Additional comments (8)
frontend/components.d.ts (1)
25-25: LGTM: Global DropZone registration is correctThe type augmentation points to the correct path and aligns with the new component. No issues from a typings perspective.
frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue (3)
95-99: LGTM: Drop handler delegates cleanly to selection logicGuarding on disabled and type, then delegating to existing selection flow is tidy and maintains a single update path.
165-185: LGTM: DropZone integration and selected-column handshake
- Placeholder toggles based on existing selection.
- test-id correctly maps to testId prop.
- selected-column shape matches DropZone’s expectations.
102-119: Accepted types match backend WikibaseDataType — no action requiredI verified backend/src/api/project/project.wikibase.ts defines WikibaseDataType with these members: 'string', 'wikibase-item', 'wikibase-property', 'quantity', 'time', 'globe-coordinate', 'url', 'external-id', 'monolingualtext', 'commonsMedia' — which exactly matches the array used in frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue.
- backend/src/api/project/project.wikibase.ts — WikibaseDataType union checked
- frontend/src/features/wikibase-schema/components/ValueMappingEditor.vue — acceptedTypes array checked
frontend/src/features/wikibase-schema/components/TermSection.vue (4)
30-40: LGTM: Validation-driven container classes are clearError and warning backgrounds are prioritized, with a stable default styled via sectionIndex. Readability and UX look good.
115-128: LGTM: Clear mapping write path by term typeMapping creation routes to the correct store method based on termType; straightforward and cohesive with the validator.
132-135: LGTM: Wrapper styling refactor improves clarityStatic base classes plus computed status classes keep style concerns separated and readable.
204-211: LGTM: DropZone integration with runtime validator
- accepted-types constrained to ['string'] is appropriate for terms.
- Validator enforces length and duplicate constraints pre-drop.
- Event handling is minimal and clear.
1 participant
No description provided.