refactor(wysiwyg): Clean up code and ensure drag-drop works with programmatic rendering
- Update drag handler to work without requestUpdate calls - Remove duplicate modal methods (using WysiwygModalManager) - Clean up unused imports and methods - Ensure all DOM updates are programmatic - Add comprehensive test files for all features - Follow separation of concerns principles
This commit is contained in:
@ -467,3 +467,50 @@ Fixed persistent focus loss issue by implementing fully programmatic rendering:
|
|||||||
```
|
```
|
||||||
|
|
||||||
This approach completely eliminates focus loss by taking full control of the DOM and preventing any framework-induced re-renders during editing.
|
This approach completely eliminates focus loss by taking full control of the DOM and preventing any framework-induced re-renders during editing.
|
||||||
|
|
||||||
|
### Code Refactoring and Cleanup (2025-06-24 - Part 11)
|
||||||
|
|
||||||
|
Completed comprehensive refactoring to ensure clean, maintainable code with separated concerns:
|
||||||
|
|
||||||
|
#### Refactoring Changes:
|
||||||
|
|
||||||
|
1. **Drag and Drop Handler Cleanup**:
|
||||||
|
- Removed all `requestUpdate()` calls from drag handler
|
||||||
|
- Handler now only updates internal state
|
||||||
|
- Parent component handles DOM updates programmatically
|
||||||
|
- Simplified drag state management
|
||||||
|
|
||||||
|
2. **Unused Code Removal**:
|
||||||
|
- Removed duplicate `showBlockSettingsModal` method (using WysiwygModalManager)
|
||||||
|
- Removed duplicate `showLanguageSelectionModal` method
|
||||||
|
- Removed unused `renderBlock` method
|
||||||
|
- Cleaned up unused imports (WysiwygBlocks, ISlashMenuItem)
|
||||||
|
|
||||||
|
3. **Import Cleanup**:
|
||||||
|
- Removed unused type imports
|
||||||
|
- Organized imports logically
|
||||||
|
- Kept only necessary dependencies
|
||||||
|
|
||||||
|
4. **Separated Concerns**:
|
||||||
|
- Modal management in WysiwygModalManager
|
||||||
|
- Block operations in WysiwygBlockOperations
|
||||||
|
- Input handling in WysiwygInputHandler
|
||||||
|
- Keyboard handling in WysiwygKeyboardHandler
|
||||||
|
- Drag/drop in WysiwygDragDropHandler
|
||||||
|
- Each class has a single responsibility
|
||||||
|
|
||||||
|
5. **Programmatic DOM Management**:
|
||||||
|
- All DOM updates happen through explicit methods
|
||||||
|
- No reactive re-renders during user interaction
|
||||||
|
- Manual class management for drag states
|
||||||
|
- Direct DOM manipulation for performance
|
||||||
|
|
||||||
|
6. **Test Files Created**:
|
||||||
|
- `test-focus-fix.html` - Verifies focus management
|
||||||
|
- `test-drag-drop.html` - Tests drag and drop functionality
|
||||||
|
- `test-comprehensive.html` - Tests all features together
|
||||||
|
|
||||||
|
The refactoring follows the principles in instructions.md:
|
||||||
|
- Uses static templates with manual DOM operations
|
||||||
|
- Maintains separated concerns in different classes
|
||||||
|
- Results in clean, concise, and manageable code
|
@ -17,8 +17,6 @@ import {
|
|||||||
wysiwygStyles,
|
wysiwygStyles,
|
||||||
WysiwygConverters,
|
WysiwygConverters,
|
||||||
WysiwygShortcuts,
|
WysiwygShortcuts,
|
||||||
WysiwygBlocks,
|
|
||||||
type ISlashMenuItem,
|
|
||||||
WysiwygFormatting,
|
WysiwygFormatting,
|
||||||
WysiwygBlockOperations,
|
WysiwygBlockOperations,
|
||||||
WysiwygInputHandler,
|
WysiwygInputHandler,
|
||||||
@ -1027,79 +1025,5 @@ export class DeesInputWysiwyg extends DeesInputBase<string> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
private async showBlockSettingsModal(block: IBlock): Promise<void> {
|
// Modal methods have been moved to WysiwygModalManager
|
||||||
let content: TemplateResult;
|
|
||||||
|
|
||||||
if (block.type === 'code') {
|
|
||||||
const currentLanguage = block.metadata?.language || 'plain text';
|
|
||||||
content = html`
|
|
||||||
<style>
|
|
||||||
.settings-section {
|
|
||||||
margin-bottom: 16px;
|
|
||||||
}
|
|
||||||
.settings-label {
|
|
||||||
font-weight: 500;
|
|
||||||
margin-bottom: 8px;
|
|
||||||
}
|
|
||||||
.language-grid {
|
|
||||||
display: grid;
|
|
||||||
grid-template-columns: repeat(3, 1fr);
|
|
||||||
gap: 8px;
|
|
||||||
}
|
|
||||||
.language-button {
|
|
||||||
padding: 8px;
|
|
||||||
background: var(--dees-color-box);
|
|
||||||
border: 1px solid var(--dees-color-line-bright);
|
|
||||||
border-radius: 4px;
|
|
||||||
cursor: pointer;
|
|
||||||
text-align: center;
|
|
||||||
transition: all 0.2s;
|
|
||||||
}
|
|
||||||
.language-button:hover {
|
|
||||||
background: var(--dees-color-box-highlight);
|
|
||||||
border-color: var(--dees-color-primary);
|
|
||||||
}
|
|
||||||
.language-button.selected {
|
|
||||||
background: var(--dees-color-primary);
|
|
||||||
color: white;
|
|
||||||
}
|
|
||||||
</style>
|
|
||||||
<div class="settings-section">
|
|
||||||
<div class="settings-label">Programming Language</div>
|
|
||||||
<div class="language-grid">
|
|
||||||
${['JavaScript', 'TypeScript', 'Python', 'Java', 'C++', 'C#', 'Go', 'Rust', 'HTML', 'CSS', 'SQL', 'Shell', 'JSON', 'YAML', 'Markdown', 'Plain Text'].map(lang => html`
|
|
||||||
<div class="language-button ${currentLanguage === lang.toLowerCase() ? 'selected' : ''}"
|
|
||||||
@click="${(e: MouseEvent) => {
|
|
||||||
if (!block.metadata) block.metadata = {};
|
|
||||||
block.metadata.language = lang.toLowerCase();
|
|
||||||
this.updateValue();
|
|
||||||
this.requestUpdate();
|
|
||||||
// Find and click the close button
|
|
||||||
const modal = (e.target as HTMLElement).closest('dees-modal');
|
|
||||||
if (modal) {
|
|
||||||
const closeButton = modal.shadowRoot?.querySelector('.bottomButton') as HTMLElement;
|
|
||||||
if (closeButton) closeButton.click();
|
|
||||||
}
|
|
||||||
}}">${lang}</div>
|
|
||||||
`)}
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
`;
|
|
||||||
} else {
|
|
||||||
content = html`<div style="padding: 16px;">No settings available for this block type.</div>`;
|
|
||||||
}
|
|
||||||
|
|
||||||
DeesModal.createAndShow({
|
|
||||||
heading: 'Block Settings',
|
|
||||||
content,
|
|
||||||
menuOptions: [
|
|
||||||
{
|
|
||||||
name: 'Close',
|
|
||||||
action: async (modal) => {
|
|
||||||
modal.destroy();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
]
|
|
||||||
});
|
|
||||||
}
|
|
||||||
}
|
}
|
@ -31,13 +31,10 @@ export class WysiwygDragDropHandler {
|
|||||||
e.dataTransfer.effectAllowed = 'move';
|
e.dataTransfer.effectAllowed = 'move';
|
||||||
e.dataTransfer.setData('text/plain', block.id);
|
e.dataTransfer.setData('text/plain', block.id);
|
||||||
|
|
||||||
// Update UI state
|
// Update component state (for parent drag handler access)
|
||||||
this.updateComponentState();
|
this.component.draggedBlockId = this.draggedBlockId;
|
||||||
|
|
||||||
// Add slight delay to show dragging state
|
// The parent component already handles adding dragging classes programmatically
|
||||||
setTimeout(() => {
|
|
||||||
this.component.requestUpdate();
|
|
||||||
}, 10);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -47,8 +44,13 @@ export class WysiwygDragDropHandler {
|
|||||||
this.draggedBlockId = null;
|
this.draggedBlockId = null;
|
||||||
this.dragOverBlockId = null;
|
this.dragOverBlockId = null;
|
||||||
this.dragOverPosition = null;
|
this.dragOverPosition = null;
|
||||||
this.updateComponentState();
|
|
||||||
this.component.requestUpdate();
|
// Update component state
|
||||||
|
this.component.draggedBlockId = null;
|
||||||
|
this.component.dragOverBlockId = null;
|
||||||
|
this.component.dragOverPosition = null;
|
||||||
|
|
||||||
|
// The parent component already handles removing dragging classes programmatically
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -65,8 +67,12 @@ export class WysiwygDragDropHandler {
|
|||||||
|
|
||||||
this.dragOverBlockId = block.id;
|
this.dragOverBlockId = block.id;
|
||||||
this.dragOverPosition = e.clientY < midpoint ? 'before' : 'after';
|
this.dragOverPosition = e.clientY < midpoint ? 'before' : 'after';
|
||||||
this.updateComponentState();
|
|
||||||
this.component.requestUpdate();
|
// Update component state
|
||||||
|
this.component.dragOverBlockId = this.dragOverBlockId;
|
||||||
|
this.component.dragOverPosition = this.dragOverPosition;
|
||||||
|
|
||||||
|
// The parent component already handles drag-over classes programmatically
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -76,8 +82,12 @@ export class WysiwygDragDropHandler {
|
|||||||
if (this.dragOverBlockId === block.id) {
|
if (this.dragOverBlockId === block.id) {
|
||||||
this.dragOverBlockId = null;
|
this.dragOverBlockId = null;
|
||||||
this.dragOverPosition = null;
|
this.dragOverPosition = null;
|
||||||
this.updateComponentState();
|
|
||||||
this.component.requestUpdate();
|
// Update component state
|
||||||
|
this.component.dragOverBlockId = null;
|
||||||
|
this.component.dragOverPosition = null;
|
||||||
|
|
||||||
|
// The parent component already handles removing drag-over classes programmatically
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -89,48 +99,11 @@ export class WysiwygDragDropHandler {
|
|||||||
|
|
||||||
if (!this.draggedBlockId || this.draggedBlockId === targetBlock.id) return;
|
if (!this.draggedBlockId || this.draggedBlockId === targetBlock.id) return;
|
||||||
|
|
||||||
const blocks = [...this.component.blocks];
|
// The parent component already has a handleDrop method that handles this programmatically
|
||||||
const draggedIndex = blocks.findIndex(b => b.id === this.draggedBlockId);
|
// We'll delegate to that to ensure proper programmatic rendering
|
||||||
const targetIndex = blocks.findIndex(b => b.id === targetBlock.id);
|
this.component.handleDrop(e, targetBlock);
|
||||||
|
|
||||||
if (draggedIndex === -1 || targetIndex === -1) return;
|
|
||||||
|
|
||||||
// Remove the dragged block
|
|
||||||
const [draggedBlock] = blocks.splice(draggedIndex, 1);
|
|
||||||
|
|
||||||
// Calculate the new index
|
|
||||||
let newIndex = targetIndex;
|
|
||||||
if (this.dragOverPosition === 'after') {
|
|
||||||
newIndex = draggedIndex < targetIndex ? targetIndex : targetIndex + 1;
|
|
||||||
} else {
|
|
||||||
newIndex = draggedIndex < targetIndex ? targetIndex - 1 : targetIndex;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Insert at new position
|
|
||||||
blocks.splice(newIndex, 0, draggedBlock);
|
|
||||||
|
|
||||||
// Update blocks
|
|
||||||
this.component.blocks = blocks;
|
|
||||||
this.component.updateValue();
|
|
||||||
this.handleDragEnd();
|
|
||||||
|
|
||||||
// Focus the moved block
|
|
||||||
setTimeout(() => {
|
|
||||||
const blockOps = this.component.blockOperations;
|
|
||||||
if (draggedBlock.type !== 'divider') {
|
|
||||||
blockOps.focusBlock(draggedBlock.id);
|
|
||||||
}
|
|
||||||
}, 100);
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Updates component drag state
|
|
||||||
*/
|
|
||||||
private updateComponentState(): void {
|
|
||||||
this.component.draggedBlockId = this.draggedBlockId;
|
|
||||||
this.component.dragOverBlockId = this.dragOverBlockId;
|
|
||||||
this.component.dragOverPosition = this.dragOverPosition;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Checks if a block is being dragged
|
* Checks if a block is being dragged
|
||||||
|
Reference in New Issue
Block a user