fix(sftp): add breadcrumb go-to-root button beside home icon - #3212
fix(sftp): add breadcrumb go-to-root button beside home icon#3212netcatty-bot wants to merge 1 commit into
Conversation
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex reported no major issues. This PR is marked ready for human review/merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3adf86bcd9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return isWindowsPath(path, pathOptions) | ||
| ? isWindowsRoot(path, pathOptions) | ||
| : /^\/{1,2}$/.test(path); |
There was a problem hiding this comment.
Keep the root action enabled for bare
// paths
When a POSIX pane is at the accepted path //, this regex marks it as already at root and disables the new button, even though rootPath is /. The path utilities deliberately preserve double-slash POSIX paths and getParentPath("//") transitions to /, so users in this state cannot use the new control to reach the actual filesystem root. Only disable it when the current POSIX path resolves to the same path as the root target.
Useful? React with 👍 / 👎.
Summary
/; on Windows-style panes it navigates to the current drive root (e.g.C:\) or the UNC share root (e.g.\\server\share).Why
Issue #3211 reports that the breadcrumb home icon navigates to the user's home directory (
/rootfor root) rather than the filesystem root. That home-directory behavior is intentional and consistent with other SFTP clients, but the reporter asked for either a configurable home target or a separate "back to root /" button, since switching between/and absolute paths is a frequent ops workflow. This implements the dedicated root button, which works for every pane without adding a settings surface.Changes
application/state/sftp/utils.ts— new pure helpergetSftpPathRoot(path, options)deriving the filesystem root (/, drive root, or UNC share root) from the current pane path; returnsnullwhen no root can be derived (e.g. relative Windows paths).components/sftp/SftpBreadcrumb.tsx— renders the disabled-aware root button in the pinned breadcrumb chrome next to Home, navigating via the existingonNavigatepath.application/i18n/locales/{en,es,ru,zh-CN,zh-TW}/…— newsftp.goRoottooltip string ("Go to root" / 回到根目录 / etc.).getSftpPathRootunit cases (POSIX, drive, UNC, forward-slash-UNC ambiguity) inapplication/state/sftp/utils.test.ts, and a jsdom interaction test incomponents/sftp/SftpBreadcrumb.test.tscovering click-to-navigate and the disabled-at-root state.Testing
node --test --import tsx components/sftp/SftpBreadcrumb.test.ts application/state/sftp/utils.test.ts components/sftp/SftpPaneToolbar.test.ts— 47 tests pass.npx eslinton the changed files — clean.npx tsc --noEmit -p tsconfig.json— no errors in touched files (repo has pre-existing unrelated errors elsewhere)./; home icon still goes to the home directory.Fixes #3211
Automation
@codex review(own/bot PRs only)