Style the no terminals open screen a bit#174
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe EmptyTabView component's UI has been restructured from a simple centered heading to a comprehensive empty state featuring an icon card, descriptive message, and interactive keyboard shortcut hints displaying relevant hotkeys for terminal and split operations. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
1-5: LGTM! Clean imports and setup.The imports follow the codebase guidelines by using the
shared/alias, and the shortcuts constant correctly references valid hotkey definitions.Optionally, you could add an explicit type annotation for better type safety and documentation:
+import type { HotkeyDefinition } from "shared/hotkeys"; + -const shortcuts = [HOTKEYS.NEW_TERMINAL, HOTKEYS.SPLIT_HORIZONTAL]; +const shortcuts: HotkeyDefinition[] = [HOTKEYS.NEW_TERMINAL, HOTKEYS.SPLIT_HORIZONTAL];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (2)
apps/desktop/src/shared/hotkeys.ts (2)
HOTKEYS(28-140)formatKeysForDisplay(169-192)packages/ui/src/components/kbd.tsx (2)
KbdGroup(28-28)Kbd(28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
9-14: LGTM! Well-structured empty state.The container layout and icon card styling follow good design patterns with proper use of Tailwind utilities and design tokens.
| {shortcuts.map((shortcut) => ( | ||
| <div key={shortcut.label} className="flex items-center gap-2"> | ||
| <KbdGroup> | ||
| {formatKeysForDisplay(shortcut.keys).map((key) => ( | ||
| <Kbd key={key}>{key}</Kbd> | ||
| ))} | ||
| </KbdGroup> | ||
| <span>{shortcut.label}</span> | ||
| </div> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Minor key uniqueness concern and accessibility opportunity.
The keyboard shortcut rendering logic works correctly for the current shortcuts, but has two areas for improvement:
-
React key uniqueness (Line 21): The inner map uses the formatted key string as the React key. If a hotkey ever contains duplicate modifiers (e.g.,
"shift+shift+t"), this would create duplicate keys and trigger React warnings. -
Accessibility: The keyboard shortcuts section lacks semantic markup. Screen reader users won't understand that these are actionable keyboard shortcuts.
Consider these improvements:
1. Ensure unique keys by including an index:
- {formatKeysForDisplay(shortcut.keys).map((key) => (
- <Kbd key={key}>{key}</Kbd>
+ {formatKeysForDisplay(shortcut.keys).map((key, index) => (
+ <Kbd key={`${key}-${index}`}>{key}</Kbd>
))}2. Add accessibility context:
- <div className="flex items-center gap-4 text-xs text-muted-foreground">
+ <div
+ className="flex items-center gap-4 text-xs text-muted-foreground"
+ role="list"
+ aria-label="Available keyboard shortcuts"
+ >
{shortcuts.map((shortcut) => (
- <div key={shortcut.label} className="flex items-center gap-2">
+ <div key={shortcut.label} className="flex items-center gap-2" role="listitem">📝 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.
| {shortcuts.map((shortcut) => ( | |
| <div key={shortcut.label} className="flex items-center gap-2"> | |
| <KbdGroup> | |
| {formatKeysForDisplay(shortcut.keys).map((key) => ( | |
| <Kbd key={key}>{key}</Kbd> | |
| ))} | |
| </KbdGroup> | |
| <span>{shortcut.label}</span> | |
| </div> | |
| </div> | |
| ))} | |
| {shortcuts.map((shortcut) => ( | |
| <div key={shortcut.label} className="flex items-center gap-2"> | |
| <KbdGroup> | |
| {formatKeysForDisplay(shortcut.keys).map((key, index) => ( | |
| <Kbd key={`${key}-${index}`}>{key}</Kbd> | |
| ))} | |
| </KbdGroup> | |
| <span>{shortcut.label}</span> | |
| </div> | |
| ))} |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
around lines 17-26, the inner map currently uses the formatted key string as the
React key which can collide for duplicated modifiers; change the key to combine
the formatted key and the map index (or another stable unique index) to
guarantee uniqueness. Also make the shortcuts semantic and accessible by
rendering them as a list (e.g., <ul> or an element with role="list" and an
appropriate aria-label) and each shortcut as a list item (e.g., <li>) and ensure
the visual keyboard UI has aria-hidden where needed and the action/label is
available to screen readers (e.g., readable text or aria-label on the list
item).
* WIP * WI p * WIP - almost nice
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.