Skip to content

Pt 3897 replace preview#2154

Open
katherinejensen00 wants to merge 25 commits intomainfrom
pt-3897-replace-preview
Open

Pt 3897 replace preview#2154
katherinejensen00 wants to merge 25 commits intomainfrom
pt-3897-replace-preview

Conversation

@katherinejensen00
Copy link
Copy Markdown
Contributor

@katherinejensen00 katherinejensen00 commented Mar 27, 2026

https://paratextstudio.atlassian.net/browse/PT-3897
image

  • Create a replace preview view options picker as per this V0: https://v0.app/chat/preview-options-picker-s08sC9IIAfJ
  • Fix history (I broke it when I implemented auto search)
  • Fix some styling that Sebastian pointed out
  • Improve keyboard navigation so the editor navigates to the reference without losing the cursor to the editor

Demo Video (had to zip it because it was just a little bit over the size limit for files):
replace-preview-options-picker.zip


This change is Reviewable


Open with Devin

@katherinejensen00 katherinejensen00 force-pushed the pt-3897-replace-preview branch 2 times, most recently from 435818b to b297cb5 Compare March 30, 2026 14:42
@katherinejensen00 katherinejensen00 marked this pull request as ready for review March 30, 2026 16:26
devin-ai-integration[bot]

This comment was marked as resolved.

@katherinejensen00 katherinejensen00 force-pushed the pt-3897-replace-preview branch from 60489f4 to b53bdfb Compare March 31, 2026 16:26
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@katherinejensen00 katherinejensen00 force-pushed the pt-3897-replace-preview branch from f3e32fe to ff1866d Compare April 1, 2026 15:30
devin-ai-integration[bot]

This comment was marked as resolved.

@katherinejensen00 katherinejensen00 force-pushed the pt-3897-replace-preview branch from 0e6df3a to d123fb1 Compare April 2, 2026 01:58
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

katherinejensen00 and others added 2 commits April 1, 2026 21:38
Fixes issue where clicking the kebab menu on find/replace results caused
the program to blank out with "Tried to send payload while not connected" error.

Changes:
1. Make focus event handler more conservative - require relatedTarget to be a
   valid Node to avoid spurious focus events during dropdown interactions
2. Add event.stopPropagation() to dropdown menu items to prevent event bubbling
3. In find.web-view.tsx handleFocusedResultChange: add silent error handling to
   prevent cascading failures if editor controller is temporarily unavailable
4. In find.web-view.tsx handleOpenAtResult: chain selectRange and setAnnotation
   calls sequentially to ensure stable connection state

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 20 additional findings in Devin Review.

Open in Devin Review

Comment on lines +363 to +366
return () => {
clearTimeout(saveSearchTermTimeoutRef.current);
setLastSearchTermSetting?.(searchTerm);
};
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Apr 2, 2026

Choose a reason for hiding this comment

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

🟡 Effect cleanup writes stale search term to project settings on every keystroke

The cleanup function of the search-term persistence effect fires on every searchTerm change (since searchTerm is a dependency). Each cleanup invocation immediately writes the previous search term value to the project setting via setLastSearchTermSetting?.(searchTerm), because the closure captures the value from the previous render. For a user typing "hello", this produces the write sequence: "h", "he", "hel", "hell", then after 1 second "hello" — four unnecessary project-setting writes per word. Since setLastSearchTermSetting writes to a useProjectSetting-backed store (likely involving network/IPC), this creates unnecessary I/O on every keystroke.

Trace of the issue

When searchTerm changes from "a" to "ab":

  1. React runs cleanup from the previous effect: setLastSearchTermSetting?.("a") — immediate write of stale value
  2. React runs the new effect: sets a 1-second timeout for "ab"
  3. 1 second later: setLastSearchTermSetting?.("ab") — final correct value

The cleanup should only clear the timeout; the eager write should be limited to unmount.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

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