Skip to content

Add disableSelect prop (rebased from #169)#331

Merged
TrevorBurnham merged 11 commits intomainfrom
disable-select
May 4, 2026
Merged

Add disableSelect prop (rebased from #169)#331
TrevorBurnham merged 11 commits intomainfrom
disable-select

Conversation

@TrevorBurnham
Copy link
Copy Markdown
Collaborator

@TrevorBurnham TrevorBurnham commented May 3, 2026

Resurrects @boriskor's #169, rebased onto current main with style cleanup. Closes #56.

Summary

Adds a disableSelect Tree prop in the spirit of disableEdit and disableDrag — accepts string | boolean | BoolFunc<T>. When the predicate returns true for a node, that node can't be selected via click, keyboard, selectionFollowsFocus, selectAll, or any of the imperative select methods. Focus still moves through the disabled node (so keyboard navigation isn't blocked); only selection is gated.

Closes #56, which @jameskerr opened in 2022 asking for exactly this API.

What changed vs #169

  • Rebased onto current main (resolved conflicts with Add options argument to selectMulti for consistency with select #266's selectMulti(opts) and the isActionPossible refactor).
  • Style cleanup commit on top: matched repo formatting in filterSelectableNodes / selectContiguous, replaced an as NodeApi<T>[] cast with a type-guarded filter predicate.
  • Original commits preserved with @boriskor as author.

Coverage

  • Gates select, selectMulti, selectContiguous, selectAll. selectionFollowsFocus flows through select so it's gated too.
  • setSelection (imperative escape hatch) is intentionally not gated.
  • Adds node.isSelectable getter on NodeApi.
  • Cypress e2e tests against the Gmail showcase (covers click + select-all behaviors).
  • Showcase updated to demo with Categories/Spam disabled.
  • README documents the new prop.

Test plan

  • CI green (build + existing tests)
  • e2e: clicking Categories doesn't select it; cmd-A skips Categories and Spam
  • Manual: arrow-key navigation still passes through a disabled node when selectionFollowsFocus is on, but the node doesn't get selected
  • Manual: existing selection is preserved when clicking a disabled node (not cleared)

🤖 Generated with Claude Code

Boris Korneev and others added 10 commits May 3, 2026 17:27
Copilot AI review requested due to automatic review settings May 3, 2026 22:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a disableSelect prop to Tree (matching the existing disableEdit / disableDrag patterns) to prevent certain nodes from being selectable while still allowing focus/navigation through them.

Changes:

  • Introduces disableSelect?: string | boolean | BoolFunc<T> to TreeProps and documents it in the README.
  • Gates selection entry points (select, selectMulti, selectContiguous, selectAll) based on a new node.isSelectable getter.
  • Updates the Gmail showcase + Cypress e2e coverage to demonstrate and validate unselectable nodes.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
modules/showcase/pages/gmail.tsx Demonstrates disableSelect by preventing selection of “Categories” and “Spam”.
modules/react-arborist/src/types/tree-props.ts Adds disableSelect to the public TreeProps type.
modules/react-arborist/src/interfaces/tree-api.ts Implements select-gating and introduces isSelectable() / filterSelectableNodes().
modules/react-arborist/src/interfaces/node-api.ts Adds node.isSelectable getter.
modules/e2e/cypress/e2e/gmail-spec.cy.js Adds e2e tests for click selection + select-all behavior with unselectable nodes.
README.md Documents the new disableSelect prop in the TreeProps interface section.
CONTRIBUTING.md Tweaks local testing instructions formatting.
Comments suppressed due to low confidence (1)

modules/react-arborist/src/interfaces/tree-api.ts:342

  • select() now calls setSelection() (which already invokes props.onSelect) and then invokes props.onSelect again at the end of select(). This results in duplicate onSelect callbacks for a single selection change, which can cause double side-effects for consumers. Consider either removing the onSelect call inside setSelection, or making select() use dispatches directly / return early so onSelect is only fired once per action.
    if (this.get(id)?.isSelectable) {
      this.setSelection({
        ids: [id],
        anchor: id,
        mostRecent: id,
      });
    }
    this.scrollTo(id, opts.align);
    if (this.focusedNode && changeFocus) {
      safeRun(this.props.onFocus, this.focusedNode);
    }
    safeRun(this.props.onSelect, this.selectedNodes);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/react-arborist/src/interfaces/tree-api.ts
Comment thread modules/e2e/cypress/e2e/gmail-spec.cy.js
Comment thread CONTRIBUTING.md Outdated
Comment thread modules/react-arborist/src/interfaces/tree-api.ts
- select(): drop redundant onSelect call (setSelection already fires it,
  so previously every selection produced two callbacks)
- selectAll(): derive anchor/mostRecent from the selectable subset so
  shift-click ranges don't start from a non-selected node
- e2e: assert existing Inbox selection survives a click on Categories
- CONTRIBUTING.md: give the localhost autolink an http:// scheme so it
  renders as a link

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@TrevorBurnham
Copy link
Copy Markdown
Collaborator Author

Pushed fd9ab93 addressing Copilot's review:

  • select() double onSelect (suppressed comment) — dropped the trailing safeRun(onSelect, ...) since setSelection already fires it. Was a regression introduced by this PR.
  • selectAll() anchor/mostRecent (Tree.onSelect #1) — derived from the selectable subset (null when none are selectable) so subsequent shift-click ranges don't start from a non-selected node.
  • e2e Inbox-survives-Categories-click (If a meta key is held, don't toggle the group. #2) — added the assertion that existing selection is preserved after clicking an unselectable node.
  • CONTRIBUTING.md autolink (TreeApi.selectById(id) #3) — added http:// scheme so it renders as a link.

Not addressed (left as a follow-up to keep this PR scoped):

@TrevorBurnham
Copy link
Copy Markdown
Collaborator Author

Follow-up cleanup tracked in #332.

@TrevorBurnham TrevorBurnham merged commit dc45120 into main May 4, 2026
1 check 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.

Easy api to prevent nodes from being selectable

2 participants