Skip to content

ENG-2110 Add node type filter dropdown menu - #1315

Merged
trangdoan982 merged 5 commits into
mainfrom
eng-2110-add-node-type-filter-dropdown-menu
Aug 22, 2026
Merged

ENG-2110 Add node type filter dropdown menu#1315
trangdoan982 merged 5 commits into
mainfrom
eng-2110-add-node-type-filter-dropdown-menu

Conversation

@trangdoan982

@trangdoan982 trangdoan982 commented Aug 19, 2026

Copy link
Copy Markdown
Member

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 to string[] ids and Obsidian's DiscourseNode (.id/.name rather than .type/.text). The piece worth attention is toPanelSelectedIds/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, and filterCandidatesByNodeTypeIds agreeing 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, a Clear filter (n) button while a filter is active, and a type-search input past 7 types. Reuses getAllDiscourseNodeColors for the dots so palette fallbacks stay index-stable. Outside-click uses activeDocument, following InlineNodeTypePicker, so it works in a popout window.

apps/obsidian/src/components/NodeSearchModal.tsxselectedNodeTypeIds state 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 filter rather than Select 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

@linear-code

linear-code Bot commented Aug 19, 2026

Copy link
Copy Markdown

ENG-2110

@vercel

vercel Bot commented Aug 19, 2026

Copy link
Copy Markdown

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

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

Request Review

@supabase

supabase Bot commented Aug 19, 2026

Copy link
Copy Markdown

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-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread apps/obsidian/src/components/NodeTypeFilterMenu.tsx
Comment thread apps/obsidian/src/components/NodeTypeFilterMenu.tsx Outdated
Comment thread apps/obsidian/src/components/NodeSearchModal.tsx Outdated
Comment thread apps/obsidian/src/utils/discourseNodeTypeFilter.ts Outdated
Comment thread apps/obsidian/src/utils/discourseNodeTypeFilter.ts Outdated
@trangdoan982
trangdoan982 force-pushed the eng-2110-add-node-type-filter-dropdown-menu branch from 830b85d to d8ab028 Compare August 22, 2026 04:27
@trangdoan982
trangdoan982 force-pushed the eng-2110-add-node-type-filter-dropdown-menu branch from d8ab028 to bf53409 Compare August 22, 2026 04:34
Base automatically changed from eng-2109-create-node-search-modal-with-ranked-results-and-preview to main August 22, 2026 04:39
trangdoan982 and others added 4 commits August 22, 2026 00: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>
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>
@trangdoan982
trangdoan982 merged commit 1341e22 into main Aug 22, 2026
8 checks passed
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.

2 participants