ENG-2112 Add sort button with sort dropdown by trangdoan982 · Pull Request #1320 · DiscourseGraphs/discourse-graph

trangdoan982 · 2026-08-20T02:17:17Z

https://www.loom.com/share/2897f29dbdc94b94b742b6cbeb11cd0a
Five client-side sort dimensions for the node search modal — relevance, alphabetical, date created, date modified, author — with Asc/Desc controls, mirroring Roam's sort menu (ENG-1732).

Three things worth a reviewer's attention

Sorting precedes truncation. The modal sliced to MAX_VISIBLE_RESULTS before anything else, so sorting after that slice would have reordered the best-matching 50 rather than all matches. Now: rank (+ type filter) → sort → slice.

Escape needs a pushed keymap scope. Obsidian registers the Modal's close-on-Escape before any plugin React tree exists, so anything added later runs second — stopImmediatePropagation in a React handler, a capture-phase window listener, and registering on the Modal's own scope all fail (the last because Scope resolves in registration order). Keymap.pushScope lands above the modal. #1315 adopted this from the diagnosis here; with the migration it now lives in one place.

The filter migration is why this is stacked on ENG-2111. The sort panel needs behaviour identical to the type filter's, so the choice was one shared shell or a second copy. NodeTypeFilterMenu now renders through SearchDropdown, dropping its duplicate trigger, badge, Escape scope, outside-click, keystroke containment and panel container. Two fixes fall out: its panel had a hardcoded shadow-[0_4px_12px_rgba(0,0,0,0.15)] and now uses --shadow-s, and isTypeFilterOpen became a shared openDropdown, so both panels can no longer be open at once.

Verification

apps/obsidian has no test runner, so verified by driving the real app over CDP against a 269-note vault — 36/36 assertions: all five dimensions and both directions, per-dimension direction defaults, unattributed-last in both directions, sort-before-truncate, Escape sparing the modal, styling (inactive rows match the panel background, shadow matches --shadow-s), the trigger holding one icon across direction changes, and — newly testable on this base — mutual exclusion of the two panels plus all five options honouring an active type filter (16 rows, none off-type).

check-types and lint clean.

Scope check

Ran $scope-check — skill unavailable in this worktree; scope assessed by hand against Done When.
Scope beyond Done When: the sort-before-truncate fix (four of five options are wrong without it); the shared shell and filter migration (see above); Escape via a pushed scope; the single openDropdown state. Nothing in Done When is unverified — including "with and without an active node type filter", which this base makes testable. Size justification in this comment.

🤖 Generated with Claude Code

linear-code · 2026-08-20T02:17:20Z

ENG-2112

vercel · 2026-08-20T02:17:23Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
discourse-graph	Skipped		Aug 22, 2026 4:34am

graphite-app · 2026-08-20T02:17:24Z

PR size/scope check

This PR is over our review-size guideline.

Recommended: ~200 lines changed
Acceptable limit: up to 400 lines when well-scoped/self-contained
Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

What single problem this PR solves
Why the files/changes are coupled

supabase · 2026-08-20T02:17:27Z

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

devin-ai-integration

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

Escape cannot be intercepted from the DOM. Obsidian registers the Modal's close-on-Escape before any plugin React tree exists, so a listener added later always runs second: preventDefault plus stopImmediatePropagation in a React handler does not work, nor does a capture-phase window listener, nor registering on the Modal's own scope, since Scope resolves in registration order. The previous DOM attempt here closed the whole modal instead of the panel. Pushing a Scope while the panel is open lands above the modal in the stack, so the panel gets Escape first. The dead DOM handlers in both the panel and the modal are gone with it. Diagnosis credit to the SearchDropdown work on ENG-2112 (#1320), which fixes the same bug for the sort dropdown. Migrating this component onto that shared shell would drop ~120 duplicated lines and is best done once both land, since SearchDropdown does not exist on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Add the discourse node search surface: a Modal hosting a React root, a result list ranked by the QueryEngine functions from ENG-2108, and a Markdown preview of the active result. Register it as "Open node search" with no default hotkey, so users bind their own and we avoid colliding with core or community bindings. Highlight matched substrings with Obsidian's renderResults, passing the same string that was scored. Using the platform renderer rather than hand-rolled markup means highlights inherit theme styling, which is the code path that produced the equivalent Roam bug. Open with every node listed in title order rather than an empty prompt, so the modal doubles as a node browser. Model candidate loading as a discriminated union covering loading, ready, empty and error; the fetch is synchronous today, but semantic search will make it a network call and threading those states through later costs far more than carrying them now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

renderResults applies Obsidian's suggestion highlight, which is styled for the quick switcher rather than for search. Point it at --text-highlight-bg instead, the variable behind the yellow in Obsidian's own search view, so matches read the same way there, here, and in the Roam implementation. Target the span element rather than Obsidian's internal class name: renderResults wraps matched ranges in spans and leaves unmatched text as bare text nodes, so every span inside the title is a match, and the rule survives a class rename. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The preview pane read the newly selected note asynchronously while `content` still held the previous note's text, so the render effect fired once with the new file's path and the old file's body — the header showed one note while the pane rendered another. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Follows the Roam result row: the node type is a rounded badge of the first three letters, inline before the title, reusing the colors the editor already paints discourse tags with so a type reads the same in both places. Author names now resolve through `plugin.settings.userNames`, which `fetchUserNames` fills with one query for every person in the vault's spaces. The modal refreshes it at most once per open, and only when an imported node is actually missing a name, so nothing queries per result. Resolution also moved to the selected result, which is the only one whose author is displayed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

getNodeTagColors clamped any index past the twelfth node type to 0, so every type beyond the palette length shared one color. Cycling spreads them instead. This also changes existing tag colors for vaults with more than twelve types. Author resolution now distinguishes the two cases the scope doc separates: no authorId means the note is local ("You"), while an authorId that cannot be resolved from settings or Supabase stays "Unknown" rather than claiming local authorship. A non-numeric authorId counts as present-but-unresolvable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Move the arrow-key handler from the search input to the modal container, so navigation keeps working when focus moves elsewhere inside the modal, and so result actions have one place to live when they arrive. Mirrors the Roam dialog, which binds its handler at the same level. Activate rows on hover as well as click, again matching Roam. Suppress the mouseenter that fires when scrolling drags a row under a stationary cursor — that is the list moving, not the user choosing, and honouring it makes arrow keys jump back a row. Prevent the default on mousedown so clicking a result never pulls focus out of the input. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A "?" chip told the reader nothing except that something was wrong. Roam handles the same case by storing the type's label on each result at index time and falling back to that; we have no stored label, but node formats are `PREFIX - {content}`, so the title still carries the prefix the badge would have shown. A note whose type was deleted, or imported from a differently configured vault, now reads QUE or CLM instead of ?. Omit the chip entirely when the title has no prefix either. Abbreviating the note's own words would produce a confident-looking label that says nothing about its type, which is worse than no label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The palette-cycling fix is a real one — past the twelfth node type every type collapsed to a single colour — but it is a behaviour change to a util shared with the editor, and nothing in the search modal needs it: this vault has nine node types, so clamping and cycling agree. Reverted here so the search PR stays to the search surface; worth its own change. Also drop the badge comments that restated their code, keeping the one that explains what Roam does differently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The revert left a whitespace-only diff: the pre-commit formatter collapsed a double blank line the file already had. Committing without it so colorUtils drops out of this PR entirely rather than appearing as a one-line change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A narrowing query rebuilds `results` before the reset effect runs, so the old index could point past the new list for one render — blanking the preview and leaving no row highlighted. Clamping at render covers that frame; the effect still resets the state so arrow keys continue from the top. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ENG-2113 Add platform-aware keyboard hint symbols 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> * ENG-2113 Add footer action bar with open in active pane and split 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> * ENG-2113 Open the active result in a new tab rather than the current one 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> * ENG-2113 Style footer keys as caps, matching Roam 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> * ENG-2113 Drop the results list tooltip 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> * ENG-2113 Make close clickable and drop the duplicate badge tooltip 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> * ENG-2113 Let focused footer buttons handle their own Enter A footer button reached by Tab had its bubbling Enter intercepted by the modal's keydown handler, whose preventDefault suppressed the button's native click. So Enter on close opened a new tab, and Enter on split opened a new tab too. Also moves the footer's layout onto Tailwind utilities, leaving only the four properties Obsidian defends with `button:not(.clickable-icon)` and `button:hover` — both (0,1,1), which outrank a single utility class — plus `font-size`, which has no inherit utility. Trims comments that explained history rather than the code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Adds a type filter beside the node search input. Roam's advanced search is the design and behaviour reference; its pure filter semantics are ported, its BlueprintJS structure is not. The search seam already supported this — rankDiscourseNodesByTitle takes nodeTypeIds and filters before scoring — so this is UI and state only. - discourseNodeTypeFilter.ts ports Roam's semantics, including the canonicalisation that makes "none selected" and "all selected" both mean no filter, matching filterCandidatesByNodeTypeIds. - NodeTypeFilterMenu renders an Obsidian-native trigger (clickable-icon + setIcon) with a count badge, over a panel with checkbox rows, colour dots, per-row Only, Select all with indeterminate state, and a type search past 7 types. - selectedNodeTypeIds lives in NodeSearch as the single source of truth so ENG-2111's chips can share it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…r Clear filter Panel keystrokes no longer reach the modal's key handler. Only Escape was stopped before, so Enter typed in the type search ran the modal's "open the highlighted result" branch — closing the modal and opening an unrelated note — and the arrows moved the result selection. Replaces the "Select all" checkbox with a "Clear filter (n)" button shown only while a filter is active. Because an empty selection and a full one are the same state, the checkbox sat checked and inert whenever nothing was filtered, so clicking it appeared to do nothing. Clearing is the control's only real function, so it now says that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

getSelectAllCheckState and SelectAllCheckState lost their only consumer when the Select all checkbox became the Clear filter button. Roam keeps its own copy, which its tri-state checkbox still uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Escape cannot be intercepted from the DOM. Obsidian registers the Modal's close-on-Escape before any plugin React tree exists, so a listener added later always runs second: preventDefault plus stopImmediatePropagation in a React handler does not work, nor does a capture-phase window listener, nor registering on the Modal's own scope, since Scope resolves in registration order. The previous DOM attempt here closed the whole modal instead of the panel. Pushing a Scope while the panel is open lands above the modal in the stack, so the panel gets Escape first. The dead DOM handlers in both the panel and the modal are gone with it. Diagnosis credit to the SearchDropdown work on ENG-2112 (#1320), which fixes the same bug for the sort dropdown. Migrating this component onto that shared shell would drop ~120 duplicated lines and is best done once both land, since SearchDropdown does not exist on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Chips and the query caret now share one field, so a keyboard-first user can narrow by type without reaching for the mouse. Typing offers the best prefix match as ghost text and Tab commits it to a chip; text left unconfirmed stays a plain keyword query, so filtering never happens by accident. Roam's advanced search is the design and behaviour reference. Obsidian has no tag-input component at all, so this is a build rather than a port: the filter semantics carry over, the BlueprintJS structure does not. - nodeTypeChipCompletion.ts holds the matching rules as pure functions, prefix only — the suggestion is drawn as a completion of what was typed, and a substring match has no suffix to render. - NodeTypeChipsSearchInput writes the raw selectedNodeTypeIds ENG-2110 owns, so chips and the filter dropdown are two views of one state. It skips that module's canonicalisation on purpose: collapsing a full selection to "no filter" would make a chip the user just added vanish. - Chips and the query are inline siblings in a block box, so they flow together left to right and top to bottom and a long query wraps beside the last chip rather than into a column of its own. That rules out an input or a textarea, both of which can only wrap inside their own box, so the query is an editable span. Its text is uncontrolled, since re-rendering it on every keystroke would move the caret. - The Tab hint is inline after the caret rather than an overlay, so it follows the text and wraps with it. It shows whenever Tab would commit, including once the query spells a type name in full and no suffix is left to ghost — Roam hides it there, precisely when the user is about to press it. - Arrow, Enter and Escape are left to bubble to NodeSearch, which already navigates and opens results. Roam forwards them through a prop instead. Enter's default is suppressed here so the editable gains no line break: the modal deliberately leaves modified Enter unhandled for the insert and dock actions. - Backspace on an empty field highlights the last chip and only removes it on a second press, so a stray keystroke cannot silently drop a filter. - Styling is Tailwind throughout and adds no CSS. Two rules this build makes non-obvious: `border-solid` is required because `@tailwind base` is omitted, so nothing sets a default border style and `border` alone renders nothing; and `ring-*` is inert for the same reason, so the focused chip takes an outline. The chip's remove button carries `clickable-icon` to dodge Obsidian's `button:not(.clickable-icon)` rule, which outranks a utility class and would otherwise paint a box behind the ×. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- Chip removal and keyboard bounds used positions in `selectedNodeTypeIds` while the index came from `chips`, the rendered list. The two cannot diverge today — the modal reads settings once with no subscription — but the assumption was load-bearing for nothing, so removal is by id and navigation is bounded by what is actually on screen. - `max-w-40` replaces the inline max-width, per apps/obsidian/AGENTS.md:100. The chip's colours stay inline: they are computed per type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The field's click handler focused the query and forced the caret to the end on every click, including clicks on the text itself — so clicking into the middle of a query dragged the caret to the end. It now runs only for clicks on the field's own padding, where there is no caret position to preserve. Measured: clicking 60% into "hello world" leaves the caret at 7 rather than 11, and clicking the blank padding still places it at the end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Five client-side sort dimensions over the search result list — relevance, alphabetical, date created, date modified, author — with a direction toggle and the active selection shown on the trigger. Sorting runs over the full ranked list and the 50-row truncation comes after it. Sorting the truncated window would have shown the alphabetically-first 50 of the best-matching 50, which is not what any of these options mean. Author resolution moves out of NodeSearchModal into discourseNodeAuthor with its behaviour unchanged, since this is the ticket that owns it. Unattributed notes are pinned last in both directions, so reversing the sort never buries the readable names under a block of "Unknown". The trigger-plus-panel shell lands in SearchDropdown rather than inside the sort menu, because the type filter needs the same outside-click, keystroke containment and Escape behaviour. Escape turned out to be unobtainable from the DOM: Obsidian registers its modal close before any plugin React tree exists, so a listener added later runs second at every phase and on every node, and registering on the modal's own scope loses to the built-in handler for the same reason. Pushing a scope while a panel is open is the one place that gets Escape first. Verified against a real vault over CDP: 25/25 assertions covering all five dimensions, both directions, the unattributed-last rule, sort-before-truncate, and Escape closing the panel without closing the modal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The option rows were rendered as buttons, so Obsidian's button chrome gave each one its own filled, padded box — five separate widgets stacked in a panel rather than a menu. Rows are now flat divs on the panel's own background that react to hover, with the active row carrying the accent fill and the check. Follows Roam's sort menu (ENG-1732) the rest of the way: a "Sort by" header, and explicit Asc / Desc controls instead of one toggle whose label changed with the dimension. The trigger goes back to icon-only, as in Roam, with the direction arrow and the accent highlight carrying the active state and the full phrase moving to the tooltip. Verified over CDP: 29/29, including that an inactive row's background matches the panel's and that only the active row is coloured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The panel shadow and badge foreground were hardcoded, against the rule in apps/obsidian/AGENTS.md. Both now come from Obsidian: the badge uses the mapped `text-on-accent` token, and the shadow uses `--shadow-s`. The shadow needs Tailwind's `shadow:` type hint. Without it, `shadow-[var(--shadow-s)]` is ambiguous and Tailwind resolves it as a shadow *colour* — it emits `--tw-shadow-color` and no `box-shadow` at all, so the class is inert and the panel renders with no shadow. `ModifyNodeModal.tsx:500` has the same silently-broken usage; not touched here. Verified over CDP: the panel's computed box-shadow now ends with exactly the layers a probe element carrying `var(--shadow-s)` produces. 30/30. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Every comment added on this branch is now a single line. Behaviour unchanged; 30/30 still passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Stacking on ENG-2111 makes the deduplication its own commit message deferred possible. NodeTypeFilterMenu keeps its rows, colours and type search and gives up everything it shared with the sort menu: the trigger button and count badge, the pushed Escape scope, the outside-mousedown close, the keystroke containment, the positioned panel container, and its local FilterIcon copy. 291 lines down to 220. Two fixes fall out of it. The panel container it dropped had the hardcoded `shadow-[0_4px_12px_rgba(0,0,0,0.15)]`, so the filter now takes its shadow from `--shadow-s` like everything else. And `isTypeFilterOpen` becomes `openDropdown === "type-filter"`, so the filter and sort panels can no longer be open simultaneously. Verified over CDP, 35/35, with five assertions added for what this stack now covers: neither panel can be open while the other is, Escape still spares the modal after the migration, the trigger badges its count, and all five sort options honour an active type filter — 16 rows of the filtered type, none off type, identical under every dimension. That last one was the Done When criterion ENG-2109 could not exercise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The repo already had a pattern for this — every other call site writes `ref={(el) => (el && setIcon(el, "name")) || undefined}` inline — so the wrapper was a new abstraction over three call sites rather than reuse. The one behaviour worth keeping is the empty-before-write, and only for SearchDropdown's trigger, whose icon name changes with the control's state: `setIcon` appends and React reuses the host node, so without it the arrows stack up. The sort menu's check and direction icons have a name fixed per element, so they use the repo's one-liner. 36/36, with a new assertion that the trigger still holds exactly one icon after repeated direction changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

CI's eslint flagged the cast as an unnecessary assertion: `FrontMatterCache` indexes to `any`, so asserting it to `Record<string, unknown> | undefined` does not change the type. Annotating the local instead keeps the read typed as `unknown` for the caller and leaves nothing for the rule to fire on. Carried over verbatim from ENG-2109 when this function moved into its own util, which is why it surfaced here. 36/36 unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Cuts the eleven comments that restated the code they sat above — prop descriptions the identifier already gave, a label for the shell's own file, the ternary conditions in the sort menu and the results memo. What survives is what someone could otherwise get wrong: the Obsidian gotchas (Escape needs a pushed scope, `activeDocument` for popouts, `setIcon` appends), the invariants (sort precedes truncation, unattributed last in both directions, the partition sits outside the direction flip), and the deliberate deviations (rows are divs, not buttons; the frontmatter read is annotated, not asserted). 24 comments down to 13. 36/36 unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ENG-2110 Add node type filter dropdown menu Adds a type filter beside the node search input. Roam's advanced search is the design and behaviour reference; its pure filter semantics are ported, its BlueprintJS structure is not. The search seam already supported this — rankDiscourseNodesByTitle takes nodeTypeIds and filters before scoring — so this is UI and state only. - discourseNodeTypeFilter.ts ports Roam's semantics, including the canonicalisation that makes "none selected" and "all selected" both mean no filter, matching filterCandidatesByNodeTypeIds. - NodeTypeFilterMenu renders an Obsidian-native trigger (clickable-icon + setIcon) with a count badge, over a panel with checkbox rows, colour dots, per-row Only, Select all with indeterminate state, and a type search past 7 types. - selectedNodeTypeIds lives in NodeSearch as the single source of truth so ENG-2111's chips can share it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2110 Address review: contain panel keystrokes, swap Select all for Clear filter Panel keystrokes no longer reach the modal's key handler. Only Escape was stopped before, so Enter typed in the type search ran the modal's "open the highlighted result" branch — closing the modal and opening an unrelated note — and the arrows moved the result selection. Replaces the "Select all" checkbox with a "Clear filter (n)" button shown only while a filter is active. Because an empty selection and a full one are the same state, the checkbox sat checked and inert whenever nothing was filtered, so clicking it appeared to do nothing. Clearing is the control's only real function, so it now says that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2110 Drop the unused select-all check state helper getSelectAllCheckState and SelectAllCheckState lost their only consumer when the Select all checkbox became the Clear filter button. Roam keeps its own copy, which its tri-state checkbox still uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2110 Close the filter panel on Escape via a pushed keymap scope Escape cannot be intercepted from the DOM. Obsidian registers the Modal's close-on-Escape before any plugin React tree exists, so a listener added later always runs second: preventDefault plus stopImmediatePropagation in a React handler does not work, nor does a capture-phase window listener, nor registering on the Modal's own scope, since Scope resolves in registration order. The previous DOM attempt here closed the whole modal instead of the panel. Pushing a Scope while the panel is open lands above the modal in the stack, so the panel gets Escape first. The dead DOM handlers in both the panel and the modal are gone with it. Diagnosis credit to the SearchDropdown work on ENG-2112 (#1320), which fixes the same bug for the sort dropdown. Migrating this component onto that shared shell would drop ~120 duplicated lines and is best done once both land, since SearchDropdown does not exist on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2110 Address review: tighten filter comments Names ENG-2111 instead of the internal "F6" shorthand, notes the search threshold is tuned for the desktop-only modal, and drops the vague "stored empty set" phrasing. One line each. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

devin-ai-integration Bot reviewed Aug 20, 2026

View reviewed changes

Comment thread apps/obsidian/src/utils/discourseNodeSort.ts

Comment thread apps/obsidian/src/components/SearchDropdown.tsx Outdated

vercel Bot temporarily deployed to Preview August 20, 2026 02:34 Inactive

vercel Bot temporarily deployed to Preview August 20, 2026 02:40 Inactive

trangdoan982 mentioned this pull request Aug 20, 2026

ENG-2110 Add node type filter dropdown menu #1315

Merged

trangdoan982 force-pushed the eng-2112-add-sort-button-with-sort-dropdown branch from 5d023c7 to a255058 Compare August 20, 2026 16:49

trangdoan982 changed the base branch from eng-2109-create-node-search-modal-with-ranked-results-and-preview to eng-2111-add-keyboard-only-node-type-filtering-with-tag-chips August 20, 2026 16:49

vercel Bot temporarily deployed to Preview August 20, 2026 16:49 Inactive

graphite-app Bot reviewed Aug 20, 2026

View reviewed changes

Comment thread apps/obsidian/src/components/SearchDropdown.tsx

vercel Bot temporarily deployed to Preview August 21, 2026 16:06 Inactive

trangdoan982 force-pushed the eng-2112-add-sort-button-with-sort-dropdown branch from 3cd4dca to 72e8f4b Compare August 21, 2026 21:51

vercel Bot temporarily deployed to Preview August 21, 2026 21:51 Inactive

vercel Bot temporarily deployed to Preview August 22, 2026 04:04 Inactive

trangdoan982 requested a review from mdroidian August 22, 2026 04:26

trangdoan982 force-pushed the eng-2112-add-sort-button-with-sort-dropdown branch from 19ed572 to 5f21839 Compare August 22, 2026 04:27

vercel Bot temporarily deployed to Preview August 22, 2026 04:27 Inactive

trangdoan982 and others added 8 commits August 22, 2026 00:34

Tighten comments on the node type badge and author name cache

39da641

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

trangdoan982 and others added 21 commits August 22, 2026 00:34

ENG-2109 Drop the remaining explanatory comments

447dd27

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ENG-2109 Address review: pointer guard, modal sizing utilities

470df4f

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ENG-2112 Reduce comments to one line each

b7a106d

Every comment added on this branch is now a single line. Behaviour unchanged; 30/30 still passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

trangdoan982 force-pushed the eng-2112-add-sort-button-with-sort-dropdown branch from 5f21839 to a4c0e6b Compare August 22, 2026 04:34

vercel Bot temporarily deployed to Preview August 22, 2026 04:34 Inactive

Base automatically changed from eng-2111-add-keyboard-only-node-type-filtering-with-tag-chips to main August 22, 2026 04:51

Navigation Menu

Uh oh!

ENG-2112 Add sort button with sort dropdown - #1320

ENG-2112 Add sort button with sort dropdown#1320
trangdoan982 wants to merge 29 commits into
mainfrom
eng-2112-add-sort-button-with-sort-dropdown

trangdoan982 commented Aug 20, 2026 •

edited

Loading

Uh oh!

linear-code Bot commented Aug 20, 2026

Uh oh!

vercel Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

graphite-app Bot commented Aug 20, 2026

Uh oh!

supabase Bot commented Aug 20, 2026

Uh oh!

devin-ai-integration Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

trangdoan982 commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Three things worth a reviewer's attention

Verification

Scope check

Uh oh!

linear-code Bot commented Aug 20, 2026

Uh oh!

vercel Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app Bot commented Aug 20, 2026

PR size/scope check

Uh oh!

supabase Bot commented Aug 20, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

trangdoan982 commented Aug 20, 2026 •

edited

Loading

vercel Bot commented Aug 20, 2026 •

edited

Loading