From 27a70ad70f30faf30d159f56b394c01f9474c7a4 Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Fri, 20 Mar 2026 20:14:18 +0530 Subject: [PATCH 1/2] fix(app): align review file comments with diff comments (#18406) --- .../app/e2e/session/session-review.spec.ts | 114 ++++++++++++++++++ packages/app/src/pages/session/file-tabs.tsx | 12 -- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/packages/app/e2e/session/session-review.spec.ts b/packages/app/e2e/session/session-review.spec.ts index c0421f0283..07071239d9 100644 --- a/packages/app/e2e/session/session-review.spec.ts +++ b/packages/app/e2e/session/session-review.spec.ts @@ -169,6 +169,70 @@ async function overflow(page: Parameters[0]["page"], file: string) } } +async function openReviewFile(page: Parameters[0]["page"], file: string) { + const row = page.locator(`[data-file="${file}"]`).first() + await expect(row).toBeVisible() + await row.hover() + + const open = row.getByRole("button", { name: /^Open file$/i }).first() + await expect(open).toBeVisible() + await open.click() + + const tab = page.getByRole("tab", { name: file }).first() + await expect(tab).toBeVisible() + await tab.click() + + const viewer = page.locator('[data-component="file"][data-mode="text"]').first() + await expect(viewer).toBeVisible() + return viewer +} + +async function fileComment(page: Parameters[0]["page"], note: string) { + const viewer = page.locator('[data-component="file"][data-mode="text"]').first() + await expect(viewer).toBeVisible() + + const line = viewer.locator('diffs-container [data-line="2"]').first() + await expect(line).toBeVisible() + await line.hover() + + const add = viewer.getByRole("button", { name: /^Comment$/ }).first() + await expect(add).toBeVisible() + await add.click() + + const area = viewer.locator('[data-slot="line-comment-textarea"]').first() + await expect(area).toBeVisible() + await area.fill(note) + + const submit = viewer.locator('[data-slot="line-comment-action"][data-variant="primary"]').first() + await expect(submit).toBeEnabled() + await submit.click() + + await expect(viewer.locator('[data-slot="line-comment-content"]').filter({ hasText: note }).first()).toBeVisible() + await expect(viewer.locator('[data-slot="line-comment-tools"]').first()).toBeVisible() +} + +async function fileOverflow(page: Parameters[0]["page"]) { + const viewer = page.locator('[data-component="file"][data-mode="text"]').first() + const view = page.locator('[role="tabpanel"] .scroll-view__viewport').first() + const pop = viewer.locator('[data-slot="line-comment-popover"][data-inline-body]').first() + const tools = viewer.locator('[data-slot="line-comment-tools"]').first() + + const [width, viewBox, popBox, toolsBox] = await Promise.all([ + view.evaluate((el) => el.scrollWidth - el.clientWidth), + view.boundingBox(), + pop.boundingBox(), + tools.boundingBox(), + ]) + + if (!viewBox || !popBox || !toolsBox) return null + + return { + width, + pop: popBox.x + popBox.width - (viewBox.x + viewBox.width), + tools: toolsBox.x + toolsBox.width - (viewBox.x + viewBox.width), + } +} + test("review applies inline comment clicks without horizontal overflow", async ({ page, withProject }) => { test.setTimeout(180_000) @@ -218,6 +282,56 @@ test("review applies inline comment clicks without horizontal overflow", async ( }) }) +test("review file comments submit on click without clipping actions", async ({ page, withProject }) => { + test.setTimeout(180_000) + + const tag = `review-file-comment-${Date.now()}` + const file = `review-file-comment-${tag}.txt` + const note = `comment ${tag}` + + await page.setViewportSize({ width: 1280, height: 900 }) + + await withProject(async (project) => { + const sdk = createSdk(project.directory) + + await withSession(sdk, `e2e review file comment ${tag}`, async (session) => { + await patch(sdk, session.id, seed([{ file, mark: tag }])) + + await expect + .poll( + async () => { + const diff = await sdk.session.diff({ sessionID: session.id }).then((res) => res.data ?? []) + return diff.length + }, + { timeout: 60_000 }, + ) + .toBe(1) + + await project.gotoSession(session.id) + await show(page) + + const tab = page.getByRole("tab", { name: /Review/i }).first() + await expect(tab).toBeVisible() + await tab.click() + + await expand(page) + await waitMark(page, file, tag) + await openReviewFile(page, file) + await fileComment(page, note) + + await expect + .poll(async () => (await fileOverflow(page))?.width ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + await expect + .poll(async () => (await fileOverflow(page))?.pop ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + await expect + .poll(async () => (await fileOverflow(page))?.tools ?? Number.POSITIVE_INFINITY, { timeout: 10_000 }) + .toBeLessThanOrEqual(1) + }) + }) +}) + test("review keeps scroll position after a live diff update", async ({ page, withProject }) => { test.skip(Boolean(process.env.CI), "Flaky in CI for now.") test.setTimeout(180_000) diff --git a/packages/app/src/pages/session/file-tabs.tsx b/packages/app/src/pages/session/file-tabs.tsx index a3379905d8..e3b57fdf28 100644 --- a/packages/app/src/pages/session/file-tabs.tsx +++ b/packages/app/src/pages/session/file-tabs.tsx @@ -217,17 +217,6 @@ export function FileTabContent(props: { tab: string }) { onDelete={controls.remove} /> ), - onDraftPopoverFocusOut: (e: FocusEvent) => { - const current = e.currentTarget as HTMLDivElement - const target = e.relatedTarget - if (target instanceof Node && current.contains(target)) return - - setTimeout(() => { - if (!document.activeElement || !current.contains(document.activeElement)) { - setNote("commenting", null) - } - }, 0) - }, }) createEffect(() => { @@ -426,7 +415,6 @@ export function FileTabContent(props: { tab: string }) { commentsUi.onLineSelectionEnd(range) }} search={search} - overflow="scroll" class="select-text" media={{ mode: "auto", From d0a57305efcf03f4fd69ca180d97ea85e6cb2f1d Mon Sep 17 00:00:00 2001 From: Brendan Allan Date: Fri, 20 Mar 2026 23:02:07 +0800 Subject: [PATCH 2/2] app: file type filter on desktop + multiple files (#18403) --- packages/app/src/components/prompt-input.tsx | 9 +- .../app/src/components/prompt-input/files.ts | 59 +----------- packages/app/src/constants/file-picker.ts | 89 +++++++++++++++++++ packages/app/src/context/platform.tsx | 2 +- packages/app/src/index.ts | 1 + packages/desktop-electron/src/main/ipc.ts | 11 ++- .../desktop-electron/src/preload/types.ts | 2 + .../desktop-electron/src/renderer/index.tsx | 4 + packages/desktop/src/index.tsx | 3 + 9 files changed, 120 insertions(+), 60 deletions(-) create mode 100644 packages/app/src/constants/file-picker.ts diff --git a/packages/app/src/components/prompt-input.tsx b/packages/app/src/components/prompt-input.tsx index 55cfaa490f..f3d3e135de 100644 --- a/packages/app/src/components/prompt-input.tsx +++ b/packages/app/src/components/prompt-input.tsx @@ -1383,11 +1383,16 @@ export const PromptInput: Component = (props) => { { - const file = e.currentTarget.files?.[0] - if (file) void addAttachment(file) + const list = e.currentTarget.files + if (list) { + for (const file of Array.from(list)) { + void addAttachment(file) + } + } e.currentTarget.value = "" }} /> diff --git a/packages/app/src/components/prompt-input/files.ts b/packages/app/src/components/prompt-input/files.ts index 594991d07a..eae8af03d9 100644 --- a/packages/app/src/components/prompt-input/files.ts +++ b/packages/app/src/components/prompt-input/files.ts @@ -1,4 +1,6 @@ -export const ACCEPTED_IMAGE_TYPES = ["image/png", "image/jpeg", "image/gif", "image/webp"] +import { ACCEPTED_FILE_TYPES, ACCEPTED_IMAGE_TYPES } from "@/constants/file-picker" + +export { ACCEPTED_FILE_TYPES } const IMAGE_MIMES = new Set(ACCEPTED_IMAGE_TYPES) const IMAGE_EXTS = new Map([ @@ -18,61 +20,6 @@ const TEXT_MIMES = new Set([ "application/yaml", ]) -export const ACCEPTED_FILE_TYPES = [ - ...ACCEPTED_IMAGE_TYPES, - "application/pdf", - "text/*", - "application/json", - "application/ld+json", - "application/toml", - "application/x-toml", - "application/x-yaml", - "application/xml", - "application/yaml", - ".c", - ".cc", - ".cjs", - ".conf", - ".cpp", - ".css", - ".csv", - ".cts", - ".env", - ".go", - ".gql", - ".graphql", - ".h", - ".hh", - ".hpp", - ".htm", - ".html", - ".ini", - ".java", - ".js", - ".json", - ".jsx", - ".log", - ".md", - ".mdx", - ".mjs", - ".mts", - ".py", - ".rb", - ".rs", - ".sass", - ".scss", - ".sh", - ".sql", - ".toml", - ".ts", - ".tsx", - ".txt", - ".xml", - ".yaml", - ".yml", - ".zsh", -] - const SAMPLE = 4096 function kind(type: string) { diff --git a/packages/app/src/constants/file-picker.ts b/packages/app/src/constants/file-picker.ts new file mode 100644 index 0000000000..c661bc8f36 --- /dev/null +++ b/packages/app/src/constants/file-picker.ts @@ -0,0 +1,89 @@ +export const ACCEPTED_IMAGE_TYPES = ["image/png", "image/jpeg", "image/gif", "image/webp"] + +export const ACCEPTED_FILE_TYPES = [ + ...ACCEPTED_IMAGE_TYPES, + "application/pdf", + "text/*", + "application/json", + "application/ld+json", + "application/toml", + "application/x-toml", + "application/x-yaml", + "application/xml", + "application/yaml", + ".c", + ".cc", + ".cjs", + ".conf", + ".cpp", + ".css", + ".csv", + ".cts", + ".env", + ".go", + ".gql", + ".graphql", + ".h", + ".hh", + ".hpp", + ".htm", + ".html", + ".ini", + ".java", + ".js", + ".json", + ".jsx", + ".log", + ".md", + ".mdx", + ".mjs", + ".mts", + ".py", + ".rb", + ".rs", + ".sass", + ".scss", + ".sh", + ".sql", + ".toml", + ".ts", + ".tsx", + ".txt", + ".xml", + ".yaml", + ".yml", + ".zsh", +] + +const MIME_EXT = new Map([ + ["image/png", "png"], + ["image/jpeg", "jpg"], + ["image/gif", "gif"], + ["image/webp", "webp"], + ["application/pdf", "pdf"], + ["application/json", "json"], + ["application/ld+json", "jsonld"], + ["application/toml", "toml"], + ["application/x-toml", "toml"], + ["application/x-yaml", "yaml"], + ["application/xml", "xml"], + ["application/yaml", "yaml"], +]) + +const TEXT_EXT = ["txt", "text", "md", "markdown", "log", "csv"] + +export const ACCEPTED_FILE_EXTENSIONS = Array.from( + new Set( + ACCEPTED_FILE_TYPES.flatMap((item) => { + if (item.startsWith(".")) return [item.slice(1)] + if (item === "text/*") return TEXT_EXT + const out = MIME_EXT.get(item) + return out ? [out] : [] + }), + ), +).sort() + +export function filePickerFilters(ext?: string[]) { + if (!ext || ext.length === 0) return undefined + return [{ name: "Files", extensions: ext }] +} diff --git a/packages/app/src/context/platform.tsx b/packages/app/src/context/platform.tsx index b8ed58e343..3bdc46391b 100644 --- a/packages/app/src/context/platform.tsx +++ b/packages/app/src/context/platform.tsx @@ -5,7 +5,7 @@ import { ServerConnection } from "./server" type PickerPaths = string | string[] | null type OpenDirectoryPickerOptions = { title?: string; multiple?: boolean } -type OpenFilePickerOptions = { title?: string; multiple?: boolean } +type OpenFilePickerOptions = { title?: string; multiple?: boolean; accept?: string[]; extensions?: string[] } type SaveFilePickerOptions = { title?: string; defaultPath?: string } type UpdateInfo = { updateAvailable: boolean; version?: string } diff --git a/packages/app/src/index.ts b/packages/app/src/index.ts index 6c870dfa4d..53063f48f8 100644 --- a/packages/app/src/index.ts +++ b/packages/app/src/index.ts @@ -1,4 +1,5 @@ export { AppBaseProviders, AppInterface } from "./app" +export { ACCEPTED_FILE_EXTENSIONS, ACCEPTED_FILE_TYPES, filePickerFilters } from "./constants/file-picker" export { useCommand } from "./context/command" export { type DisplayBackend, type Platform, PlatformProvider } from "./context/platform" export { ServerConnection } from "./context/server" diff --git a/packages/desktop-electron/src/main/ipc.ts b/packages/desktop-electron/src/main/ipc.ts index 71b3c33958..543f857a5e 100644 --- a/packages/desktop-electron/src/main/ipc.ts +++ b/packages/desktop-electron/src/main/ipc.ts @@ -6,6 +6,11 @@ import type { InitStep, ServerReadyData, SqliteMigrationProgress, TitlebarTheme, import { getStore } from "./store" import { setTitlebar } from "./windows" +const pickerFilters = (ext?: string[]) => { + if (!ext || ext.length === 0) return undefined + return [{ name: "Files", extensions: ext }] +} + type Deps = { killSidecar: () => void installCli: () => Promise @@ -94,11 +99,15 @@ export function registerIpcHandlers(deps: Deps) { ipcMain.handle( "open-file-picker", - async (_event: IpcMainInvokeEvent, opts?: { multiple?: boolean; title?: string; defaultPath?: string }) => { + async ( + _event: IpcMainInvokeEvent, + opts?: { multiple?: boolean; title?: string; defaultPath?: string; accept?: string[]; extensions?: string[] }, + ) => { const result = await dialog.showOpenDialog({ properties: ["openFile", ...(opts?.multiple ? ["multiSelections" as const] : [])], title: opts?.title ?? "Choose a file", defaultPath: opts?.defaultPath, + filters: pickerFilters(opts?.extensions), }) if (result.canceled) return null return opts?.multiple ? result.filePaths : result.filePaths[0] diff --git a/packages/desktop-electron/src/preload/types.ts b/packages/desktop-electron/src/preload/types.ts index 100508fcdd..f8e6d52c7d 100644 --- a/packages/desktop-electron/src/preload/types.ts +++ b/packages/desktop-electron/src/preload/types.ts @@ -50,6 +50,8 @@ export type ElectronAPI = { multiple?: boolean title?: string defaultPath?: string + accept?: string[] + extensions?: string[] }) => Promise saveFilePicker: (opts?: { title?: string; defaultPath?: string }) => Promise openLink: (url: string) => void diff --git a/packages/desktop-electron/src/renderer/index.tsx b/packages/desktop-electron/src/renderer/index.tsx index 30e882e237..ec2b4d1e7a 100644 --- a/packages/desktop-electron/src/renderer/index.tsx +++ b/packages/desktop-electron/src/renderer/index.tsx @@ -1,6 +1,8 @@ // @refresh reload import { + ACCEPTED_FILE_EXTENSIONS, + ACCEPTED_FILE_TYPES, AppBaseProviders, AppInterface, handleNotificationClick, @@ -111,6 +113,8 @@ const createPlatform = (): Platform => { const result = await window.api.openFilePicker({ multiple: opts?.multiple ?? false, title: opts?.title ?? t("desktop.dialog.chooseFile"), + accept: opts?.accept ?? ACCEPTED_FILE_TYPES, + extensions: opts?.extensions ?? ACCEPTED_FILE_EXTENSIONS, }) return handleWslPicker(result) }, diff --git a/packages/desktop/src/index.tsx b/packages/desktop/src/index.tsx index 65149f34bc..e677956440 100644 --- a/packages/desktop/src/index.tsx +++ b/packages/desktop/src/index.tsx @@ -1,6 +1,8 @@ // @refresh reload import { + ACCEPTED_FILE_EXTENSIONS, + filePickerFilters, AppBaseProviders, AppInterface, handleNotificationClick, @@ -98,6 +100,7 @@ const createPlatform = (): Platform => { directory: false, multiple: opts?.multiple ?? false, title: opts?.title ?? t("desktop.dialog.chooseFile"), + filters: filePickerFilters(opts?.extensions ?? ACCEPTED_FILE_EXTENSIONS), }) return handleWslPicker(result) },