ENG-2110 Add node type filter dropdown menu - #1315
Merged
Merged
Conversation
|
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. |
1 task
mdroidian
approved these changes
Aug 21, 2026
trangdoan982
force-pushed
the
eng-2110-add-node-type-filter-dropdown-menu
branch
from
August 22, 2026 04:27
830b85d to
d8ab028
Compare
trangdoan982
force-pushed
the
eng-2110-add-node-type-filter-dropdown-menu
branch
from
August 22, 2026 04:34
d8ab028 to
bf53409
Compare
Base automatically changed from
eng-2109-create-node-search-modal-with-ranked-results-and-preview
to
main
August 22, 2026 04:39
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>
trangdoan982
force-pushed
the
eng-2110-add-node-type-filter-dropdown-menu
branch
from
August 22, 2026 04:39
bf53409 to
5360a8d
Compare
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>
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
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.
https://www.loom.com/share/f8349cfbe61f447c80986c968f7b9547
What this does
Adds a node type filter beside the search input in the node search modal. Selecting types narrows results immediately, the active filter shows as a count badge on the trigger, clearing restores the full set, and multiple types return the union.
Changes
apps/obsidian/src/utils/discourseNodeTypeFilter.ts(new) — Roam's semantics ported, narrowed tostring[]ids and Obsidian'sDiscourseNode(.id/.namerather than.type/.text). The piece worth attention istoPanelSelectedIds/fromPanelSelectedIds: the stored empty set means "no filter", which the panel must render as every row checked, and selecting all normalises back to empty. That keeps the stored state, the count badge, andfilterCandidatesByNodeTypeIdsagreeing on what counts as a filter.apps/obsidian/src/components/NodeTypeFilterMenu.tsx(new) — Obsidian-native trigger (clickable-icon+setIcon) with a count badge, over a panel with checkbox rows, colour dots, a per-row "Only" button, aClear filter (n)button while a filter is active, and a type-search input past 7 types. ReusesgetAllDiscourseNodeColorsfor the dots so palette fallbacks stay index-stable. Outside-click usesactiveDocument, followingInlineNodeTypePicker, so it works in a popout window.apps/obsidian/src/components/NodeSearchModal.tsx—selectedNodeTypeIdsstate as the single source of truth so ENG-2111's chips can read and write the same state; trigger inline-right of the input, leaving the row beneath free for those chips; focus returns to the input when the panel closes.Two behaviours worth understanding before reviewing
Clear filterrather thanSelect all. Roam has a tri-state "Select all" checkbox here, and it is the one place this PR knowingly diverges. Because an empty selection and a full one are the same state by design, that checkbox sits checked and inert whenever no filter is active, so clicking it appears to do nothing. Clearing is the control's only real function, so it now says so and appears only when there is something to clear.🤖 Generated with Claude Code