Skip to content
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.

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

feat: a bunch of smaller enhancements #30

Merged
merged 1 commit into from
Mar 24, 2025
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
32 changes: 31 additions & 1 deletion src/components/ChatInput.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Send, Loader2, Settings } from 'lucide-react';
import { Button } from '@/components/ui/button';
import { Textarea } from '@/components/ui/textarea';
import { useState, type FC, type FormEvent, type KeyboardEvent } from 'react';
import { useState, useEffect, useRef, type FC, type FormEvent, type KeyboardEvent } from 'react';
import { useApi } from '@/contexts/ApiContext';
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover';
import {
Expand All @@ -28,6 +28,7 @@ interface Props {
isGenerating$: Observable<boolean>;
defaultModel?: string;
availableModels?: string[];
autoFocus?: boolean;
}

export const ChatInput: FC<Props> = ({
Expand All @@ -37,11 +38,35 @@ export const ChatInput: FC<Props> = ({
isGenerating$,
defaultModel = '',
availableModels = [],
autoFocus = false,
}) => {
const [message, setMessage] = useState('');
const [streamingEnabled, setStreamingEnabled] = useState(true);
const [selectedModel, setSelectedModel] = useState(defaultModel || '');
const api = useApi();
const textareaRef = useRef<HTMLTextAreaElement>(null);

// Focus the textarea when autoFocus is true and component is interactive
useEffect(() => {
if (autoFocus && textareaRef.current && !isReadOnly && api.isConnected) {
textareaRef.current.focus();
}
}, [autoFocus, isReadOnly, api.isConnected]);

// Global keyboard shortcut for interrupting generation with Escape key
useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider if duplicate Escape handling (global listener vs. Textarea keydown) might trigger onInterrupt twice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe only have the global listener, so that user can escape once to unfocus textarea and once again to interrupt?

Not sure how these handlers work together right now.

const handleGlobalKeyDown = (e: globalThis.KeyboardEvent) => {
if (e.key === 'Escape' && isGenerating$.get() && onInterrupt) {
console.log('[ChatInput] Global Escape pressed, interrupting generation...');
onInterrupt();
}
};

document.addEventListener('keydown', handleGlobalKeyDown);
return () => {
document.removeEventListener('keydown', handleGlobalKeyDown);
};
}, [isGenerating$, onInterrupt]);

const handleSubmit = async (e: FormEvent) => {
e.preventDefault();
Expand All @@ -68,6 +93,10 @@ export const ChatInput: FC<Props> = ({
if (e.key === 'Enter' && !e.shiftKey) {
e.preventDefault();
handleSubmit(e);
} else if (e.key === 'Escape' && isGenerating$.get() && onInterrupt) {
e.preventDefault();
console.log('[ChatInput] Escape pressed, interrupting generation...');
onInterrupt();
}
};

Expand All @@ -84,6 +113,7 @@ export const ChatInput: FC<Props> = ({
<div className="flex flex-1">
<div className="relative flex flex-1">
<Textarea
ref={textareaRef}
value={message}
onChange={(e) => {
setMessage(e.target.value);
Expand Down
80 changes: 80 additions & 0 deletions src/components/CodeDisplay.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { useState, useEffect } from 'react';
import { Button } from '@/components/ui/button';
import { Clipboard, Check } from 'lucide-react';
import { highlightCode } from '@/utils/highlightUtils';
// We still need the CSS though
import 'highlight.js/styles/github-dark.css';

interface CodeDisplayProps {
code: string;
maxHeight?: string;
showLineNumbers?: boolean;
language?: string;
}

export function CodeDisplay({
code,
maxHeight = '300px',
showLineNumbers = true,
language,
}: CodeDisplayProps) {
const [copied, setCopied] = useState(false);
const [highlightedCode, setHighlightedCode] = useState('');

useEffect(() => {
if (!code) return;

// Use our shared utility
setHighlightedCode(highlightCode(code, language, true, 1000));
}, [code, language]);

if (!code) return null;

const handleCopy = () => {
navigator.clipboard.writeText(code);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding error handling for navigator.clipboard.writeText in handleCopy to gracefully handle clipboard API failures.

Suggested change
navigator.clipboard.writeText(code);
try { navigator.clipboard.writeText(code); } catch (err) { console.error('Failed to copy text:', err); }

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this is necessary.

setCopied(true);
setTimeout(() => setCopied(false), 2000);
};

const lines = code.split('\n');

return (
<div className="relative overflow-hidden rounded bg-muted">
<div className="absolute right-1 z-10">
<Button
variant="ghost"
size="sm"
onClick={handleCopy}
className="h-10 w-10 p-1 opacity-50 hover:opacity-100"
>
{copied ? <Check size={16} /> : <Clipboard className="drop-shadow-md" size={16} />}
</Button>
</div>

{/* Single scrollable container for both line numbers and code */}
<div className="overflow-auto" style={{ maxHeight }}>
<div className="flex">
{showLineNumbers && lines.length > 1 && (
<div className="min-w-12 flex-none select-none border-r border-muted-foreground/20 bg-muted/50 py-1 pr-1 text-right font-mono text-muted-foreground">
{lines.map((_, i) => (
<div key={i} className="px-2 text-xs leading-6">
{i + 1}
</div>
))}
</div>
)}

<div className="flex-1">
{highlightedCode ? (
<pre className="whitespace-pre px-4 py-1 leading-6">
<code dangerouslySetInnerHTML={{ __html: highlightedCode }} />
</pre>
) : (
<pre className="whitespace-pre px-4 py-1 leading-6">{code}</pre>
)}
</div>
</div>
</div>
</div>
);
}
30 changes: 29 additions & 1 deletion src/components/ConversationContent.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { FC } from 'react';
import { useRef } from 'react';
import { useRef, useState, useEffect } from 'react';
import { ChatMessage } from './ChatMessage';
import { ChatInput, type ChatOptions } from './ChatInput';
import { useConversation } from '@/hooks/useConversation';
Expand Down Expand Up @@ -38,6 +38,33 @@ export const ConversationContent: FC<Props> = ({ conversation }) => {
interruptGeneration,
} = useConversation(conversation);

// State to track when to auto-focus the input
const [shouldFocus, setShouldFocus] = useState(false);
// Store the previous conversation name to detect changes
const prevConversationNameRef = useRef<string | null>(null);

// Detect when the conversation changes and set focus
useEffect(() => {
if (conversation.name !== prevConversationNameRef.current) {
// New conversation detected - set focus flag
setShouldFocus(true);

// Store the current conversation name for future comparisons
prevConversationNameRef.current = conversation.name;
}
}, [conversation.name]);

// Reset focus flag after it's been used
useEffect(() => {
if (shouldFocus) {
const timer = setTimeout(() => {
setShouldFocus(false);
}, 100);

return () => clearTimeout(timer);
}
}, [shouldFocus]);

const showInitialSystem$ = useObservable<boolean>(false);

const firstNonSystemIndex$ = useObservable(() => {
Expand Down Expand Up @@ -174,6 +201,7 @@ export const ConversationContent: FC<Props> = ({ conversation }) => {
isGenerating$={isGenerating$}
availableModels={AVAILABLE_MODELS}
defaultModel={AVAILABLE_MODELS[0]}
autoFocus={shouldFocus}
/>
</main>
);
Expand Down
49 changes: 36 additions & 13 deletions src/components/ToolConfirmationDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import type { PendingTool } from '@/hooks/useConversation';
import { Loader2 } from 'lucide-react';
import { type Observable } from '@legendapp/state';
import { use$ } from '@legendapp/state/react';
import { CodeDisplay } from '@/components/CodeDisplay';
import { detectToolLanguage } from '@/utils/highlightUtils';

interface ToolConfirmationDialogProps {
pendingTool$: Observable<PendingTool | null>;
onConfirm: () => Promise<void>;
Expand Down Expand Up @@ -88,6 +91,7 @@ export function ToolConfirmationDialog({
// Format args for display
const formatArgs = (args: string[]) => {
if (!args || args.length === 0) return 'No arguments';
if (args.length === 1) return args[0];
return args.map((arg, i) => `${i + 1}. ${arg}`).join('\n');
};

Expand All @@ -104,24 +108,34 @@ export function ToolConfirmationDialog({
</DialogHeader>

<div className="grid gap-4 py-4">
<div className="grid grid-cols-4 items-center gap-4">
<div className="grid grid-cols-6 items-center gap-4">
<div className="font-medium">Tool:</div>
<div className="col-span-3 rounded bg-muted p-2 font-mono">
<div className="col-span-5 rounded bg-muted p-2 font-mono">
{pendingTool.tooluse.tool}
</div>
</div>

<div className="grid grid-cols-4 items-start gap-4">
<div className="font-medium">Arguments:</div>
<div className="col-span-3 whitespace-pre-wrap rounded bg-muted p-2 font-mono">
{formatArgs(pendingTool.tooluse.args)}
</div>
</div>
{
/* Arguments */
pendingTool.tooluse.args.length > 0 && (
<div className="grid grid-cols-6 items-start gap-4">
<div className="font-medium">Arguments:</div>
<div className="col-span-5">
<CodeDisplay
code={formatArgs(pendingTool.tooluse.args)}
maxHeight="150px"
showLineNumbers={false}
// Arguments are usually formatted plain text, so no language needed
/>
</div>
</div>
)
}

{isEditing ? (
<div className="grid grid-cols-4 items-start gap-4">
<div className="grid grid-cols-6 items-start gap-4">
<div className="font-medium">Edit Code:</div>
<div className="col-span-3">
<div className="col-span-5">
<Textarea
value={editedContent}
onChange={(e) => setEditedContent(e.target.value)}
Expand All @@ -131,10 +145,19 @@ export function ToolConfirmationDialog({
</div>
</div>
) : (
<div className="grid grid-cols-4 items-start gap-4">
<div className="grid grid-cols-6 items-start gap-4">
<div className="font-medium">Code:</div>
<div className="col-span-3 whitespace-pre-wrap rounded bg-muted p-2 font-mono">
{pendingTool.tooluse.content}
<div className="col-span-5">
<CodeDisplay
code={pendingTool.tooluse.content}
maxHeight="300px"
showLineNumbers={true}
language={detectToolLanguage(
pendingTool.tooluse.tool,
pendingTool.tooluse.args,
pendingTool.tooluse.content
)}
/>
</div>
</div>
)}
Expand Down
33 changes: 29 additions & 4 deletions src/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,36 @@
}

.chat-message details summary {
@apply cursor-pointer rounded-md bg-muted px-3 py-2 text-xs transition-colors hover:ring hover:ring-inset hover:ring-ring;
@apply cursor-pointer rounded-md bg-muted px-3 py-2 text-xs transition-colors;
user-select: none;
}

.chat-message details:not([open]) summary {
@apply hover:ring hover:ring-inset hover:ring-ring;
}

.chat-message details[open] {
@apply hover:bg-secondary hover:text-secondary-foreground;
}

.chat-message details[type='thinking'][open] summary {
@apply bg-transparent;
}

.chat-message details[type='thinking'][open] {
@apply text-xs;
box-shadow: 0 0 10px 4px hsl(var(--border)) inset;
/*
inset box shadow
*/
}

.chat-message details[type='thinking'] > *:not(summary) {
@apply italic text-gray-400;
}

.chat-message details[open] {
@apply rounded-lg border border-border;
@apply rounded-lg border border-border pb-2;
}

.chat-message details[open] > summary {
Expand All @@ -160,11 +185,11 @@ details > details {
}

.chat-message details[open] > *:not(summary):not(pre):not(details) {
@apply px-4 py-2;
@apply px-3 py-1;
}

.chat-message pre {
@apply block p-3;
@apply block;
}

.chat-message ul {
Expand Down
12 changes: 7 additions & 5 deletions src/utils/__tests__/markdownUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ describe('transformThinkingTags', () => {
it('should transform thinking tags to details/summary', () => {
const input = 'Before <thinking>Some thoughts</thinking> After';
const expected =
'Before <details><summary>💭 Thinking</summary>\n\nSome thoughts\n\n</details> After';
'Before <details type="thinking"><summary>💭 Thinking</summary>\n\nSome thoughts\n\n</details> After';
expect(transformThinkingTags(input)).toBe(expected);
});

it('should handle multiple thinking tags', () => {
const input = '<thinking>First thought</thinking> Middle <thinking>Second thought</thinking>';
const expected =
'<details><summary>💭 Thinking</summary>\n\nFirst thought\n\n</details> Middle <details><summary>💭 Thinking</summary>\n\nSecond thought\n\n</details>';
'<details type="thinking"><summary>💭 Thinking</summary>\n\nFirst thought\n\n</details> Middle <details type="thinking"><summary>💭 Thinking</summary>\n\nSecond thought\n\n</details>';
expect(transformThinkingTags(input)).toBe(expected);
});

Expand All @@ -95,7 +95,7 @@ describe('transformThinkingTags', () => {
it('preserves content outside thinking tags', () => {
const input = 'Before <thinking>thinking</thinking> after';
const expected =
'Before <details><summary>💭 Thinking</summary>\n\nthinking\n\n</details> after';
'Before <details type="thinking"><summary>💭 Thinking</summary>\n\nthinking\n\n</details> after';
expect(transformThinkingTags(input)).toBe(expected);
});
});
Expand Down Expand Up @@ -227,7 +227,9 @@ You can try the web UI by:
'<code class="hljs language-shell"><span class="hljs-comment"># This shows as a tool</span>'
);
expect(result).toContain('# This shows as command output');
expect(result).toContain('drwxr-xr-x 2 user user');

// Check for the directory listing - with HTML tags stripped
expect(result.replace(/<[^>]*>/g, '')).toContain('drwxr-xr-x 2 user user');

// Check final content is included
expect(result).toContain('You can try the web UI by:');
Expand All @@ -236,7 +238,7 @@ You can try the web UI by:
expect(result).toContain('http://localhost:5173');

// Check thinking block
expect(result).toContain('<details><summary>💭 Thinking</summary>');
expect(result).toContain('<details type="thinking"><summary>💭 Thinking</summary>');
expect(result).toContain('Thinking blocks are collapsible');
});
});
Loading