Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions frontend/src/components/icons/XIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
interface XIconProps {
className?: string;
}

export default function XIcon({ className = 'w-4 h-4' }: XIconProps) {
return (
<svg className={className} fill="none" viewBox="0 0 24 24" stroke="currentColor">
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M6 18L18 6M6 6l12 12" />
</svg>
);
}
111 changes: 111 additions & 0 deletions frontend/src/components/tool/ResultsTabs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { useMemo } from 'react';
import { cn } from '../../lib/utils';
import { ResultsTable } from './ResultsTable';
import type { ResultTab } from './types';
import XIcon from '../icons/XIcon';

interface ResultsTabsProps {
tabs: ResultTab[];
activeTabId: string | null;
onTabSelect: (id: string) => void;
onTabClose: (id: string) => void;
isLoading?: boolean;
}

function formatTimestamp(date: Date): string {
return date.toLocaleTimeString('en-US', {
hour12: false,
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
});
}

export function ResultsTabs({
tabs,
activeTabId,
onTabSelect,
onTabClose,
isLoading,
}: ResultsTabsProps) {
const activeTab = useMemo(
() => tabs.find((tab) => tab.id === activeTabId),
[tabs, activeTabId]
);

// Loading state (no tabs yet)
if (isLoading && tabs.length === 0) {
return (
<div className="border border-border rounded-lg bg-card p-8 text-center">
<div className="text-muted-foreground">Running query...</div>
</div>
);
}

// Empty state
if (tabs.length === 0) {
return (
<div className="border border-border rounded-lg bg-card p-8 text-center">
<p className="text-muted-foreground text-sm">
Run a query to see results
</p>
</div>
);
}

return (
<div className="space-y-2">
{/* Tab bar */}
<div className="flex items-center gap-1 border-b border-border overflow-x-auto">
{tabs.map((tab) => (
<button
key={tab.id}
type="button"
onClick={() => onTabSelect(tab.id)}
className={cn(
'group flex items-center gap-1.5 px-3 py-1.5 text-sm whitespace-nowrap',
'border-b-2 -mb-px transition-colors cursor-pointer',
tab.id === activeTabId
? 'border-primary text-foreground'
: 'border-transparent text-muted-foreground hover:text-foreground'
)}
>
Comment on lines +61 to +72
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The tab buttons are missing keyboard navigation support for arrow keys. Users expect to be able to navigate between tabs using Left/Right arrow keys when focused on a tab. Consider implementing ARIA tabs pattern with proper keyboard navigation.

Copilot uses AI. Check for mistakes.
<span>{formatTimestamp(tab.timestamp)}</span>
{tab.error && (
<span className="w-1.5 h-1.5 rounded-full bg-destructive" aria-label="Error" />
)}
<span
role="button"
tabIndex={0}
aria-label="Close tab"
onClick={(e) => {
e.stopPropagation();
onTabClose(tab.id);
}}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.stopPropagation();
onTabClose(tab.id);
}
}}
className="opacity-0 group-hover:opacity-100 hover:bg-muted rounded p-0.5 transition-opacity"
>
<XIcon className="w-3 h-3" />
</span>
</button>
))}
</div>

{/* Active tab content */}
{activeTab && (
<ResultsTable
result={activeTab.result}
error={activeTab.error}
isLoading={isLoading}
executedSql={activeTab.executedSql}
executionTimeMs={activeTab.executionTimeMs}
/>
)}
</div>
);
}
2 changes: 2 additions & 0 deletions frontend/src/components/tool/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ export { SqlEditor } from './SqlEditor';
export { ParameterForm } from './ParameterForm';
export { RunButton } from './RunButton';
export { ResultsTable } from './ResultsTable';
export { ResultsTabs } from './ResultsTabs';
export type { ResultTab } from './types';
10 changes: 10 additions & 0 deletions frontend/src/components/tool/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { QueryResult } from '../../api/tools';

export interface ResultTab {
id: string;
timestamp: Date;
result: QueryResult | null;
error: string | null;
executedSql: string;
executionTimeMs: number;
}
67 changes: 46 additions & 21 deletions frontend/src/components/views/ToolDetailView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { fetchSource } from '../../api/sources';
import { executeTool, type QueryResult } from '../../api/tools';
import { ApiError } from '../../api/errors';
import type { Tool } from '../../types/datasource';
import { SqlEditor, ParameterForm, RunButton, ResultsTable } from '../tool';
import { SqlEditor, ParameterForm, RunButton, ResultsTabs, type ResultTab } from '../tool';
import LockIcon from '../icons/LockIcon';
import CopyIcon from '../icons/CopyIcon';
import CheckIcon from '../icons/CheckIcon';
Expand All @@ -22,11 +22,9 @@ export default function ToolDetailView() {
return searchParams.get('sql') || '';
});
const [params, setParams] = useState<Record<string, any>>({});
const [result, setResult] = useState<QueryResult | null>(null);
const [queryError, setQueryError] = useState<string | null>(null);
const [resultTabs, setResultTabs] = useState<ResultTab[]>([]);
const [activeTabId, setActiveTabId] = useState<string | null>(null);
const [isRunning, setIsRunning] = useState(false);
const [executionTimeMs, setExecutionTimeMs] = useState<number | null>(null);
const [executedSql, setExecutedSql] = useState<string>('');
const [copied, setCopied] = useState(false);

useEffect(() => {
Expand All @@ -35,10 +33,8 @@ export default function ToolDetailView() {
setIsLoading(true);
setError(null);
// Reset result state when switching tools
setResult(null);
setQueryError(null);
setExecutionTimeMs(null);
setExecutedSql('');
setResultTabs([]);
setActiveTabId(null);

fetchSource(sourceId)
.then((sourceData) => {
Expand Down Expand Up @@ -206,9 +202,6 @@ export default function ToolDetailView() {
if (!tool || !toolName) return;

setIsRunning(true);
setQueryError(null);
setResult(null);
setExecutionTimeMs(null);

const startTime = performance.now();

Expand All @@ -227,11 +220,27 @@ export default function ToolDetailView() {
const endTime = performance.now();
const duration = endTime - startTime;

setResult(queryResult);
setExecutionTimeMs(duration);
setExecutedSql(sqlToExecute);
const newTab: ResultTab = {
id: crypto.randomUUID(),
timestamp: new Date(),
result: queryResult,
error: null,
executedSql: sqlToExecute,
executionTimeMs: duration,
};
setResultTabs(prev => [newTab, ...prev]);
setActiveTabId(newTab.id);
Comment on lines +231 to +232
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

There's potential for memory leaks as the tabs array grows unbounded. Each query execution adds a new tab to the beginning of the array, but there's no limit on the total number of tabs. Consider implementing a maximum tab limit or providing a "clear all" functionality to prevent memory issues during extended usage sessions.

Copilot uses AI. Check for mistakes.
} catch (err) {
setQueryError(err instanceof Error ? err.message : 'Query failed');
const errorTab: ResultTab = {
id: crypto.randomUUID(),
timestamp: new Date(),
result: null,
error: err instanceof Error ? err.message : 'Query failed',
executedSql: toolType === 'execute_sql' ? sql : getSqlPreview(),
executionTimeMs: 0,
};
setResultTabs(prev => [errorTab, ...prev]);
setActiveTabId(errorTab.id);
} finally {
setIsRunning(false);
}
Expand All @@ -249,6 +258,22 @@ export default function ToolDetailView() {
setTimeout(() => setCopied(false), 2000);
}, [toolType, sql, getSqlPreview]);

const handleTabClose = useCallback((idToClose: string) => {
setResultTabs(prev => {
const index = prev.findIndex(tab => tab.id === idToClose);
const newTabs = prev.filter(tab => tab.id !== idToClose);

if (idToClose === activeTabId && newTabs.length > 0) {
const nextIndex = Math.min(index, newTabs.length - 1);
Comment on lines +264 to +267
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

When the tab being closed is the active tab and there are remaining tabs, the next tab selection logic could be improved. Currently it uses Math.min(index, newTabs.length - 1), but this doesn't account for the case where the closed tab was at the end. Consider selecting the previous tab (index - 1) when closing the last tab, or the next tab (same index) when closing any other tab, for a more intuitive user experience.

Suggested change
const newTabs = prev.filter(tab => tab.id !== idToClose);
if (idToClose === activeTabId && newTabs.length > 0) {
const nextIndex = Math.min(index, newTabs.length - 1);
// If the tab to close is not found, return the previous state unchanged
if (index === -1) {
return prev;
}
const newTabs = prev.filter(tab => tab.id !== idToClose);
if (idToClose === activeTabId && newTabs.length > 0) {
// When closing the active tab:
// - if it was the last tab, select the previous tab
// - otherwise, select the tab that shifted into its position (the "next" tab)
const nextIndex = index >= newTabs.length ? newTabs.length - 1 : index;

Copilot uses AI. Check for mistakes.
setActiveTabId(newTabs[nextIndex].id);
} else if (newTabs.length === 0) {
setActiveTabId(null);
}

return newTabs;
});
}, [activeTabId]);

if (!sourceId || !toolName) {
return <Navigate to="/" replace />;
}
Expand Down Expand Up @@ -382,12 +407,12 @@ export default function ToolDetailView() {
/>

{/* Results */}
<ResultsTable
result={result}
error={queryError}
<ResultsTabs
tabs={resultTabs}
activeTabId={activeTabId}
onTabSelect={setActiveTabId}
onTabClose={handleTabClose}
isLoading={isRunning}
executedSql={executedSql}
executionTimeMs={executionTimeMs ?? undefined}
/>
</div>
</div>
Expand Down