Skip to content

fix(explorer): make Backspace go up a level - #1500

Open
addyCooks wants to merge 3 commits into
Nano-Collective:mainfrom
addyCooks:fix/explorer-go-up-1455
Open

addyCooks wants to merge 3 commits into
Nano-Collective:mainfrom
addyCooks:fix/explorer-go-up-1455

Conversation

@addyCooks

@addyCooks addyCooks commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1455 (part of #907)

Description

4002073 made the physical Backspace key reach handleGoUp in tree mode (key.backspace || key.delete), which was the root cause named in the issue. But the handler behind it still didn't do what the issue expects ("collapses the current directory / moves the selection up a level"), so the issue's own repro steps still did nothing:

  • It collapsed the parent of the highlighted node, not the node. After Enter expands a directory the selection stays on it, so Backspace on a top-level directory was a no-op (no parent). On a nested one it collapsed the parent instead, hiding the directory you had just opened.
  • It never moved the selection. After a collapse the selection kept its index and landed on whatever row now sat there.
  • It never worked on Windows. It split node.path on /, but buildFileTree builds paths with path.join, which uses \ there, so it never found a parent.
  • It wasn't discoverable. The tree-view help line didn't mention it.

Backspace now follows the usual tree-view convention:

  • On an open directory, it collapses that directory and keeps the selection.
  • Anywhere else, it moves the selection to the parent row, found with path.dirname, which splits on the same separator path.join used to build the path.
File Change
source/components/file-explorer/index.tsx handleGoUp collapses an open directory, otherwise selects the parent row; help line lists Backspace: up
source/components/file-explorer/index.spec.tsx Regression test driving the real component; help-line stand-in updated
docs/features/file-explorer.md Backspace row describes the new behaviour

Recording

B32-FIXED-explorer-backspace-goes-up cording

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (.changeset/explorer-backspace-go-up.md, patch)

Testing

  • New test renders the real FileExplorer in a temp directory (outer/inner/file.txt, top.txt), opens both directories, highlights the file, then presses Backspace four times: file → inner (still open) → inner collapsed → outer → outer collapsed. It checks the status-bar path and visible rows at each step. On main it fails: after Backspace on the file, the frame is unchanged.
  • file-explorer/index (20/20), tree-item, utils and file-tree specs pass
  • tsc, Biome format and lint, and knip clean

Checklist

  • I was assigned to the issue
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated
  • No breaking changes

Backspace already reaches handleGoUp (4002073), but handleGoUp collapsed
the parent of the highlighted node: a no-op on a top-level directory, it
hid the directory just opened on a nested one, left the selection on an
unrelated row, and split paths on '/' although buildFileTree joins them
with the platform separator, so on Windows it never did anything.

Collapse the highlighted directory when it is open, otherwise move the
selection to the parent row found with path.dirname. List the key in the
tree view's help line and update the docs table.

Closes Nano-Collective#1455
@github-actions github-actions Bot added area:tui Terminal UI area:docs Documentation labels Sep 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

nc-review: comments — 2 nits

@addyCooks — a few things worth a look, none blocking.

PR #1500 correctly addresses issue #1455 and the deeper issues the author uncovered: Backspace in /explorer tree mode now (a) accepts both key.backspace and key.delete so mainstream terminals reach it, (b) collapses an open directory or moves the selection to the parent row, (c) uses path.dirname so it works on Windows where buildFileTree joins paths with \, and (d) is advertised in the help line and docs. The implementation is straightforward and the new regression test exercises the real component against a tmpdir fixture. One small design point worth flagging: the new behavior treats a file the same as a collapsed directory and always moves the selection to the parent — but on a top-level file that means a no-op, with no audible signal. Not a bug, just worth a thought.

⚪ nit · design · source/components/file-explorer/index.tsx:256

handleGoUp silently no-ops on a top-level file: dirname('top.txt') is ., no item in filteredList has that path, and the user gets no feedback. The same is true of the existing behavior on a top-level directory, so this is not a regression — just a place where the new logic inherits an old silence. If the team wants polish, a single line like if (parentPath === '.') return; plus an opt-in status hint would do it; otherwise leave it.

⚪ nit · tests · source/components/file-explorer/index.spec.tsx:460

The new test relies on a hard-coded 50ms / 30ms delay around stdin.write and a 3s poll. The comment explains why the delay is needed (stale closure on the previous render's handler), which is good — but the chosen constants are not justified. On a slow runner the 50ms could be too tight; on a fast one the cumulative 80ms × 7 presses slows the suite. Not worth blocking on; if it ever flakes, the right fix is to wait for the post-update lastFrame() to settle rather than to bump the constant.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:comments nc-review left non-blocking findings label Sep 25, 2026
@addyCooks

Copy link
Copy Markdown
Collaborator Author

Heads up for review: this builds on 4002073, which already routes Backspace to handleGoUp, so the diff only touches what that handler does. The new test fails on main at the first Backspace (the frame is unchanged, because the / split finds no parent in a Windows path), and passes with the fix.

This also touches the end of file-explorer/index.spec.tsx, like my PR for #1454 does. The import lines are identical, but whichever merges second will need a trivial "keep both tests" resolution. Happy to rebase whichever lands last.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings area:docs Documentation area:tui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] /explorer: "go up a directory" is bound to Backspace, a key mainstream terminals never actually send

1 participant