Repository navigation
feat(browser): new tab page with a workspace card and suggested sites - #31656
Conversation
…switch to tab A new tab's bar held the text "about:blank", so a click before typing merged the input into it and Enter silently failed.
…us when the list closes
…en tabs by exact URL Enter on a completion skipped the remote pane's file-URL check, and the history key dropped meaningful trailing slashes.
…tion the caret accepts
…switch-to-tab subscription
…mpletes to the same text
A row without an icon of its own, like a host completion, takes one from history on the same host.
…hosts A background tab's title update re-ranks history, which could move row 0 off the page the bar shows.
…s row Row 0 now follows the shown completion text, so evicted history can't move it either.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 32.98% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 53 files. (6 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: a browser new-tab page with workspace content and suggested sites. Description check The description is detailed and covers the required user impact, implementation, rationale, visual proof, testing, scope, and checklist. The Linked Issue section says "None," which is acceptable only … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is detailed and covers the required user impact, implementation, rationale, visual proof, testing, scope, and checklist. The Linked Issue section says "None," which is acceptable only under the template's maintainer exception.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 32.98% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 53 files. (6 skipped: 6 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
bb706684-6e1d-444c-a3d9-722dd03955a4
⛔ Files ignored due to path filters: 1
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing: 58
src/main/ipc/workspace-ports.tssrc/main/ports/workspace-port-page-probe.test.tssrc/main/ports/workspace-port-page-probe.tssrc/main/runtime/rpc/methods/workspace-ports.tssrc/preload/api/workspace-port-api.tssrc/preload/api/workspace-ports-bridge.tssrc/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/BrowserAddressBar.test.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/BrowserAddressBar.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/BrowserAddressBarSuggestionList.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-address-bar-expansion.tssrc/renderer/src/components/browser-pane/assemble-chrome/browser-address-bar-suggestions.test.tssrc/renderer/src/components/browser-pane/assemble-chrome/browser-address-bar-suggestions.tssrc/renderer/src/components/browser-pane/assemble-chrome/browser-navigation-control-row.test.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-toolbar.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-viewport-overlays.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-workspace-pane.adoption-chrome.test.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/use-browser-address-bar-edit-handoff.tssrc/renderer/src/components/browser-pane/assemble-chrome/use-browser-address-bar-inline-completion.tssrc/renderer/src/components/browser-pane/assemble-chrome/use-browser-address-bar-switch-to-tab.tssrc/renderer/src/components/browser-pane/browser-client-page-retained-elements.tssrc/renderer/src/components/browser-pane/browser-client-page-retained-registry.test.tssrc/renderer/src/components/browser-pane/browser-client-page-retained-registry.tssrc/renderer/src/components/browser-pane/browser-client-page-retained-state.tssrc/renderer/src/components/browser-pane/describe-page/browser-page-url-display.test.tssrc/renderer/src/components/browser-pane/describe-page/browser-page-url-display.tssrc/renderer/src/components/browser-pane/new-tab/BrowserNewTabPage.tsxsrc/renderer/src/components/browser-pane/new-tab/browser-new-tab-server-tile.tsxsrc/renderer/src/components/browser-pane/new-tab/browser-new-tab-tile.tsxsrc/renderer/src/components/browser-pane/new-tab/use-browser-new-tab-linked-items.tssrc/renderer/src/components/browser-pane/new-tab/use-browser-new-tab-servers.tssrc/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.address-bar.test.tsxsrc/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.tsxsrc/renderer/src/components/browser-pane/stream-remote/remote-browser-page-toolbar.tsxsrc/renderer/src/components/browser-pane/stream-remote/remote-browser-page-viewport.tsxsrc/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.toolbar.test.tsxsrc/renderer/src/components/browser-pane/workspace-doc/doc-preview-address-edit.tsxsrc/renderer/src/components/sidebar/WorktreeCardLinkedItemsSection.tsxsrc/renderer/src/components/sidebar/workspace-hover-attachments.tssrc/renderer/src/components/status-bar/PortsStatusSegment.tsxsrc/renderer/src/components/tab-bar/open-tab-search-entries.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/browser-history-match.test.tssrc/renderer/src/lib/browser-history-match.tssrc/renderer/src/lib/workspace-port-actions.tssrc/renderer/src/lib/workspace-port-scan-client.tssrc/renderer/src/lib/workspace-port-scan-refresh.tssrc/renderer/src/store/slices/browser.test.tssrc/renderer/src/store/slices/browser/browser-history-actions.tssrc/renderer/src/store/slices/browser/browser-slice-contract.tssrc/renderer/src/web/preload-api/web-workspace-ports-api.tssrc/shared/rpc-contract/workspace-ports-params.tssrc/shared/workspace-ports.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| const origin = `${parsed.protocol}//${parsed.host}` | ||
| const forms = withinHost | ||
| ? [historyHost(url), parsed.host, origin] | ||
| : [formatBrowserHistoryUrl(url), formatBrowserPaletteUrl(url), url] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the origin form only when the typed text has a scheme.
When the user types inside the host, forms always includes origin (for example https://github.com). The prefix check then matches scheme-less input against the scheme. As a result, typing h, ht, htt, http or https completes inline to the origin of the first matching history row.
Example: the user types h to reach hn.example. The bar fills in https://github.com and selects it. Enter then opens GitHub. This contradicts the comment at Line 194, which says the completion keeps what the user typed. Chrome does not complete scheme prefixes this way.
Keep the origin form only when schemeLength > 0. Add a test case like ['h', 'https://github.com/x'] to the does not complete table.
🐛 Proposed fix
--- "a/src/renderer/src/components/browser-pane/assemble-chrome/browser-address-bar-suggestions.ts"
+++ "b/src/renderer/src/components/browser-pane/assemble-chrome/browser-address-bar-suggestions.ts"
@@ -214,10 +214,12 @@
if (withinHost && parsed.host === '') {
return null
}
const origin = `${parsed.protocol}//${parsed.host}`
const forms = withinHost
- ? [historyHost(url), parsed.host, origin]
+ ? schemeLength > 0
+ ? [origin]
+ : [historyHost(url), parsed.host]
: [formatBrowserHistoryUrl(url), formatBrowserPaletteUrl(url), url]
// Why case only counts from the path on: hosts are case-insensitive, paths and queries aren't.
const prefixEnd = withinHost ? typed.length : pathStart
const form = forms.find(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const origin = `${parsed.protocol}//${parsed.host}` | |
| const forms = withinHost | |
| ? [historyHost(url), parsed.host, origin] | |
| : [formatBrowserHistoryUrl(url), formatBrowserPaletteUrl(url), url] | |
| const origin = `${parsed.protocol}//${parsed.host}` | |
| const forms = withinHost | |
| ? schemeLength > 0 | |
| ? [origin] | |
| : [historyHost(url), parsed.host] | |
| : [formatBrowserHistoryUrl(url), formatBrowserPaletteUrl(url), url] |
088e05a to
483878f
Compare
…ed sites A blank tab shows this workspace's running dev servers (only ports that serve an HTML page), its linked PR, issue and ticket, and the most visited sites, removable with Undo. Works in local, client-hosted and server-streamed panes.
483878f to
af8e8cf
Compare
AmoabaKelvin
changed the title
Stacked on #31604: review the commits after bdaf5a5.
ELI5
A new browser tab used to be empty. Now it opens on a card for the workspace you're in: the dev servers it's running, and its PR, issue and ticket with their full titles and state. Under the card, a collapsed line lists servers running in your other workspaces. Below that are tiles for the sites you visit most. Click anything to open it in this tab. Hover a site and click × to forget it, with Undo in the toast.
What Changed
GET /with a 2xxtext/htmlpage. A row shows the page's<title>and favicon overlocalhost:5173 · vite. APIs, databases andCannot GET /404s stay off.getWorkspaceLinkedItemRows, now shared by the sidebar's linked-items list and this page. A review row shows its full title,#number · Checks passing/failing/pending, and a state pill. Issue and ticket rows show their state.localhost. Folder workspaces show servers only.*.localhostpages are left out because live servers have their own rows. Removing a tile deletes that history entry, so it leaves the address bar too. Undo puts it back with its visit count. Tiles are one per page, as in Chrome, so removing one never reveals another page from the same site.workspacePorts:probePage(local IPC) andworkspacePorts.probePage(runtime RPC) run on the machine that owns the port.method_not_found, and the page then falls back to ports a terminal printed as a URL.load-commit. It sits above the pane as a click target, so the page under it would otherwise be covered.openWorkspacePortInBrowser, as the sidebar chip does, because the page runs on this client while the port is on the server.refreshWorkspacePortScanForTargetso the page can reuse it. Opening a new tab rescans so a server started seconds ago shows up.Why
The blank page was dead space in an app built around running servers and PRs. The design was picked from three HTML directions. The workspace card won over an all-tiles version because issues and PRs read as their full titles, so more fits at a glance. Suggested sites stay as tiles, and there is no separate Recent list.
Not in this PR:
openWorkspacePortInBrowser./tmp/xon macOS) gets no port attribution, because the process cwd resolves to/private/tmp/x. Same in the sidebar.Linked Issue
None.
Visual Proof
Same profile, history, linked issue and running server in both.
Other workspaces expanded:
Hover on a suggested tile (remove control and tooltip):
Dark:
Split pane:
Client-hosted pane on a paired server. The server row was probed on the host over RPC:
Remove and Undo, typing opens the dropdown, other workspaces expand, then a server row opens the dev server in place:
after-interactions.mp4
Testing
On macOS in hidden dev builds:
Not run live: a server on an older Orca without the probe. That path is the same
method_not_foundhandling the port scan already uses. Linux and Windows aren't affected, since the probe is Nodehttpagainst loopback.New tests:
AI Disclosure
Review
Agent skill upstream boundary
Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)