Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
66 changes: 66 additions & 0 deletions webview-ui/src/components/chat/ChatTextArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,72 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
}
}

// Special handling for concrete folder selections:
// - Keep the picker open
// - Ensure trailing slash
// - Insert without trailing space
// - Trigger a follow-up search to show folder contents
if (type === ContextMenuOptionType.Folder && value && textAreaRef.current) {
// Normalize folder path to end with a trailing slash
Copy link

Choose a reason for hiding this comment

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

Unescaped spaces in the manually constructed folder path will cause shouldShowContextMenu() to hide the picker due to unescaped whitespace after '@'. Suggest escaping spaces after normalizing the trailing slash, and add the import at the top.

Suggested change:

// Normalize trailing slash first
let folderPath = value
if (!folderPath.endsWith("/")) {
  folderPath = folderPath + "/"
}
// Ensure spaces are escaped because we’re not using insertMention here
if (folderPath.includes(" ") && !folderPath.includes("\\ ")) {
  folderPath = escapeSpaces(folderPath)
}

And add import near other utils imports:

import { escapeSpaces } from "@src/utils/path-mentions"

References: shouldShowContextMenu(text, position) checks for unescaped whitespace (negative lookbehind).

Copy link
Member

Choose a reason for hiding this comment

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

When constructing a folder path manually, escape spaces before insertion; unescaped whitespace after '@' can hide the picker.

let folderPath = value
if (!folderPath.endsWith("/")) {
folderPath = folderPath + "/"
}

// Manually build insertion without a trailing space (more deterministic than insertMention)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Manual insertion bypasses escaping handled by TypeScript.insertMention(). If a folder path contains spaces (e.g. "/my folder/"), unescaped whitespace after "@" can cause TypeScript.shouldShowContextMenu() to evaluate false on subsequent keystrokes, collapsing the picker. Prefer delegating replacement to insertMention(), then strip the trailing space and keep the menu open.

Suggested change
// Manually build insertion without a trailing space (more deterministic than insertMention)
// Normalize to trailing slash
let folderPath = value
if (!folderPath.endsWith("/")) {
folderPath = folderPath + "/"
}
// Use insertMention to handle proper escaping and partial-mention replacement,
// then strip the trailing space to keep the picker logic active for drilldown.
const { newValue, mentionIndex } = insertMention(
textAreaRef.current.value,
cursorPosition,
folderPath,
/* isSlashCommand */ false
)
const updatedValue = newValue.endsWith(" ") ? newValue.slice(0, -1) : newValue
const afterMentionPos = mentionIndex + 1 + folderPath.length
setInputValue(updatedValue)
setCursorPosition(afterMentionPos)
setIntendedCursorPosition(afterMentionPos)
// Keep the context menu open for drill-down, and preselect first item
setShowContextMenu(true)
setSelectedType(null)
setSelectedMenuIndex(0)
// Query: text after '@' up to the caret (here it's just the inserted path)
const nextQuery = folderPath
setSearchQuery(nextQuery)
const reqId = Math.random().toString(36).substring(2, 9)
setSearchRequestId(reqId)
setSearchLoading(true)
vscode.postMessage({
type: "searchFiles",
query: unescapeSpaces(nextQuery),
requestId: reqId,
})
setTimeout(() => {
textAreaRef.current?.focus()
}, 0)
return

Copy link
Member

Choose a reason for hiding this comment

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

Manual replacement (slicing/regex) is brittle for common path tokens; delegate to insertMention() and strip its trailing space to keep the menu open.

const original = textAreaRef.current.value
const beforeCursor = original.slice(0, cursorPosition)
const afterCursor = original.slice(cursorPosition)
const lastAtIndex = beforeCursor.lastIndexOf("@")

let beforeMention = beforeCursor
let afterCursorContent = afterCursor

if (lastAtIndex !== -1) {
// Replace everything from '@' to cursor with the folder path
beforeMention = original.slice(0, lastAtIndex)
// Match insertMention behavior for non-space-delimited languages
const isAlphaNumSpace = /^[a-zA-Z0-9\s]*$/.test(afterCursor)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suffix cleanup is brittle: the isAlphaNumSpace check and replace(/^[^\s]*/, "") can leave stray characters for common path tokens like / . - _. Replacing via TypeScript.insertMention() avoids this entire branch and keeps behavior consistent.

afterCursorContent = isAlphaNumSpace ? afterCursor.replace(/^[^\s]*/, "") : afterCursor
}

const updatedValue = beforeMention + "@" + folderPath + afterCursorContent
const afterMentionPos =
(lastAtIndex !== -1 ? lastAtIndex : beforeCursor.length) + 1 + folderPath.length

setInputValue(updatedValue)
setCursorPosition(afterMentionPos)
setIntendedCursorPosition(afterMentionPos)

// Keep the context menu open for drill-down
setShowContextMenu(true)
setSelectedType(null)
Copy link

Choose a reason for hiding this comment

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

After keeping the menu open for drilldown, reset the highlighted option to maintain stable keyboard navigation. Without this, selectedMenuIndex may point at a stale option from the previous state.

Suggested addition:

setShowContextMenu(true)
setSelectedType(null)
setSelectedMenuIndex(0) // ensure a valid selection after drilldown

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

UX: After inserting the folder and keeping the picker open, preselect the first child so Enter drills in immediately.

Suggested change
setSelectedType(null)
setShowContextMenu(true)
setSelectedType(null)
setSelectedMenuIndex(0)

Copy link
Member

Choose a reason for hiding this comment

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

After keeping the picker open, reset the highlighted option (e.g., index 0) so Enter continues drilldown predictably.


// Compute the next-level query (text after '@' up to the caret)
const nextQuery = updatedValue.slice(
(lastAtIndex !== -1 ? lastAtIndex : beforeCursor.length) + 1,
afterMentionPos,
)
setSearchQuery(nextQuery)

// Kick off a search to populate folder children
const reqId = Math.random().toString(36).substring(2, 9)
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

Using Math.random() for request ID generation could lead to collisions. Consider using crypto.randomUUID() or a more robust ID generation method to ensure uniqueness.

Suggested change
const reqId = Math.random().toString(36).substring(2, 9)
const reqId = crypto.randomUUID()

Copilot uses AI. Check for mistakes.
setSearchRequestId(reqId)
setSearchLoading(true)
vscode.postMessage({
type: "searchFiles",
query: unescapeSpaces(nextQuery),
requestId: reqId,
})

// Keep focus in the textarea for continued typing
setTimeout(() => {
textAreaRef.current?.focus()
}, 0)
Comment on lines +411 to +413
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

Using setTimeout with 0 delay is a code smell that suggests timing issues. Consider using requestAnimationFrame or a more deterministic approach to handle focus management.

Suggested change
setTimeout(() => {
textAreaRef.current?.focus()
}, 0)
requestAnimationFrame(() => {
textAreaRef.current?.focus()
})

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Prefer requestAnimationFrame over a zero-delay timeout for deterministic focus after DOM updates.


return
}

setShowContextMenu(false)
setSelectedType(null)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import React from "react"
import { render, fireEvent, screen } from "@src/utils/test-utils"
import { useExtensionState } from "@src/context/ExtensionStateContext"
import { vscode } from "@src/utils/vscode"
import { ContextMenuOptionType } from "@src/utils/context-mentions"
import { ChatTextArea } from "../ChatTextArea"

// Mock VS Code messaging
vi.mock("@src/utils/vscode", () => ({
vscode: {
postMessage: vi.fn(),
},
}))

// Capture the last props passed to ContextMenu so we can invoke onSelect directly
let lastContextMenuProps: any = null
vi.mock("../ContextMenu", () => {
return {
__esModule: true,
default: (props: any) => {
lastContextMenuProps = props
return <div data-testid="context-menu" />
},
__getLastProps: () => lastContextMenuProps,
}
})

// Mock ExtensionStateContext
vi.mock("@src/context/ExtensionStateContext")

describe("ChatTextArea - folder drilldown behavior", () => {
const defaultProps = {
inputValue: "",
setInputValue: vi.fn(),
onSend: vi.fn(),
sendingDisabled: false,
selectApiConfigDisabled: false,
onSelectImages: vi.fn(),
shouldDisableImages: false,
placeholderText: "Type a message...",
selectedImages: [],
setSelectedImages: vi.fn(),
onHeightChange: vi.fn(),
mode: "architect",
setMode: vi.fn(),
modeShortcutText: "(⌘. for next mode)",
}

beforeEach(() => {
vi.clearAllMocks()
;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
filePaths: ["src/", "src/index.ts"],
openedTabs: [],
taskHistory: [],
cwd: "/test/workspace",
})
})

it("keeps picker open and triggers folder children search when selecting a folder", () => {
const setInputValue = vi.fn()

const { container } = render(<ChatTextArea {...defaultProps} setInputValue={setInputValue} />)

// Type to open the @-context menu and set a query
const textarea = container.querySelector("textarea")!
fireEvent.change(textarea, {
target: { value: "@s", selectionStart: 2 },
})

// Ensure our mocked ContextMenu rendered and captured props
expect(screen.getByTestId("context-menu")).toBeInTheDocument()
const props = lastContextMenuProps
expect(props).toBeTruthy()
expect(typeof props.onSelect).toBe("function")

// Simulate selecting a concrete folder suggestion (e.g. "/src")
props.onSelect(ContextMenuOptionType.Folder, "/src")

// The input should contain "@/src/" with NO trailing space and the picker should remain open
expect(setInputValue).toHaveBeenCalled()
const finalValue = setInputValue.mock.calls.at(-1)?.[0]
expect(finalValue).toBe("@/src/")

// Context menu should still be present (picker remains open)
expect(screen.getByTestId("context-menu")).toBeInTheDocument()

// It should have kicked off a searchFiles request for the folder children
const pm = vscode.postMessage as ReturnType<typeof vi.fn>
expect(pm).toHaveBeenCalled()
const lastMsg = pm.mock.calls.at(-1)?.[0]
expect(lastMsg).toMatchObject({ type: "searchFiles" })
// Query mirrors substring after '@' including leading slash per existing logic
expect(lastMsg.query).toBe("/src/")
expect(typeof lastMsg.requestId).toBe("string")
})
})
Copy link

Choose a reason for hiding this comment

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

Recommend adding a coverage case for folders with spaces to prevent regressions in the manual insertion path where spaces must be escaped, while the search query uses unescaped spaces.

Suggested test:

it("keeps picker open and escapes spaces for folders with spaces", () => {
  const setInputValue = vi.fn()
  const { container } = render(<ChatTextArea {...defaultProps} setInputValue={setInputValue} />)

  const textarea = container.querySelector("textarea")!
  fireEvent.change(textarea, { target: { value: "@my", selectionStart: 3 } })

  const props = (global as any).lastContextMenuProps
  expect(props && typeof props.onSelect).toBe("function")

  // Select a folder with spaces
  props.onSelect(ContextMenuOptionType.Folder, "/My Folder")

  // Should insert escaped spaces and keep picker open
  const finalValue = setInputValue.mock.calls.at(-1)?.[0]
  expect(finalValue).toBe("@/My\\ Folder/")

  const pm = vscode.postMessage as ReturnType<typeof vi.fn>
  const lastMsg = pm.mock.calls.at(-1)?.[0]
  expect(lastMsg).toMatchObject({ type: "searchFiles" })
  expect(lastMsg.query).toBe("/My Folder/") // unescaped in the search query
})

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add coverage for folder names with spaces to catch escaping regressions.

Suggested change
})
it("handles folders with spaces: escapes path, keeps picker open, and triggers children search", () => {
const setInputValue = vi.fn()
const { container } = render(<ChatTextArea {...defaultProps} setInputValue={setInputValue} />)
const textarea = container.querySelector("textarea")!
fireEvent.change(textarea, { target: { value: "@my", selectionStart: 3 } })
expect(screen.getByTestId("context-menu")).toBeInTheDocument()
const props = lastContextMenuProps
expect(props).toBeTruthy()
expect(typeof props.onSelect).toBe("function")
// Simulate selecting a concrete folder with spaces (e.g. "/my folder")
props.onSelect(ContextMenuOptionType.Folder, "/my folder")
// Input should contain "@/my\\ folder/" (escaped space) with NO trailing space
expect(setInputValue).toHaveBeenCalled()
const finalValue = setInputValue.mock.calls.at(-1)?.[0]
expect(finalValue).toBe("@/my\\ folder/")
// Picker remains open
expect(screen.getByTestId("context-menu")).toBeInTheDocument()
// searchFiles is triggered with unescaped query
const pm = vscode.postMessage as ReturnType<typeof vi.fn>
expect(pm).toHaveBeenCalled()
const lastMsg = pm.mock.calls.at(-1)?.[0]
expect(lastMsg).toMatchObject({ type: "searchFiles" })
expect(lastMsg.query).toBe("/my folder/")
expect(typeof lastMsg.requestId).toBe("string")
})

Copy link
Member

Choose a reason for hiding this comment

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

Add a test for folders with spaces: escaped insertion in the input and unescaped query in searchFiles.

Loading