refactor: remove redundant conditional check in EditorComponent

[saadiq]

Summary

• Removed nested redundant condition in EditorComponent.tsx content sync useEffect
• The outer condition already validates when updates are needed, making the inner check unnecessary
• Improves code clarity without changing behavior

Details

The useEffect at line 62-77 had a redundant nested conditional:

• Outer check: (isViewModeTransition || (viewMode === 'wysiwyg' && hasContentChanged))
• Inner check: (isViewModeTransition || hasContentChanged) ❌ Redundant

When the outer condition is true, the inner condition is guaranteed to be true, making it unnecessary.

Test plan

• Verify WYSIWYG ↔ Markdown mode switching still works
• Verify content updates properly when switching from markdown to WYSIWYG
• Verify no infinite re-render loops occur
• Run existing editor tests

[claude]

Code Review

Summary

This is a valid refactoring that removes redundant logic. The change is correct and improves code clarity.

Analysis

Original Logic:

if (editor && content && (isViewModeTransition || (viewMode === 'wysiwyg' && hasContentChanged))) {
  if (isViewModeTransition || hasContentChanged) {  // ❌ Redundant
    const json = markdownToJSON(content);
    editor.commands.setContent(json);
  }
}

After Refactoring:

if (editor && content && (isViewModeTransition || (viewMode === 'wysiwyg' && hasContentChanged))) {
  const json = markdownToJSON(content);
  editor.commands.setContent(json);
}

Why This Is Correct

The inner condition checks (isViewModeTransition || hasContentChanged), but the outer condition already ensures:

• Either isViewModeTransition is true, OR
• Both viewMode === 'wysiwyg' AND hasContentChanged are true

In both cases, the inner condition (isViewModeTransition || hasContentChanged) is guaranteed to be true:

• Case 1: If isViewModeTransition is true → inner condition is true ✅
• Case 2: If viewMode === 'wysiwyg' && hasContentChanged is true → hasContentChanged is true → inner condition is true ✅

Therefore, the inner check is redundant and can be safely removed.

Code Quality: ✅ Excellent

• Clean refactoring with clear reasoning
• Maintains exact same behavior
• Improves readability
• Follows DRY principles

Potential Issues: ✅ None Found

• Logic is sound
• No race conditions introduced
• No performance regressions

Security: ✅ No Concerns

• No security-related changes
• Content still flows through markdownToJSON sanitization

Test Coverage: ⚠️ Needs Attention

The test plan is well-defined, but there are no automated tests for EditorComponent. Only markdown.test.ts and App.cache.test.tsx exist in the test suite.

Recommendation: Consider adding component tests for EditorComponent to cover:

• View mode switching (WYSIWYG ↔ Markdown)
• Content synchronization during mode transitions
• useEffect behavior verification

However, for this specific refactoring, manual testing should be sufficient since the logic is provably equivalent.

Performance: ✅ No Impact

• Same number of operations executed
• Slightly less conditional evaluation (negligible improvement)

Verdict: APPROVED ✅

This is a clean, low-risk refactoring that improves code quality. The logic analysis confirms the inner conditional is mathematically redundant.

Suggestions:

1. ✅ Code change looks good as-is
2. ⚠️ Consider adding automated tests for EditorComponent in a future PR (not blocking for this change)
3. ✅ The PR description clearly explains the reasoning