Skip to content

fix(loading): cursor positioning, render-phase defaultValue sync, remove unnecessary useMemo#4396

Merged
waleedlatif1 merged 3 commits intostagingfrom
fix/log-v2
May 2, 2026
Merged

fix(loading): cursor positioning, render-phase defaultValue sync, remove unnecessary useMemo#4396
waleedlatif1 merged 3 commits intostagingfrom
fix/log-v2

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Restore cursor to end of textarea when navigating back to a task with a saved draft (canonical `focus()` + `setSelectionRange` pattern, no rAF)
  • Replace `useEffect` + `useRef` defaultValue sync with render-phase state update to avoid stale-value flash
  • Remove `useMemo` wrapper from `precedingUserContentByIndex` — deps changed every render so memo bought nothing, replaced with plain inline loop

Type of Change

  • Bug fix
  • Improvement

Testing

Tested manually — cursor lands at end of restored draft text, defaultValue sync correct, no regressions

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 2, 2026 3:50am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 2, 2026

PR Summary

Medium Risk
Medium risk because it changes UserInput state synchronization and focus/selection behavior, which can affect render stability and user typing experience if edge cases regress.

Overview
Fixes chat input restoration and syncing behavior: when a draft is restored, the textarea is now focused and the cursor is placed at the end of the restored text.

Reworks UserInput defaultValue syncing to update value immediately (during render) instead of via an effect/ref, reducing stale-value flashes, and removes a redundant useMemo in MothershipChat by computing precedingUserContentByIndex inline each render.

Reviewed by Cursor Bugbot for commit 3cb4a15. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 2, 2026

Greptile Summary

This PR makes three focused improvements to the home chat UI: it replaces the useEffect+useRef defaultValue-sync pattern with a render-phase state update (eliminating the one-render stale-value flash), adds focus() + setSelectionRange in the draft-restore effect so the cursor lands at the end of restored text, and inlines the precedingUserContentByIndex loop that was wrapped in a useMemo whose deps invalidated on every render anyway. All three changes are well-reasoned and the prior review thread confirms the intentional asymmetry in the defaultValue sync and the effect-ordering contract.

Confidence Score: 5/5

Safe to merge — changes are targeted, well-reasoned, and no new logic paths introduce regressions.

No P0 or P1 findings. The render-phase derived-state pattern is the canonical React approach and initialises correctly. The cursor placement correctly uses the isEditingElsewhere guard to survive the subsequent auto-focus RAF. The useMemo removal is correct given unstable upstream references. All prior thread concerns are resolved.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx Three targeted fixes: render-phase defaultValue sync (replaces useEffect+useRef), cursor-end positioning on draft restore, and removal of a stale inline comment
apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx Removes useMemo wrapper around precedingUserContentByIndex; justified because messages reference is unstable on every streaming delta so memo never bought a cache hit

Sequence Diagram

sequenceDiagram
    participant Parent
    participant UserInput
    participant React
    participant DOM

    Note over UserInput: Mount
    UserInput->>React: useState initialises value and prevDefaultValue
    React->>DOM: Render textarea with draft text

    Note over UserInput: Draft restore effect runs
    UserInput->>DOM: textarea.focus()
    UserInput->>DOM: textarea.setSelectionRange(len, len)

    Note over UserInput: Auto-focus RAF fires
    UserInput->>DOM: check activeElement
    DOM-->>UserInput: activeElement is textarea
    Note over UserInput: isEditingElsewhere=true, focus skipped, cursor preserved

    Note over Parent: defaultValue prop changes
    Parent->>UserInput: new defaultValue string
    Note over UserInput: Render-phase derived state update
    UserInput->>React: setPrevDefaultValue + setValue batched
    React->>DOM: Re-render with new value, no stale flash
Loading

Reviews (2): Last reviewed commit: "chore(user-input): remove extraneous com..." | Re-trigger Greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3cb4a15. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant