Skip to content

Style the no terminals open screen a bit #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 3 commits into from
Nov 28, 2025
Merged

Style the no terminals open screen a bit #174

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8ee7e3
WIP
saddlepaddle Nov 28, 2025
34fcf47
WI
saddlepaddle Nov 28, 2025
e082b5b
WIP - almost nice
saddlepaddle Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions ...c/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import { Kbd, KbdGroup } from "@superset/ui/kbd";
import { HiMiniCommandLine } from "react-icons/hi2";
import { formatKeysForDisplay, HOTKEYS } from "shared/hotkeys";

const shortcuts = [HOTKEYS.NEW_TERMINAL, HOTKEYS.SPLIT_HORIZONTAL];

export function EmptyTabView() {
return (
<div className="flex-1 h-full overflow-auto">
<div className="h-full w-full p-6">
<div className="flex items-center justify-center h-full">
<div className="text-center">
<h2 className="text-2xl font-semibold text-foreground mb-2">
No Active Tab
</h2>
<p className="text-muted-foreground">
Create a new tab to get started
</p>
<div className="flex-1 h-full flex flex-col items-center justify-center gap-6">
<div className="p-4 rounded-lg bg-muted border border-border">
<HiMiniCommandLine className="size-8 text-muted-foreground" />
</div>

<p className="text-sm text-muted-foreground">No terminal open</p>

<div className="flex items-center gap-4 text-xs text-muted-foreground">
{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>
))}
Comment on lines +17 to +26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor key uniqueness concern and accessibility opportunity.

The keyboard shortcut rendering logic works correctly for the current shortcuts, but has two areas for improvement:

  1. 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.

  2. 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.

Suggested change
{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).

</div>
</div>
);
Expand Down