Skip to content

fix: add tooltips to truncated file paths in chat UI #8280

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

Closed
wants to merge 1 commit into from
Closed

fix: add tooltips to truncated file paths in chat UI #8280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions webview-ui/src/components/chat/BatchFilePermission.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { memo } from "react"
import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock"
import { vscode } from "@src/utils/vscode"
import { removeLeadingNonAlphanumeric } from "@src/utils/removeLeadingNonAlphanumeric"
import { StandardTooltip } from "@src/components/ui"

interface FilePermissionItem {
path: string
Expand Down Expand Up @@ -35,10 +36,13 @@ export const BatchFilePermission = memo(({ files = [], onPermissionResponse, ts
<ToolUseBlockHeader
onClick={() => vscode.postMessage({ type: "openFile", text: file.content })}>
{file.path?.startsWith(".") && <span>.</span>}
<span className="whitespace-nowrap overflow-hidden text-ellipsis text-left mr-2 rtl">
{removeLeadingNonAlphanumeric(file.path ?? "") + "\u200E"}
{file.lineSnippet && ` ${file.lineSnippet}`}
</span>
<StandardTooltip
content={`${file.path}${file.lineSnippet ? ` ${file.lineSnippet}` : ""}`}>
Copy link
Contributor Author

@roomote roomote bot Sep 24, 2025

Choose a reason for hiding this comment

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

The tooltip uses file.path directly and may render the literal "undefined" when absent, and it doesn’t prefer the full path provided on FilePermissionItem.content. Use content fallback and nullish coalescing. Suggested:\n content={${file.content ?? file.path ?? ""}${file.lineSnippet ? ${file.lineSnippet} : ""}}\nOptionally add maxWidth for very long paths (e.g., maxWidth: 600).

Copy link
Member

@daniel-lxs daniel-lxs Sep 25, 2025

Choose a reason for hiding this comment

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

[P2] Same issue here. Guard against undefined and add an LRM so the tooltip mirrors the RTL-safe rendering used for the visible text.

Suggested change
content={`${file.path}${file.lineSnippet ? ` ${file.lineSnippet}` : ""}`}>
<StandardTooltip content={`${(file.path ?? "")}${file.lineSnippet ? ` ${file.lineSnippet}` : ""}${(file.path ?? "") || file.lineSnippet ? "\u200E" : ""}`}>

<span className="whitespace-nowrap overflow-hidden text-ellipsis text-left mr-2 rtl">
Copy link
Contributor Author

@roomote roomote bot Sep 24, 2025

Choose a reason for hiding this comment

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

Accessibility: the tooltip trigger is a which is not keyboard-focusable, so keyboard users can’t access the tooltip. Make the trigger focusable and surface the value via aria-label so focus opens the tooltip. Example:\n <span tabIndex={0} aria-label={${file.content ?? file.path ?? ""}${file.lineSnippet ? ${file.lineSnippet} : ""}}>…

{removeLeadingNonAlphanumeric(file.path ?? "") + "\u200E"}
{file.lineSnippet && ` ${file.lineSnippet}`}
</span>
</StandardTooltip>
<div className="flex-grow"></div>
<span className="codicon codicon-link-external text-[13.5px] my-[1px]" />
</ToolUseBlockHeader>
Expand Down
11 changes: 7 additions & 4 deletions webview-ui/src/components/chat/ChatRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { findMatchingResourceOrTemplate } from "@src/utils/mcp"
import { vscode } from "@src/utils/vscode"
import { removeLeadingNonAlphanumeric } from "@src/utils/removeLeadingNonAlphanumeric"
import { getLanguageFromPath } from "@src/utils/getLanguageFromPath"
import { StandardTooltip } from "@src/components/ui"

import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock"
import UpdateTodoListToolBlock from "./UpdateTodoListToolBlock"
Expand Down Expand Up @@ -600,10 +601,12 @@ export const ChatRowContent = ({
className="group"
onClick={() => vscode.postMessage({ type: "openFile", text: tool.content })}>
{tool.path?.startsWith(".") && <span>.</span>}
<span className="whitespace-nowrap overflow-hidden text-ellipsis text-left mr-2 rtl">
{removeLeadingNonAlphanumeric(tool.path ?? "") + "\u200E"}
{tool.reason}
</span>
<StandardTooltip content={`${tool.path}${tool.reason ? ` ${tool.reason}` : ""}`}>
Copy link
Contributor Author

@roomote roomote bot Sep 24, 2025

Choose a reason for hiding this comment

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

The tooltip content interpolates tool.path without nullish coalescing, which can display "undefined". Prefer a safe fallback and, if available, the full path from tool.content. Suggested:\n <StandardTooltip content={${(typeof tool.content === "string" ? tool.content : tool.path) ?? ""}${tool.reason ? ${tool.reason} : ""}}>

Copy link
Member

@daniel-lxs daniel-lxs Sep 25, 2025

Choose a reason for hiding this comment

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

[P2] If tool.path is undefined the tooltip renders 'undefined'. Use a safe fallback and include an LRM to match the visible text’s directionality.

Suggested change
<StandardTooltip content={`${tool.path}${tool.reason ? ` ${tool.reason}` : ""}`}>
<StandardTooltip content={`${(tool.path ?? "")}${tool.reason ? ` ${tool.reason}` : ""}${(tool.path ?? "") || tool.reason ? "\u200E" : ""}`}>

<span className="whitespace-nowrap overflow-hidden text-ellipsis text-left mr-2 rtl">
Copy link
Contributor Author

@roomote roomote bot Sep 24, 2025

Choose a reason for hiding this comment

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

Accessibility: make the tooltip trigger focusable and add an aria-label so keyboard users can access the same info. For bidi consistency similar to the visible text, append \u200E at end of the aria-label. Example:\n <span tabIndex={0} aria-label={${((typeof tool.content === "string" ? tool.content : tool.path) ?? "")}${tool.reason ? ${tool.reason} : ""}\u200E}>…

{removeLeadingNonAlphanumeric(tool.path ?? "") + "\u200E"}
{tool.reason}
</span>
</StandardTooltip>
<div style={{ flexGrow: 1 }}></div>
<SquareArrowOutUpRight
className="w-4 codicon codicon-link-external opacity-0 group-hover:opacity-100 transition-opacity"
Expand Down
Loading