Skip to content

Focus diff comment popover from ref#3333

Merged
nwparker merged 1 commit into
mainfrom
nwparker/react-perf-diff-comment-popover-focus-ref
May 31, 2026
Merged

Focus diff comment popover from ref#3333
nwparker merged 1 commit into
mainfrom
nwparker/react-perf-diff-comment-popover-focus-ref

Conversation

@nwparker

@nwparker nwparker commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Move the diff-comment popover textarea mount focus from a passive Effect into the textarea ref callback.
  • Keep the document mousedown outside-click subscription in an Effect because that is real external DOM synchronization.
  • Removes 1 Effect hook call site from DiffCommentPopover.tsx.

Validation

  • pnpm exec oxlint src/renderer/src/components/diff-comments/DiffCommentPopover.tsx
  • pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/diff-comments/diff-comment-popover-position.test.ts
  • pnpm run typecheck:web
  • git diff --check
  • Effect hook AST count: origin/main 911, working 910

Risk

Medium. This changes editor/review-note popover focus timing only; it does not change note persistence, diff comment submission, IPC, SSH, runtime ownership, or git-provider behavior.

Risk assessment

Risk: MEDIUM

Historic risk metadata backfill based on the existing risk:medium label.

Risk: medium

Historic risk metadata backfill based on the existing risk:medium label.

@nwparker

Copy link
Copy Markdown
Contributor Author

Risk-down blocked: exact head is not mergeable against current origin/main, so I am leaving this PR at risk:medium.

Exact head: 72c4761c79bbb77ae7c41c0d0d42e8a63cdbf28c

Mergeability evidence:

  • git diff --check origin/main...refs/riskdown/pr-3333 passed.
  • git merge-tree --write-tree origin/main refs/riskdown/pr-3333 reports a content conflict in package.json.
  • The merge also auto-merged the touched renderer files, but exact-head validation is blocked by the package manifest conflict.

Because the exact PR head cannot be cleanly validated or merged on top of current main, I did not lower the risk label.

@nwparker
nwparker force-pushed the nwparker/react-perf-diff-comment-popover-focus-ref branch from 72c4761 to 2ef3f87 Compare May 31, 2026 10:37
@nwparker
nwparker merged commit f24e987 into main May 31, 2026
1 check passed
@nwparker

Copy link
Copy Markdown
Contributor Author

Validated and merged.

Evidence:

  • Rebased and pushed final head 2ef3f87de493e89d92cc9f1dc2a096dff9def1c5.
  • GitHub PR Checks verify succeeded on that head: https://github.com/stablyai/orca/actions/runs/26710300115/job/78719174287
  • Local checks passed: diff-comment-popover-position.test.ts, pnpm run typecheck:web, targeted oxlint, targeted oxfmt --check, and git diff --check origin/main...HEAD.
  • Merged with rebase/admin. Merge commit: f24e987578761e1ec98778932fc6fadc7898bc53.

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

Labels

risk:medium Medium risk change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants