refactor: Modularize UI components and utilities
- Extract UI components into separate JS files - Centralize configuration values in config.js - Introduce a dedicated logger module - Improve file tree drag-and-drop and undo functionality - Refactor modal handling to a single manager - Add URL routing support for SPA navigation - Implement view mode for read-only access
This commit is contained in:
Mahmoud-Emad
426
refactor-plan.md
Normal file
426
refactor-plan.md
Normal file
@@ -0,0 +1,426 @@
|
||||
# UI Code Refactoring Plan
|
||||
|
||||
**Project:** Markdown Editor
|
||||
**Date:** 2025-10-26
|
||||
**Status:** In Progress
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
This document outlines a comprehensive refactoring plan for the UI codebase to improve maintainability, remove dead code, extract utilities, and standardize patterns. The refactoring is organized into 6 phases with 14 tasks, prioritized by risk and impact.
|
||||
|
||||
**Key Metrics:**
|
||||
|
||||
- Total Lines of Code: ~3,587
|
||||
- Dead Code to Remove: 213 lines (6%)
|
||||
- Estimated Effort: 5-8 days
|
||||
- Risk Level: Mostly LOW to MEDIUM
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Analysis Summary
|
||||
|
||||
### Files Reviewed
|
||||
|
||||
**JavaScript Files (10):**
|
||||
|
||||
- `/static/js/app.js` (484 lines)
|
||||
- `/static/js/column-resizer.js` (100 lines)
|
||||
- `/static/js/confirmation.js` (170 lines)
|
||||
- `/static/js/editor.js` (420 lines)
|
||||
- `/static/js/file-tree-actions.js` (482 lines)
|
||||
- `/static/js/file-tree.js` (865 lines)
|
||||
- `/static/js/macro-parser.js` (103 lines)
|
||||
- `/static/js/macro-processor.js` (157 lines)
|
||||
- `/static/js/ui-utils.js` (305 lines)
|
||||
- `/static/js/webdav-client.js` (266 lines)
|
||||
|
||||
**CSS Files (6):**
|
||||
|
||||
- `/static/css/variables.css` (32 lines)
|
||||
- `/static/css/layout.css`
|
||||
- `/static/css/file-tree.css`
|
||||
- `/static/css/editor.css`
|
||||
- `/static/css/components.css`
|
||||
- `/static/css/modal.css`
|
||||
|
||||
**HTML Templates (1):**
|
||||
|
||||
- `/templates/index.html` (203 lines)
|
||||
|
||||
---
|
||||
|
||||
## Issues Found
|
||||
|
||||
### 🔴 HIGH PRIORITY
|
||||
|
||||
1. **Deprecated Modal Code (Dead Code)**
|
||||
- Location: `/static/js/file-tree-actions.js` lines 262-474
|
||||
- Impact: 213 lines of unused code (44% of file)
|
||||
- Risk: LOW to remove
|
||||
|
||||
2. **Duplicated Event Bus Implementation**
|
||||
- Location: `/static/js/app.js` lines 16-30
|
||||
- Should be extracted to reusable module
|
||||
|
||||
3. **Duplicated Debounce Function**
|
||||
- Location: `/static/js/editor.js` lines 404-414
|
||||
- Should be shared utility
|
||||
|
||||
4. **Inconsistent Notification Usage**
|
||||
- Mixed usage of `window.showNotification` vs `showNotification`
|
||||
|
||||
5. **Duplicated File Download Logic**
|
||||
- Location: `/static/js/file-tree.js` lines 829-839
|
||||
- Should be shared utility
|
||||
|
||||
6. **Hard-coded Values**
|
||||
- Long-press threshold: 400ms
|
||||
- Debounce delay: 300ms
|
||||
- Drag preview width: 200px
|
||||
- Toast delay: 3000ms
|
||||
|
||||
### 🟡 MEDIUM PRIORITY
|
||||
|
||||
7. **Global State Management**
|
||||
- Location: `/static/js/app.js` lines 6-13
|
||||
- Makes testing difficult
|
||||
|
||||
8. **Duplicated Path Manipulation**
|
||||
- `path.split('/').pop()` appears 10+ times
|
||||
- `path.substring(0, path.lastIndexOf('/'))` appears 5+ times
|
||||
|
||||
9. **Mixed Responsibility in ui-utils.js**
|
||||
- Contains 6 different classes/utilities
|
||||
- Should be split into separate modules
|
||||
|
||||
10. **Deprecated Event Handler**
|
||||
- Location: `/static/js/file-tree-actions.js` line 329
|
||||
- Uses deprecated `onkeypress`
|
||||
|
||||
### 🟢 LOW PRIORITY
|
||||
|
||||
11. **Unused Function Parameters**
|
||||
12. **Magic Numbers in Styling**
|
||||
13. **Inconsistent Comment Styles**
|
||||
14. **Console.log Statements**
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Proposed Reusable Components
|
||||
|
||||
### 1. Config Module (`/static/js/config.js`)
|
||||
|
||||
Centralize all configuration values:
|
||||
|
||||
```javascript
|
||||
export const Config = {
|
||||
// Timing
|
||||
LONG_PRESS_THRESHOLD: 400,
|
||||
DEBOUNCE_DELAY: 300,
|
||||
TOAST_DURATION: 3000,
|
||||
|
||||
// UI
|
||||
DRAG_PREVIEW_WIDTH: 200,
|
||||
TREE_INDENT_PX: 12,
|
||||
MOUSE_MOVE_THRESHOLD: 5,
|
||||
|
||||
// Validation
|
||||
FILENAME_PATTERN: /^[a-z0-9_]+(\.[a-z0-9_]+)*$/,
|
||||
|
||||
// Storage Keys
|
||||
STORAGE_KEYS: {
|
||||
DARK_MODE: 'darkMode',
|
||||
SELECTED_COLLECTION: 'selectedCollection',
|
||||
LAST_VIEWED_PAGE: 'lastViewedPage',
|
||||
COLUMN_DIMENSIONS: 'columnDimensions'
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
### 2. Logger Module (`/static/js/logger.js`)
|
||||
|
||||
Structured logging with levels:
|
||||
|
||||
```javascript
|
||||
export class Logger {
|
||||
static debug(message, ...args)
|
||||
static info(message, ...args)
|
||||
static warn(message, ...args)
|
||||
static error(message, ...args)
|
||||
static setLevel(level)
|
||||
}
|
||||
```
|
||||
|
||||
### 3. Event Bus Module (`/static/js/event-bus.js`)
|
||||
|
||||
Centralized event system:
|
||||
|
||||
```javascript
|
||||
export class EventBus {
|
||||
on(event, callback)
|
||||
off(event, callback)
|
||||
once(event, callback)
|
||||
dispatch(event, data)
|
||||
clear(event)
|
||||
}
|
||||
```
|
||||
|
||||
### 4. Utilities Module (`/static/js/utils.js`)
|
||||
|
||||
Common utility functions:
|
||||
|
||||
```javascript
|
||||
export const PathUtils = {
|
||||
getFileName(path),
|
||||
getParentPath(path),
|
||||
normalizePath(path),
|
||||
joinPaths(...paths),
|
||||
getExtension(path)
|
||||
};
|
||||
|
||||
export const TimingUtils = {
|
||||
debounce(func, wait),
|
||||
throttle(func, wait)
|
||||
};
|
||||
|
||||
export const DownloadUtils = {
|
||||
triggerDownload(content, filename),
|
||||
downloadAsBlob(blob, filename)
|
||||
};
|
||||
|
||||
export const ValidationUtils = {
|
||||
validateFileName(name, isFolder),
|
||||
sanitizeFileName(name)
|
||||
};
|
||||
```
|
||||
|
||||
### 5. Notification Service (`/static/js/notification-service.js`)
|
||||
|
||||
Standardized notifications:
|
||||
|
||||
```javascript
|
||||
export class NotificationService {
|
||||
static success(message)
|
||||
static error(message)
|
||||
static warning(message)
|
||||
static info(message)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Refactoring Tasks
|
||||
|
||||
### 🔴 HIGH PRIORITY
|
||||
|
||||
**Task 1: Remove Dead Code**
|
||||
|
||||
- Files: `/static/js/file-tree-actions.js`
|
||||
- Lines: 262-474 (213 lines)
|
||||
- Risk: LOW
|
||||
- Dependencies: None
|
||||
|
||||
**Task 2: Extract Event Bus**
|
||||
|
||||
- Files: NEW `/static/js/event-bus.js`, MODIFY `app.js`, `editor.js`
|
||||
- Risk: MEDIUM
|
||||
- Dependencies: None
|
||||
|
||||
**Task 3: Create Utilities Module**
|
||||
|
||||
- Files: NEW `/static/js/utils.js`, MODIFY multiple files
|
||||
- Risk: MEDIUM
|
||||
- Dependencies: None
|
||||
|
||||
**Task 4: Create Config Module**
|
||||
|
||||
- Files: NEW `/static/js/config.js`, MODIFY multiple files
|
||||
- Risk: LOW
|
||||
- Dependencies: None
|
||||
|
||||
**Task 5: Standardize Notification Usage**
|
||||
|
||||
- Files: NEW `/static/js/notification-service.js`, MODIFY multiple files
|
||||
- Risk: LOW
|
||||
- Dependencies: None
|
||||
|
||||
### 🟡 MEDIUM PRIORITY
|
||||
|
||||
**Task 6: Fix Deprecated Event Handler**
|
||||
|
||||
- Files: `/static/js/file-tree-actions.js` line 329
|
||||
- Risk: LOW
|
||||
- Dependencies: None
|
||||
|
||||
**Task 7: Refactor ui-utils.js**
|
||||
|
||||
- Files: DELETE `ui-utils.js`, CREATE 5 new modules
|
||||
- Risk: HIGH
|
||||
- Dependencies: Task 5
|
||||
|
||||
**Task 8: Standardize Class Export Pattern**
|
||||
|
||||
- Files: All class files
|
||||
- Risk: MEDIUM
|
||||
- Dependencies: None
|
||||
|
||||
**Task 9: Create Logger Module**
|
||||
|
||||
- Files: NEW `/static/js/logger.js`, MODIFY multiple files
|
||||
- Risk: LOW
|
||||
- Dependencies: None
|
||||
|
||||
**Task 10: Implement Download Action**
|
||||
|
||||
- Files: `/static/js/file-tree-actions.js`
|
||||
- Risk: LOW
|
||||
- Dependencies: Task 3
|
||||
|
||||
### 🟢 LOW PRIORITY
|
||||
|
||||
**Task 11: Standardize JSDoc Comments**
|
||||
**Task 12: Extract Magic Numbers to CSS**
|
||||
**Task 13: Add Error Boundaries**
|
||||
**Task 14: Cache DOM Elements**
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Implementation Order
|
||||
|
||||
### Step 1: Foundation (Do First)
|
||||
|
||||
1. Create Config Module (Task 4)
|
||||
2. Create Logger Module (Task 9)
|
||||
3. Create Event Bus Module (Task 2)
|
||||
|
||||
### Step 2: Utilities (Do Second)
|
||||
|
||||
4. Create Utilities Module (Task 3)
|
||||
5. Create Notification Service (Task 5)
|
||||
|
||||
### Step 3: Cleanup (Do Third)
|
||||
|
||||
6. Remove Dead Code (Task 1)
|
||||
7. Fix Deprecated Event Handler (Task 6)
|
||||
|
||||
### Step 4: Restructuring (Do Fourth)
|
||||
|
||||
8. Refactor ui-utils.js (Task 7)
|
||||
9. Standardize Class Export Pattern (Task 8)
|
||||
|
||||
### Step 5: Enhancements (Do Fifth)
|
||||
|
||||
10. Implement Download Action (Task 10)
|
||||
11. Add Error Boundaries (Task 13)
|
||||
|
||||
### Step 6: Polish (Do Last)
|
||||
|
||||
12. Standardize JSDoc Comments (Task 11)
|
||||
13. Extract Magic Numbers to CSS (Task 12)
|
||||
14. Cache DOM Elements (Task 14)
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: Testing Checklist
|
||||
|
||||
### Core Functionality
|
||||
|
||||
- [ ] File tree loads and displays correctly
|
||||
- [ ] Files can be selected and opened
|
||||
- [ ] Folders can be expanded/collapsed
|
||||
- [ ] Editor loads file content
|
||||
- [ ] Preview renders markdown correctly
|
||||
- [ ] Save button saves files
|
||||
- [ ] Delete button deletes files
|
||||
- [ ] New button creates new files
|
||||
|
||||
### Context Menu Actions
|
||||
|
||||
- [ ] Right-click shows context menu
|
||||
- [ ] New file action works
|
||||
- [ ] New folder action works
|
||||
- [ ] Rename action works
|
||||
- [ ] Delete action works
|
||||
- [ ] Copy/Cut/Paste actions work
|
||||
- [ ] Upload action works
|
||||
|
||||
### Drag and Drop
|
||||
|
||||
- [ ] Long-press detection works
|
||||
- [ ] Drag preview appears correctly
|
||||
- [ ] Drop targets highlight properly
|
||||
- [ ] Files can be moved
|
||||
- [ ] Undo (Ctrl+Z) works
|
||||
|
||||
### Modals
|
||||
|
||||
- [ ] Confirmation modals appear
|
||||
- [ ] Prompt modals appear
|
||||
- [ ] Modals don't double-open
|
||||
- [ ] Enter/Escape keys work
|
||||
|
||||
### UI Features
|
||||
|
||||
- [ ] Dark mode toggle works
|
||||
- [ ] Collection selector works
|
||||
- [ ] Column resizers work
|
||||
- [ ] Notifications appear
|
||||
- [ ] URL routing works
|
||||
- [ ] View/Edit modes work
|
||||
|
||||
---
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Immediate Actions (Before Production)
|
||||
|
||||
1. Remove dead code (Task 1)
|
||||
2. Fix deprecated event handler (Task 6)
|
||||
3. Create config module (Task 4)
|
||||
|
||||
### Short-term Actions (Next Sprint)
|
||||
|
||||
4. Extract utilities (Task 3)
|
||||
5. Standardize notifications (Task 5)
|
||||
6. Create event bus (Task 2)
|
||||
|
||||
### Medium-term Actions (Future Sprints)
|
||||
|
||||
7. Refactor ui-utils.js (Task 7)
|
||||
8. Add logger (Task 9)
|
||||
9. Standardize exports (Task 8)
|
||||
|
||||
---
|
||||
|
||||
## Success Metrics
|
||||
|
||||
**Before Refactoring:**
|
||||
|
||||
- Total Lines: ~3,587
|
||||
- Dead Code: 213 lines (6%)
|
||||
- Duplicated Code: ~50 lines
|
||||
- Hard-coded Values: 15+
|
||||
|
||||
**After Refactoring:**
|
||||
|
||||
- Total Lines: ~3,400 (-5%)
|
||||
- Dead Code: 0 lines
|
||||
- Duplicated Code: 0 lines
|
||||
- Hard-coded Values: 0
|
||||
|
||||
**Estimated Effort:** 5-8 days
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
The UI codebase is generally well-structured. Main improvements needed:
|
||||
|
||||
1. Remove dead code
|
||||
2. Extract duplicated utilities
|
||||
3. Centralize configuration
|
||||
4. Standardize patterns
|
||||
|
||||
Start with high-impact, low-risk changes first to ensure production readiness.
|
||||
Reference in New Issue
Block a user