Fix: New tab opens with scroll position at top

[malibio]

Summary

• Adds { preventScroll: true } to all focus() calls in the cursor positioning system
• Prevents browser auto-scrolling behavior when focusing textarea elements
• Ensures new tabs open with scroll position at top

Changes

• textarea-controller.svelte.ts: Updated TextareaController.focus() method (main entry point)
• cursor-positioning-service.ts: Updated all direct textarea.focus() calls
• base-node-viewer.svelte: Updated cursor restoration focus calls

Test plan

• Enter key creates new node - cursor should be in new node
• Arrow up/down navigation between nodes
• Clicking on a node to edit
• Creating a node via slash command
• Tab switching preserves scroll position
• New tab opens at scroll top
• Indent/outdent operations
• Node type conversion (text → task, etc.)

Note: Unit tests pass but manual testing is required as mocked DOM doesn't detect real browser focus/scroll behavior.

Closes #860

🤖 Generated with Claude Code

[malibio]

Code Review: PR #862 - Fix: New tab opens with scroll position at top

Review Type: Initial Review
Reviewer: Principal Engineer (Automated)
Verdict: APPROVE

---

Requirements Validation (Issue #860)

Acceptance Criterion	Status
Add { preventScroll: true } to focus calls in cursor positioning system	✅ Complete
Prevent browser auto-scrolling when focusing textarea elements	✅ Complete
New tabs open with scroll position at top	✅ Addressed
Tab switching preserves scroll position	✅ Addressed

---

Code Review Findings

Architectural Assessment

Summary: This is a well-scoped, surgical fix that addresses the root cause identified in the issue. The change follows the principle of least surprise by modifying the centralized focus mechanism rather than scattering fixes throughout the codebase.

Design Quality:

• ✅ Single Responsibility: Changes are limited to focus behavior only
• ✅ DRY Principle: The primary fix in TextareaController.focus() is the main entry point - all controller.focus() calls (in position-cursor.ts, base-node.svelte) automatically inherit this behavior
• ✅ Defense in Depth: Direct textarea.focus() calls in CursorPositioningService and base-node-viewer.svelte are also updated for completeness

---

:large_green_circle: Suggestions (Non-Blocking)

1. Documentation Quality - Excellent

📁 packages/desktop-app/src/lib/design/components/textarea-controller.svelte.ts:434-438

The inline comments clearly document the "why" (the three critical scenarios) rather than just the "what". This is exemplary documentation practice that will help future maintainers understand the rationale.

2. Comprehensive Coverage Verification

I verified that all production focus() calls are properly covered:

Location	Type	Coverage
TextareaController.focus()	Central method	✅ Updated directly
position-cursor.ts (5 calls)	Uses controller.focus()	✅ Inherits fix
base-node.svelte (3 calls)	Uses controller.focus()	✅ Inherits fix
CursorPositioningService (3 calls)	Direct textarea.focus()	✅ Updated directly
base-node-viewer.svelte (2 calls)	Direct element.focus()	✅ Updated directly

No orphaned element.focus() or textarea.focus() calls remain in /src/lib/ that could cause scroll issues.

---

🟡 Important Notes

Manual Testing Required

As correctly noted in the PR description, this change requires manual testing because:

1. Unit tests use mocked DOM that does not simulate real browser focus/scroll behavior
2. Happy-DOM environment does not trigger actual scroll events

The test plan provided in the PR description is comprehensive and covers the critical scenarios:

• Enter key node creation
• Arrow navigation between nodes
• Click-to-edit behavior
• Slash command interactions
• Tab switching scroll preservation
• New tab scroll position

---

Security Assessment

• ✅ No security concerns - this is a UI behavior change only
• ✅ No new dependencies introduced
• ✅ No API surface changes

---

Performance Assessment

• ✅ No performance impact - preventScroll is a native browser API option
• ✅ May slightly improve perceived performance by eliminating unnecessary scroll jank

---

Final Verdict: APPROVE

This is a clean, well-documented fix that:

1. Correctly identifies and addresses the root cause
2. Applies the fix consistently across all focus call sites
3. Includes clear documentation of the rationale
4. Acknowledges testing limitations and provides a manual test plan

The change is a clear net positive for code health with minimal risk.

---

🤖 Generated with Claude Code - Principal Engineer Review

[malibio]

Review complete. Recommendation: APPROVE. This is a well-scoped fix that correctly addresses the root cause. See detailed review comment above.