ENG-2113 Add footer action bar with open in new tab and split - #1292
Conversation
Obsidian renders modifiers as glyphs on macOS and as words on Windows and Linux. Roam's search footer hardcoded the macOS glyphs at each call site and showed the wrong hint on Windows (ENG-2000); routing every hint through one map is what keeps that from repeating. `formatHintKeys` takes `isMacOS` so the non-mac branch can be exercised without that platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Enter opens the active result in the current pane, Shift+Enter in a split, and both close the modal. Mod+Enter and Alt+Enter deliberately fall through, so the insert action (ENG-2114) can claim Mod+Enter as it does in Roam. The footer reuses Obsidian's own `prompt-instruction` markup, the classes `SuggestModal.setInstructions()` emits, so it matches the native quick switcher. This modal extends plain `Modal`, so that API is unavailable. Its actions are left-aligned rather than centred because they sit under a full-width result list. The Enter branch lives in the existing wrapper `onKeyDown`, which ENG-2109 moved off the input so result actions would have one place to live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Replacing the page the user was already reading loses their place, which is the
opposite of what a lookup surface should do. `getLeaf("tab")` adds a tab to the
main panel instead, so the previous note stays open behind it.
This reuses the existing `openFileInNewTab`, so the `openFileInActivePane`
helper added earlier in this branch is no longer needed. The label now reads
"open in new tab" to match.
Diverges from the ticket's stated Solution, which specified `getLeaf(false)`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
trangdoan982
changed the title
Obsidian's `prompt-instruction-command` is bold with no border, which made the lone `esc` hint read as emphasis rather than as a key. Roam's search footer draws every key as a bordered cap instead, so `esc` sits with the rest of the set. Keeps the `prompt-instructions` container for its native type and spacing, and takes the cap's border, radius, and background from Obsidian's CSS variables so it still follows the active theme. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Obsidian renders `aria-label` as a hover tooltip, so labelling the listbox meant a tooltip covered the results as soon as the pointer entered the list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The close hint was the only footer item that ignored a click, which read as broken next to two working actions. It now goes through the same `FooterAction` as the others and calls the modal's own close. The badge carried both `title` and `aria-label` with the same text, so hovering one stacked a native tooltip on top of Obsidian's. Keeping `aria-label`, since Obsidian's is the themed one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 005b5a2036
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| event.preventDefault(); | ||
| moveActiveIndex(event.key === "ArrowDown" ? 1 : -1); | ||
| openActiveResult(event.shiftKey ? openFileInNewLeaf : openFileInNewTab); |
There was a problem hiding this comment.
When a keyboard user Tabs into a footer button, its bubbling Enter keydown is still intercepted here, preventDefault() suppresses the button's native click, and the selection shortcut runs instead. Consequently, Enter on the close button opens the active result in a new tab, while Enter on the split button also opens a new tab; ignore Enter events originating from footer buttons or scope this shortcut to the search input.
Useful? React with 👍 / 👎.
| onClick: () => void; | ||
| }; | ||
|
|
||
| // Roam's footer renders each key as a bordered cap rather than bold text, which |
There was a problem hiding this comment.
I'm not sure what this comment is supposed to be informing me. Was there a different choice made in Obsidian as opposed to Roam that this comment is trying to infer about?
| className="prompt-instruction dg-search-footer-action" | ||
| disabled={disabled} | ||
| onClick={onClick} | ||
| // Same reason as the result rows: keep focus in the query input so arrow-key |
| event.preventDefault(); | ||
| moveActiveIndex(event.key === "ArrowDown" ? 1 : -1); | ||
| openActiveResult(event.shiftKey ? openFileInNewLeaf : openFileInNewTab); |
| this footer sits under a full-width result list, so the actions line up with | ||
| its left edge instead. `flex-shrink` keeps the footer from collapsing inside | ||
| the fixed-height flex column above it. */ | ||
| .dg-node-search-modal .dg-search-footer { |
| export type HintKey = "Mod" | "Alt" | "Shift" | "Enter" | "Escape"; | ||
|
|
||
| // Obsidian shows glyphs on macOS and spelled-out words everywhere else, so the | ||
| // same shortcut has to render two ways. Roam's search footer hardcoded the macOS |
There was a problem hiding this comment.
Some unnecessary comments here
2 participants
https://www.loom.com/share/85492db69f9f4c0d92f09d3234d64eb7
Scope check
$scope-checkagainst ENG-2113 and the final diff.Done When: two deliberate deviations, both cosmetic-to-small.Done Whenonly requires that "both actions open the correct note and close the modal", so this stays inside the acceptance boundary — but it contradicts the ticket'sSolution, which specifiedapp.workspace.getLeaf(false), and the ticket title. Replacing the note the user was already reading loses their place, which is the opposite of what a lookup surface should do. The footer label reads "open in new tab" to match.esc closehint. A third, non-clickable footer item for Escape, which Obsidian's modal scope already handles. Included so the footer matches Roam'sAdvancedSearchFooterand the native quick switcher, both of which show one.getLeaf(false)first would mean changing it immediately. (2) No, easily dropped.Solutionand title should be updated to say "new tab" rather than "active pane" — flagging rather than editing the ticket myself.Stacked on #1285 (ENG-2109) — review that first. Base is its branch, not
main.What this does
F8.
Enteropens the active result in a new tab in the main panel,Shift+Enteropens it in a split, both close the modal, and both are clickable in a new footer.Mod+EnterandAlt+Enterdeliberately fall through untouched so ENG-2114's insert-at-cursor can claimMod+Enter, the same combo Roam uses.Shift+Enterhas no native conflict: it means "create new file" only inside Obsidian's own quick switcher, which this modal does not offer.The Enter branch lives in the existing wrapper
onKeyDown— ENG-2109 moved that handler off the input specifically so result actions would have one place to live. It also guardsisComposing, so committing an IME candidate never opens a file.Both open helpers (
openFileInNewTab,openFileInNewLeaf) already existed and are reused unchanged, so no file outside the search feature is modified.Avoiding ENG-2000
Roam's footer hardcodes Blueprint's macOS glyph icons per action (
keyIcons={["key-option", "key-enter"]}), so Windows users see ⌘/⌥ while the handler actually accepts Ctrl. Rather than port that shape, every hint here goes through onekeyboardHints.tsmap keyed onPlatform.isMacOS. No call site can name a platform-specific symbol.formatHintKeystakesisMacOSas a parameter so the Windows/Linux branch is verifiable without that platform:Native styling
prompt-instructions/prompt-instruction/prompt-instruction-commandclasses — whatSuggestModal.setInstructions()emits.Beyond that it only needed
flex-shrink: 0(the.modal-contentcolumn is fixed-height withoverflow: hidden) and a reset for Obsidian's globalbuttonchrome.🤖 Generated with Claude Code