mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-01 21:20:44 +00:00
fix(webui): improve robustness and accessibility based on code review
- Fix groupContent error detection: only treat as error when contentObj.error is truthy - Fix safeTitle: add try/catch for circular reference handling - Update CopyButton to use PlatformContext with navigator.clipboard fallback - Improve FileLink accessibility: use button element with proper keyboard support - Add aria-disabled and disabled attributes when file opening is unavailable
This commit is contained in:
parent
ef48ebc118
commit
e81cdbbcb1
3 changed files with 96 additions and 33 deletions
|
|
@ -69,6 +69,7 @@ function buildFullPath(
|
|||
* - Support line and column number navigation
|
||||
* - Hover to show full path
|
||||
* - Optional display mode (full path vs filename only)
|
||||
* - Full keyboard accessibility (Enter and Space keys)
|
||||
*
|
||||
* @example
|
||||
* ```tsx
|
||||
|
|
@ -86,22 +87,18 @@ export const FileLink: React.FC<FileLinkProps> = ({
|
|||
}) => {
|
||||
const platform = usePlatform();
|
||||
|
||||
/**
|
||||
* Handle click event - Open file using platform-specific method
|
||||
*/
|
||||
const handleClick = (e: React.MouseEvent) => {
|
||||
// Always prevent default behavior (prevent <a> tag # navigation)
|
||||
e.preventDefault();
|
||||
// Check if file opening is available
|
||||
const canOpenFile = platform.features?.canOpenFile !== false;
|
||||
const isDisabled = disableClick || !canOpenFile;
|
||||
|
||||
if (disableClick) {
|
||||
// If click is disabled, return directly without stopping propagation
|
||||
// This allows parent elements to handle click events
|
||||
/**
|
||||
* Open file using platform-specific method
|
||||
*/
|
||||
const openFile = () => {
|
||||
if (isDisabled) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If click is enabled, stop event propagation
|
||||
e.stopPropagation();
|
||||
|
||||
// Build full path including line and column numbers
|
||||
const fullPath = buildFullPath(path, line, column);
|
||||
|
||||
|
|
@ -116,6 +113,32 @@ export const FileLink: React.FC<FileLinkProps> = ({
|
|||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Handle click event
|
||||
*/
|
||||
const handleClick = (e: React.MouseEvent) => {
|
||||
e.preventDefault();
|
||||
if (!isDisabled) {
|
||||
e.stopPropagation();
|
||||
openFile();
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Handle keyboard event - Support Space key for button behavior
|
||||
*/
|
||||
const handleKeyDown = (e: React.KeyboardEvent) => {
|
||||
if (isDisabled) {
|
||||
return;
|
||||
}
|
||||
// Space key triggers button action (Enter is handled by default for buttons)
|
||||
if (e.key === ' ' || e.key === 'Enter') {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
openFile();
|
||||
}
|
||||
};
|
||||
|
||||
// Build display text
|
||||
const displayPath = showFullPath ? path : getFileName(path);
|
||||
|
||||
|
|
@ -123,31 +146,36 @@ export const FileLink: React.FC<FileLinkProps> = ({
|
|||
const fullDisplayText = buildFullPath(path, line, column);
|
||||
|
||||
return (
|
||||
<a
|
||||
href="#"
|
||||
<button
|
||||
type="button"
|
||||
className={[
|
||||
'file-link',
|
||||
// Reset button styles
|
||||
'bg-transparent border-none p-0 m-0 font-inherit',
|
||||
// Layout + interaction
|
||||
// Use items-center + leading-none to vertically center within surrounding rows
|
||||
'inline-flex items-center leading-none',
|
||||
disableClick
|
||||
? 'pointer-events-none cursor-[inherit] hover:no-underline'
|
||||
: 'cursor-pointer',
|
||||
isDisabled
|
||||
? 'cursor-default opacity-60'
|
||||
: 'cursor-pointer hover:underline',
|
||||
// Typography + color: match theme body text and fixed size
|
||||
'text-[11px] no-underline hover:underline',
|
||||
'text-[11px] no-underline',
|
||||
'text-[var(--app-primary-foreground)]',
|
||||
// Transitions
|
||||
'transition-colors duration-100 ease-in-out',
|
||||
// Focus ring (keyboard nav)
|
||||
'focus:outline focus:outline-1 focus:outline-[var(--vscode-focusBorder)] focus:outline-offset-2 focus:rounded-[2px]',
|
||||
// Active state
|
||||
'active:opacity-80',
|
||||
!isDisabled && 'active:opacity-80',
|
||||
className,
|
||||
].join(' ')}
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(' ')}
|
||||
onClick={handleClick}
|
||||
onKeyDown={handleKeyDown}
|
||||
title={fullDisplayText}
|
||||
role="button"
|
||||
aria-label={`Open file: ${fullDisplayText}`}
|
||||
aria-disabled={isDisabled}
|
||||
disabled={isDisabled}
|
||||
>
|
||||
<span className="file-link-path">{displayPath}</span>
|
||||
{line !== null && line !== undefined && (
|
||||
|
|
@ -156,6 +184,6 @@ export const FileLink: React.FC<FileLinkProps> = ({
|
|||
{column !== null && column !== undefined && <>:{column}</>}
|
||||
</span>
|
||||
)}
|
||||
</a>
|
||||
</button>
|
||||
);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -7,20 +7,28 @@
|
|||
*/
|
||||
|
||||
import type React from 'react';
|
||||
import { useState } from 'react';
|
||||
import { useState, useCallback } from 'react';
|
||||
import { usePlatform } from '../../../context/PlatformContext.js';
|
||||
|
||||
/**
|
||||
* Handle copy to clipboard
|
||||
* Handle copy to clipboard using platform-specific API with fallback
|
||||
* @param text Text to copy
|
||||
* @param event Mouse event to stop propagation
|
||||
* @param platformCopy Optional platform-specific copy function
|
||||
*/
|
||||
export const handleCopyToClipboard = async (
|
||||
text: string,
|
||||
event: React.MouseEvent,
|
||||
platformCopy?: (text: string) => Promise<void>,
|
||||
): Promise<void> => {
|
||||
event.stopPropagation(); // Prevent triggering the row click
|
||||
try {
|
||||
await navigator.clipboard.writeText(text);
|
||||
// Use platform-specific copy if available, otherwise fall back to navigator.clipboard
|
||||
if (platformCopy) {
|
||||
await platformCopy(text);
|
||||
} else {
|
||||
await navigator.clipboard.writeText(text);
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('Failed to copy text:', err);
|
||||
}
|
||||
|
|
@ -35,21 +43,36 @@ interface CopyButtonProps {
|
|||
|
||||
/**
|
||||
* CopyButton - Shared copy button component with Tailwind styles
|
||||
* Uses PlatformContext for platform-specific clipboard access with fallback
|
||||
* Note: Parent element should have 'group' class for hover effect
|
||||
*/
|
||||
export const CopyButton: React.FC<CopyButtonProps> = ({ text }) => {
|
||||
const [showTooltip, setShowTooltip] = useState(false);
|
||||
const platform = usePlatform();
|
||||
|
||||
const handleClick = useCallback(
|
||||
async (e: React.MouseEvent) => {
|
||||
await handleCopyToClipboard(text, e, platform.copyToClipboard);
|
||||
setShowTooltip(true);
|
||||
setTimeout(() => setShowTooltip(false), 1000);
|
||||
},
|
||||
[text, platform.copyToClipboard],
|
||||
);
|
||||
|
||||
// Check if copy feature is available
|
||||
const canCopy = platform.features?.canCopy !== false;
|
||||
|
||||
if (!canCopy) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return (
|
||||
<button
|
||||
className="col-start-3 bg-transparent border-none px-2 py-1.5 cursor-pointer text-[var(--app-secondary-foreground)] opacity-0 transition-opacity duration-200 ease-out flex items-center justify-center rounded relative group-hover:opacity-70 hover:!opacity-100 hover:bg-[var(--app-input-border)] active:scale-95"
|
||||
onClick={async (e) => {
|
||||
await handleCopyToClipboard(text, e);
|
||||
setShowTooltip(true);
|
||||
setTimeout(() => setShowTooltip(false), 1000);
|
||||
}}
|
||||
onClick={handleClick}
|
||||
title="Copy"
|
||||
aria-label="Copy to clipboard"
|
||||
type="button"
|
||||
>
|
||||
<svg
|
||||
width="14"
|
||||
|
|
|
|||
|
|
@ -128,13 +128,19 @@ export const formatValue = (value: unknown): string => {
|
|||
/**
|
||||
* Safely convert title to string, handling object types
|
||||
* Returns empty string if no meaningful title
|
||||
* Uses try/catch to handle circular references safely
|
||||
*/
|
||||
export const safeTitle = (title: unknown): string => {
|
||||
if (typeof title === 'string' && title.trim()) {
|
||||
return title;
|
||||
}
|
||||
if (title && typeof title === 'object') {
|
||||
return JSON.stringify(title);
|
||||
try {
|
||||
return JSON.stringify(title);
|
||||
} catch (_e) {
|
||||
// Handle circular references or BigInt
|
||||
return String(title);
|
||||
}
|
||||
}
|
||||
return '';
|
||||
};
|
||||
|
|
@ -148,6 +154,8 @@ export const shouldShowToolCall = (kind: string): boolean =>
|
|||
|
||||
/**
|
||||
* Group tool call content by type to avoid duplicate labels
|
||||
* Note: Only treats content as error if contentObj.type === 'error'
|
||||
* or if contentObj.error is explicitly set (not null/undefined)
|
||||
*/
|
||||
export const groupContent = (content?: ToolCallContent[]): GroupedContent => {
|
||||
const textOutputs: string[] = [];
|
||||
|
|
@ -161,7 +169,11 @@ export const groupContent = (content?: ToolCallContent[]): GroupedContent => {
|
|||
} else if (item.content) {
|
||||
const contentObj = item.content;
|
||||
|
||||
if (contentObj.type === 'error' || 'error' in contentObj) {
|
||||
// Only treat as error if type is explicitly 'error'
|
||||
// or if error field has a truthy value (not null/undefined)
|
||||
const hasError = contentObj.type === 'error' || contentObj.error != null;
|
||||
|
||||
if (hasError) {
|
||||
let errorMsg = '';
|
||||
|
||||
if (typeof contentObj.error === 'string') {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue