Refactor: Extract constants, improve code quality, and fix bugs#2
Open
Refactor: Extract constants, improve code quality, and fix bugs#2
Conversation
Detailed security, performance, code quality, and architecture audit covering all 3,830 lines of the Threads DraftCraft Chrome extension. https://claude.ai/code/session_01Na7TPMpUpohXy29azuD8fD
Security: - Replace all innerHTML usage with safe DOM API (createElement/textContent) - Remove web_accessible_resources exposing content/ directory - Replace console.log/error with debug-flag-guarded logging Bugs: - Fix random-based scheduling causing unstable sort (now deterministic) - Fix scheduledDrafts count showing totalDrafts instead of scheduled count - Fix MutationObserver never disconnecting (memory leak) - Fix daysUntil=0 edge case (same day now maps to next week) - Fix double initialization in background.js (remove redundant handleInstallation) - Fix collectScheduledByDate() called inside loop (compute once outside) Performance: - Add debounce to MutationObserver processing (150ms) - Remove fragile .x1n2onr6 CSS selector, use only [role="dialog"] - Simplify extension mutation detection with className.includes() Code quality: - Extract shared regex patterns to module-level constants - Extract magic numbers into named constants - Refactor 6 duplicated toggle handlers into generic _toggleSetting() - Move all inline styles to CSS classes - Add _to24Hour() and _daysUntilDay() helper methods - Add _matchesDayNamePattern() shared helper Accessibility: - Add aria-label to all toggle inputs and select - Add role="alert" to error messages, role="status" to success/stats - Add aria-label to close buttons - Add role="region" with aria-label to sections - Add aria-hidden to decorative icons https://claude.ai/code/session_01Na7TPMpUpohXy29azuD8fD
Bugs fixed: - Add return true to message listener for async response support - Add ping handler for background script content script detection - Fix index=-1 producing wrong scheduling times (new _safeElementIndex helper) - Fix unstable sort for equal timestamps (tiebreaker by originalOrder) - Add missing showDateDivider to background.js getSettings() defaults - Add Date validity check (isNaN) after Date construction - Fix logError missing .catch() on promise chain, simplify with slice(-50) - Add _daysUntilDay input validation for out-of-range day indices Performance: - Combine 4 querySelectorAll calls into single selector in removeExistingEnhancements - Reuse getDateKey() in addDateDividers instead of inline string construction Code quality: - Remove dead code: unreachable m===60 check, commented-out console.log block - Simplify randomNonZeroMinute to one-liner Robustness: - Add document.contains() check before DOM operations in reorderDraftElements - Add ARIA labels to dynamically created time indicators - Add tabindex and aria-live to popup error/success messages for focus management - Add focus() call on error message display https://claude.ai/code/session_01Na7TPMpUpohXy29azuD8fD
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
This PR refactors the codebase to improve maintainability, fix several bugs, and enhance code quality through constant extraction, code deduplication, and better error handling.
Key Changes
Constants & Configuration
MIN_DRAFT_TEXT_LENGTH,DEBOUNCE_DELAY_MS,SCHEDULE_TIME_PATTERNS, etc.)hasScheduleableContent()andextractScheduledTime()DRAFTCRAFT_DEBUGandBG_DEBUGflags for conditional loggingBug Fixes
stats.scheduledDraftsinstead ofstats.totalDraftsdisconnect()method and cleanup on observer lifecycle_debounceTimerto prevent excessive DOM processing during rapid mutationshandleInstallation()call in background script constructorCode Quality Improvements
_toggleSetting()and_sendToActiveTab()helper methods to consolidate 6 nearly-identical toggle handlers_to24Hour()and_daysUntilDay()for time conversion logic.x1n2onr6), now relies on semantic[role="dialog"]attributesstyleattributes with CSS classes (.threads-draftcraft-status-label,.threads-draftcraft-time-info, etc.)console.log()calls with conditionaldebugLog()andbgLog()functionsAccessibility Enhancements
role="region"andaria-labelattributes to popup sectionsrole="status"to draft statistics elementsSecurity & Manifest
web_accessible_resourcesdeclaration (was exposing entirecontent/*directory)Performance
Implementation Details
_to24Hour,_daysUntilDay) to indicate internal useclearTimeout()before setting new timer to prevent race conditions.threads-draftcraft-*https://claude.ai/code/session_01Na7TPMpUpohXy29azuD8fD