From 8e5d77a76e8cea0128297302a7a2e2861302ce08 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Fri, 6 Nov 2026 21:10:53 -0800 Subject: [PATCH 1/3] feat(clients): hide documentation files in diffs next to hide tests Reviewers already filter out test noise; docs (Markdown and similar) can crowd the same view, so add a matching hide-documentation toggle on web and mobile. --- .../pull-requests/PullRequestFiles.tsx | 78 +++++++++++++++---- .../pull-requests/PullRequestRouteScreen.tsx | 70 +++++++++++++---- .../pullRequestFileFilters.test.ts | 51 +++++++++--- .../pull-requests/pullRequestFileFilters.ts | 25 ++++-- .../pull-requests/usePullRequestFileFilter.ts | 25 ++++-- .../src/persistence/mobile-preferences.ts | 6 ++ apps/web/src/components/DiffPanel.tsx | 35 ++++++++- .../diffs/useHideDocumentationFiles.ts | 9 +++ .../pullRequest/PullRequestCodeTab.tsx | 66 ++++++++++++++-- packages/shared/src/changedFileKind.test.ts | 21 ++++- packages/shared/src/changedFileKind.ts | 4 + 11 files changed, 329 insertions(+), 61 deletions(-) create mode 100644 apps/web/src/components/diffs/useHideDocumentationFiles.ts diff --git a/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx b/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx index 7244945878861..cfaad50ce90e4 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx @@ -78,14 +78,20 @@ function describeFiles( { filter, viewed }: ChangedFiles, ) { const noun = (count: number) => (count === 1 ? "file" : "files"); + const hiddenKinds = [ + filter.hideTestFiles ? "tests" : null, + filter.hideDocumentationFiles ? "docs" : null, + ].filter(Boolean); return [ `${parsed.fileCount} ${filter.hiddenCount ? "visible" : "changed"} ${noun(parsed.fileCount)}`, viewed.supported ? `${parsed.files.filter((file) => viewed.isViewed(file.path)).length} viewed` : null, - filter.hideTestFiles - ? `${filter.hiddenCount} test ${noun(filter.hiddenCount)} hidden` - : "Test files included", + filter.hiddenCount > 0 + ? `${filter.hiddenCount} ${hiddenKinds.join(" + ")} ${noun(filter.hiddenCount)} hidden` + : hiddenKinds.length > 0 + ? `${hiddenKinds.join(" + ")} hidden` + : "All file kinds included", ] .filter(Boolean) .join(" · "); @@ -166,31 +172,68 @@ function FileFilterMenu({ filter }: { readonly filter: ChangedFiles["filter"] }) image: "line.3.horizontal.decrease", state: filter.hideTestFiles ? "on" : "off", }, + { + id: "hide-docs", + title: "Hide documentation", + image: "doc.text", + state: filter.hideDocumentationFiles ? "on" : "off", + }, ]} - onPressAction={filter.toggle} + onPressAction={({ nativeEvent }) => { + if (nativeEvent.event === "hide-tests") filter.toggleTests(); + if (nativeEvent.event === "hide-docs") filter.toggleDocumentation(); + }} > ); } +function hiddenFilesEmptyState(filter: ChangedFiles["filter"], nextCursor: string | null) { + const hidingTests = filter.hideTestFiles; + const hidingDocs = filter.hideDocumentationFiles; + if (hidingTests && hidingDocs) { + return { + title: nextCursor ? "No matching files loaded yet" : "Only tests and docs changed", + detail: "Test and documentation files are hidden from this review.", + label: "Show all files", + }; + } + if (hidingDocs) { + return { + title: nextCursor ? "No non-documentation files loaded yet" : "Only documentation changed", + detail: "Documentation files are hidden from this review.", + label: "Show documentation", + }; + } + return { + title: nextCursor ? "No non-test files loaded yet" : "Only test files changed", + detail: "Test files are hidden from this review.", + label: "Show test files", + }; +} + function NoVisibleFiles({ files }: { readonly files: ChangedFiles }) { const { diff, filter, parsed } = files; if (parsed.kind === "files") { - return parsed.files.length === 0 && filter.hiddenCount > 0 ? ( - } - variant="plain" - /> - ) : null; + if (parsed.files.length === 0 && filter.hiddenCount > 0) { + const empty = hiddenFilesEmptyState(filter, diff.nextCursor); + return ( + } + variant="plain" + /> + ); + } + return null; } return !diff.isPending && !diff.error ? ( @@ -425,7 +468,14 @@ export function PullRequestFilesSheet({ route }: StaticScreenProps(EMPTY_IDS); const [visibleFileId, setVisibleFileId] = useState(null); const nativeView = useRef(null); - // The filter the current jump was made under. Hiding or showing test files replaces the rows, - // so a jump made under the other filter has to run again. Native applies the entry + // The filter the current jump was made under. Hiding or showing filtered files replaces the + // rows, so a jump made under another filter has to run again. Native applies the entry // destination before its first viewport, so the first one counts as made. - const jumpedUnder = useRef(filter.hideTestFiles); - const hasTestFiles = + const fileFilterKey = `${filter.hideTestFiles}:${filter.hideDocumentationFiles}`; + const jumpedUnder = useRef(fileFilterKey); + const hasFilterableFiles = result.parsed.kind === "files" && - result.parsed.files.some((file) => isTestChangedFile(file.path)); + result.parsed.files.some( + (file) => isTestChangedFile(file.path) || isDocumentationChangedFile(file.path), + ); const jumpStarted = () => - jumpedUnder.current !== null && (!hasTestFiles || jumpedUnder.current === filter.hideTestFiles); + jumpedUnder.current !== null && (!hasFilterableFiles || jumpedUnder.current === fileFilterKey); const scrollToFile = (fileId: string) => { if (!nativeView.current) return; - jumpedUnder.current = filter.hideTestFiles; + jumpedUnder.current = fileFilterKey; void nativeView.current.scrollToFile(fileId, false).catch(() => { jumpedUnder.current = null; }); @@ -991,7 +994,7 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) { nativeView, jumpStarted, setJumpStarted: (started) => { - jumpedUnder.current = started ? filter.hideTestFiles : null; + jumpedUnder.current = started ? fileFilterKey : null; }, onDebug: bridge.onDebug, loadMore, @@ -1004,11 +1007,15 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) { ); }; const visibleFile = data.files.find((file) => file.id === visibleFileId) ?? targetFile; - const toggleTestFiles = () => { + const toggleFileFilter = (toggle: () => void) => { setJumpPath(visibleFile?.path ?? route.params.path); setSelectedRowId(null); - filter.toggle(); + toggle(); }; + const hiddenKinds = [ + filter.hideTestFiles ? "tests" : null, + filter.hideDocumentationFiles ? "docs" : null, + ].filter(Boolean); const title = commit ? "Commit changes" : "Changes"; const seekingFile = !targetFile && result.nextCursor !== null; const loadingDiff = (result.isPending && !data.rows.length) || (seekingFile && !result.error); @@ -1016,7 +1023,7 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) { navigation.goBack()} menus={[ { @@ -1063,7 +1070,14 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) { title: "Hide test files", icon: "line.3.horizontal.decrease", selected: filter.hideTestFiles, - onPress: toggleTestFiles, + onPress: () => toggleFileFilter(filter.toggleTests), + }, + { + id: "hide-docs", + title: "Hide documentation", + icon: "doc.text", + selected: filter.hideDocumentationFiles, + onPress: () => toggleFileFilter(filter.toggleDocumentation), }, ...(selectedRow && selectedFile && canCommentOnDiff ? [ @@ -1210,9 +1224,33 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) { !seekingFile ? ( } + title={ + filter.hideTestFiles && filter.hideDocumentationFiles + ? "Only tests and docs changed" + : filter.hideDocumentationFiles + ? "Only documentation changed" + : "Only test files changed" + } + detail={ + filter.hideTestFiles && filter.hideDocumentationFiles + ? "Test and documentation files are hidden from this review." + : filter.hideDocumentationFiles + ? "Documentation files are hidden from this review." + : "Test files are hidden from this review." + } + action={ + ) : null} + {hiddenDocumentationFileCount > 0 ? ( + + ) : null} {filesViewed.enabled && files.length > 0 ? ( @@ -1385,6 +1417,26 @@ function PullRequestCodeTab({ ) : null} + {documentationFileCount > 0 ? ( + + setHideDocumentationFiles(Boolean(pressed))} + /> + } + > + + + + {hideDocumentationFiles ? "Show documentation" : "Hide documentation"} + + + ) : null} {fileKeys.length > 0 ? ( { it.each([ @@ -16,3 +20,18 @@ describe("classifyChangedFile", () => { expect(classifyChangedFile(path)).toBe(kind); }); }); + +describe("changed file kind helpers", () => { + it.each([ + ["src/app.test.ts", true, false], + ["src/__mocks__/client.ts", true, false], + ["README.md", false, true], + ["docs/guide.mdx", false, true], + ["tests/README.md", true, false], + ["src/app.ts", false, false], + ["requirements.txt", false, false], + ] as const)("%s", (path, test, docs) => { + expect(isTestChangedFile(path)).toBe(test); + expect(isDocumentationChangedFile(path)).toBe(docs); + }); +}); diff --git a/packages/shared/src/changedFileKind.ts b/packages/shared/src/changedFileKind.ts index 8e7d59f15eddf..7f93fad9e630c 100644 --- a/packages/shared/src/changedFileKind.ts +++ b/packages/shared/src/changedFileKind.ts @@ -73,6 +73,10 @@ export function isTestChangedFile(path: string): boolean { return kind === "tests" || kind === "mocks"; } +export function isDocumentationChangedFile(path: string): boolean { + return classifyChangedFile(path) === "docs"; +} + export function classifyChangedFile(path: string): ChangedFileKind { const segments = path.replaceAll("\\", "/").split("/"); const name = segments.pop() ?? ""; From 28bb3ed11667cd6bd863a1d2036d1f49c1fea3f5 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 11 Nov 2026 14:54:25 +0000 Subject: [PATCH 2/3] fix(web): say files are filtered when every changed file is hidden --- apps/web/src/components/pullRequest/PullRequestCodeTab.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index 7786d4e775355..bc02720d30f08 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -1596,7 +1596,11 @@ function PullRequestCodeTab({ if (items.length === 0 && nextCursor === null) { return withToolbar(

- {commit === null ? `This ${term} has no file changes.` : "This commit has no file changes."} + {loadedFiles.length > 0 + ? "All changed files are hidden by the file filters." + : commit === null + ? `This ${term} has no file changes.` + : "This commit has no file changes."}

, ); } From 3d0aeead502f38993b862af0103282c647c12aa2 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Wed, 11 Nov 2026 09:50:59 -0800 Subject: [PATCH 3/3] fix(web): keep PR conversations when filters hide every file A fully filtered file list still needs the orphan conversation rail and toolbar recovery, not the empty-diff early return. Co-authored-by: Cursor --- .../pullRequest/PullRequestCodeTab.tsx | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index bc02720d30f08..833f5eb5c3f24 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -1593,14 +1593,11 @@ function PullRequestCodeTab({ return withToolbar(rawSlicesView); } - if (items.length === 0 && nextCursor === null) { + // Filtered-empty still needs the conversation rail; only a real empty diff bails here. + if (items.length === 0 && nextCursor === null && loadedFiles.length === 0) { return withToolbar(

- {loadedFiles.length > 0 - ? "All changed files are hidden by the file filters." - : commit === null - ? `This ${term} has no file changes.` - : "This commit has no file changes."} + {commit === null ? `This ${term} has no file changes.` : "This commit has no file changes."}

, ); } @@ -1697,23 +1694,29 @@ function PullRequestCodeTab({ } }} > - {/* The viewer must own its scroll element to position virtual rows within it. */} - - - className="@container h-full overflow-auto [scrollbar-gutter:stable]" - containerRef={scrollerRef} - viewerRef={codeViewReveal.attach} - items={items} - selectedLines={selectedLines} - onSelectedLinesChange={setSelectedLines} - options={diffViewOptions} - renderCodeViewFooter={renderCodeViewFooter} - renderHeaderPrefix={renderHeaderPrefix} - renderHeaderMetadata={renderHeaderMetadata} - renderAnnotation={renderAnnotation} - unsafeCSSExtra={REPLACE_FILE_COUNTS_CSS} - /> - + {items.length === 0 && loadedFiles.length > 0 ? ( +

+ All changed files are hidden by the file filters. +

+ ) : ( + // The viewer must own its scroll element to position virtual rows within it. + + + className="@container h-full overflow-auto [scrollbar-gutter:stable]" + containerRef={scrollerRef} + viewerRef={codeViewReveal.attach} + items={items} + selectedLines={selectedLines} + onSelectedLinesChange={setSelectedLines} + options={diffViewOptions} + renderCodeViewFooter={renderCodeViewFooter} + renderHeaderPrefix={renderHeaderPrefix} + renderHeaderMetadata={renderHeaderMetadata} + renderAnnotation={renderAnnotation} + unsafeCSSExtra={REPLACE_FILE_COUNTS_CSS} + /> + + )} {fileTreeOpen ? (