Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between e985d7e2cf94b420c7ff718f45e977a953f744e8 and 8910c5c. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an optional workspace setting, enabled by default, to control automatic terminal creation during workspace activation. The setting is propagated through folder, worktree, created-agent, and web-runtime wake flows. When disabled, empty workspace and tab-group views render actions for creating terminals, markdown notes, or browser tabs, with shortcut hints where available. Settings search, unit tests, activation tests, and localized UI strings are updated. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsx (1)
58-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew locale keys use semantic names instead of the file's hash-key convention. Both new settings entries key their translations as
...autoCreateTerminalOnWorkspaceActivation/...Descriptionrather than the 10-char hashes every sibling entry in these two files uses.
src/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsx#L58-L87: consider a hash-based key (e.g. via the bootstrap-locale-catalog flow) to match the surroundingtranslate('auto.components.settings.GeneralWorkspaceSettingsSection.<hash>', ...)entries in this file.src/renderer/src/components/settings/general-search.ts#L38-L66: same — surrounding entries in this catalog all use hash keys (e.g.4c95d08fa2,141f71c69f).Based on learnings, "use hash-based locale keys with
translate('auto.components.<Dir>.<ComponentFile>.<10-char-hash>', ...)" is the established convention for settings components, while a separate learning notes semantic keys are acceptable "when they match the established neighboring namespaces" — which isn't the case in these two files (neighbors are hash-keyed). Non-blocking; flagging for consistency only.Source: Learnings
src/renderer/src/components/tab-group/TabGroupPanel.tsx (1)
28-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
TabGroupEmptyStatere-runs the whole workspace model hook instead of reusing the parent'scommands.
TabGroupEmptyStateindependently callsuseTabGroupWorkspaceModel({ groupId, worktreeId })just for.commands, duplicating the same selector/memo workTabGroupPanelalready did with its ownmodelon line 145. This hook subscribes (viauseShallow) tostate.openFiles, a global slice, so the empty state re-renders on any file open/close app-wide, not just its own worktree —memo()doesn't prevent this since it's an internal hook-triggered re-render, not a prop change.Pass
commandsdown as a prop instead of calling the hook again.♻️ Proposed fix
const TabGroupEmptyState = memo(function TabGroupEmptyState({ worktreeId, - groupId + groupId, + commands }: { worktreeId: string groupId: string + commands: ReturnType<typeof useTabGroupWorkspaceModel>['commands'] }): React.JSX.Element { const newTerminalShortcut = useShortcutKeyDetails('tab.newTerminal') const newBrowserShortcut = useShortcutKeyDetails('tab.newBrowser') const newMarkdownShortcut = useShortcutKeyDetails('tab.newMarkdown') - const commands = useTabGroupWorkspaceModel({ groupId, worktreeId }).commandsAnd at the call site:
- {activeTab === null && - terminalTabs.length === 0 && - editorItems.length === 0 && - browserItems.length === 0 && - !autoCreateTerminalOnWorkspaceActivation && ( - <TabGroupEmptyState worktreeId={worktreeId} groupId={groupId} /> - )} + {activeTab === null && + terminalTabs.length === 0 && + editorItems.length === 0 && + browserItems.length === 0 && + !autoCreateTerminalOnWorkspaceActivation && ( + <TabGroupEmptyState worktreeId={worktreeId} groupId={groupId} commands={commands} /> + )}Also applies to: 145-146
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5eb3c922-952b-4a7c-83a3-fc6374bd31c2
📥 Commits
Reviewing files that changed from the base of the PR and between 6ad6241 and e985d7e2cf94b420c7ff718f45e977a953f744e8.
📒 Files selected for processing (20)
src/renderer/src/components/Terminal.tsxsrc/renderer/src/components/settings/GeneralPane.test.tssrc/renderer/src/components/settings/GeneralWorkspaceSettingsSection.test.tsxsrc/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsxsrc/renderer/src/components/settings/general-search.tssrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/components/terminal/initial-terminal.test.tssrc/renderer/src/components/terminal/initial-terminal.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/worktree-activation-created-agent.test.tssrc/renderer/src/lib/worktree-activation-empty-remote.test.tssrc/renderer/src/lib/worktree-activation-folder-workspace.test.tssrc/renderer/src/lib/worktree-activation.test.tssrc/renderer/src/lib/worktree-activation.tssrc/shared/constants.tssrc/shared/types.ts
e985d7e to
8910c5c
Compare
|
Thank you so much, @yiuf63, for the work you put into this PR. We referred to your empty-tab-group action design while comparing the community implementations with the empty-pane launch UI that has now landed in #32182. Giving people explicit terminal, document, and browser actions is valuable, and your implementation helped make that overlap clear. I have added a named acknowledgment and a link to this PR in #32182 so your contribution is visible. One important scope distinction: #32182 adds the empty-pane actions and host-aware agent launches; it does not add your preference to disable automatic terminal creation. That broader request is still tracked in #11108. Thank you again for contributing the implementation and tests, and for helping improve this workflow. |
Summary
Testing
ELI5
You can turn off automatic first-terminal creation when selecting an empty workspace. Empty workspaces show actions to open a terminal, note, or browser yourself instead of always spawning a shell.