Repository navigation
ENG-2265 Add a preview toggle to advanced node search - #1561
Open
trangdoan982 wants to merge 6 commits into
Open
trangdoan982 wants to merge 6 commits into
trangdoan982 wants to merge 6 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M553MJAJGAZENTTPQS6XNF25
Move shared caret and key helpers into utils, restore focus when the focused list unmounts, and share one canAct gate between → and the footer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M555GDHXSDER5W65G8H13XB0
Keep isPlainCharacterKey beside its only caller in the chip input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M555VDTK8PCFF5EDKM8R6R9H
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M55E3ZQ8GRR7X8TNBJSRATKS
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M55ED9K2G71M0ZSJTY2RHQK8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M55EGXF6KJM9FVEDCJ59MFYW
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
trangdoan982
commented
Oct 17, 2026
Comment on lines
+666
to
+670
| onFocus={(event) => | ||
| setIsResultListFocused(event.target === listRef.current) | ||
| } | ||
| // Focus can leave to non-focusable modal areas, which fire no focus event here. | ||
| onBlur={() => setIsResultListFocused(false)} |
Member
Author
There was a problem hiding this comment.
Why the preview toggle only appears while the result list has focus:
- In the query input, ←/→ stay caret movement, so the preview shortcut can only work while the result list has focus. Clicking a result gives it focus.
- The footer should only advertise a key where that key works. Showing "→ preview" while the user types would point at a key that moves the caret instead.
onFocusrecords whether the list has focus.onBlurclears it, because clicking somewhere in the modal that can't take focus (for example the preview text) fires no focus event inside this wrapper. Without it, the toggle would stay visible after ←/→ stopped working.
Trade-off: a keyboard-only user can't open the preview, because only a click focuses the list. It's listed under Risk in the PR description.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewer brief
handleKeyDown(NodeSearchModal.tsx). With the list focused, every key outsideRESULT_LIST_KEYSmoves focus to the query and is left unprevented, so typing, Backspace and paste land there. Backspace on an empty query is replayed on the query field so the chip input still highlights the last chip.border-0 border-r border-solid. apps/obsidian ships no Tailwind preflight, soborder-ralone draws nothing. The split view onmainhad the same invisible divider.aria-activedescendant.prompt-instructionson mobile, the preview can't be opened there.Verification
2e717d86. All 13 pass:pnpm install --frozen-lockfile && pnpm ci:validatepasses: 23/23 Obsidian test files (227 tests), with typecheck and lint clean.Loom video
https://www.loom.com/edit/309af51172d840e7ac2eae4d98ca1217
Scope check
$scope-checkagainst ENG-2265 and the final diff.Done When: The preview opens with → only while the result list has focus, which a click on a result gives it, not "once the cursor is at the end of the query" as the ticket's Solution says. To support that, the result list can take focus, keys it does not use (typing, Backspace, paste) go back to the query, Backspace on an empty query still highlights the last filter chip, the footer toggle shows only while the list has focus, and focus returns to the query if the list disappears while focused.setCaretToEndmoved intosrc/utilsfor its second caller.Standards check
$dg-pr-adherence-checkagainst the final diff and PR metadata.Local delegated full review
$dg-delegated-full-reviewwhen no other full-review workflow is available.🤖 Generated with Claude Code