feat(mobile): hide viewed files in pull request file views

apps/mobile/src/features/pull-requests/PullRequestFiles.tsx

@@ -30,6 +30,7 @@ import {
 } from "./usePullRequestDiff";
 import { usePullRequestFileFilter } from "./usePullRequestFileFilter";
 import { usePullRequestFilesViewed } from "./usePullRequestFilesViewed";
+import { usePullRequestReviewDraft } from "./usePullRequestReviewDraft";
 import { pullRequestFileJumpScope, requestPullRequestFileJump } from "./pullRequestFileJump";
 
 const BREAKDOWN_LABELS = {
@@ -54,7 +55,13 @@ function useChangedFiles(params: PullRequestDiffParams) {
   ).changeRequest;
   const diff = usePullRequestDiff(params, terminology);
   const viewed = usePullRequestFilesViewed(params, pr);
-  const filter = usePullRequestFileFilter(diff.parsed);
+  const reviewDraft = usePullRequestReviewDraft(params);
+  const filter = usePullRequestFileFilter(
+    diff.parsed,
+    viewed,
+    undefined,
+    reviewDraft.draft.comments.map((comment) => comment.path),
+  );
   const parsed = filter.parsed;
   return {
     diff,
@@ -78,11 +85,14 @@ function describeFiles(
   { filter, viewed }: ChangedFiles,
 ) {
   const noun = (count: number) => (count === 1 ? "file" : "files");
+  const hiding = filter.hiddenCount > 0 || filter.hiddenViewedCount > 0;
   return [
-    `${parsed.fileCount} ${filter.hiddenCount ? "visible" : "changed"} ${noun(parsed.fileCount)}`,
-    viewed.supported
-      ? `${parsed.files.filter((file) => viewed.isViewed(file.path)).length} viewed`
-      : null,
+    `${parsed.fileCount} ${hiding ? "visible" : "changed"} ${noun(parsed.fileCount)}`,
+    !viewed.supported
+      ? null
+      : filter.hideViewedFiles
+        ? `${filter.hiddenViewedCount} viewed ${noun(filter.hiddenViewedCount)} hidden`
+        : `${filter.viewedCount} viewed`,
     filter.hideTestFiles
       ? `${filter.hiddenCount} test ${noun(filter.hiddenCount)} hidden`
       : "Test files included",
@@ -166,14 +176,26 @@ function FileFilterMenu({ filter }: { readonly filter: ChangedFiles["filter"] })
           image: "line.3.horizontal.decrease",
           state: filter.hideTestFiles ? "on" : "off",
         },
+        ...(filter.canHideViewedFiles
+          ? [
+              {
+                id: "hide-viewed",
+                title: "Hide viewed files",
+                image: "eye.slash",
+                state: filter.hideViewedFiles ? ("on" as const) : ("off" as const),
+              },
+            ]
+          : []),
       ]}
-      onPressAction={filter.toggle}
+      onPressAction={({ nativeEvent }) =>
+        nativeEvent.event === "hide-viewed" ? filter.toggleViewedFiles() : filter.toggle()
+      }
     >
       <View pointerEvents="none">
         <IconButton
           icon="line.3.horizontal.decrease"
           accessibilityLabel="Filter review files"
-          selected={filter.hideTestFiles}
+          selected={filter.hideTestFiles || filter.hideViewedFiles}
         />
       </View>
     </Menu>
@@ -182,6 +204,18 @@ function FileFilterMenu({ filter }: { readonly filter: ChangedFiles["filter"] })
 
 function NoVisibleFiles({ files }: { readonly files: ChangedFiles }) {
   const { diff, filter, parsed } = files;
+  if (parsed.kind === "files" && parsed.files.length === 0 && filter.hiddenViewedCount > 0) {
+    return (
+      <EmptyState
+        title={diff.nextCursor ? "No unviewed files loaded yet" : "All files are hidden"}
+        detail="Viewed files are hidden from this review."
+        action={
+          <Button label="Show viewed files" tone="contrast" onPress={filter.showViewedFiles} />
+        }
+        variant="plain"
+      />
+    );
+  }
   if (parsed.kind === "files") {
     return parsed.files.length === 0 && filter.hiddenCount > 0 ? (
       <EmptyState
@@ -427,6 +461,17 @@ export function PullRequestFilesSheet({ route }: StaticScreenProps<PullRequestDi
                 selected: filter.hideTestFiles,
                 onPress: filter.toggle,
               },
+              ...(filter.canHideViewedFiles
+                ? [
+                    {
+                      id: "hide-viewed",
+                      title: "Hide viewed files",
+                      icon: "eye.slash",
+                      selected: filter.hideViewedFiles,
+                      onPress: filter.toggleViewedFiles,
+                    },
+                  ]
+                : []),
             ],
           },
         ]}

apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx

@@ -977,16 +977,22 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
       ? result.parsed.files.find((file) => pullRequestDiffPathMatches(linkAnchor.digest, file.path))
           ?.path
       : undefined;
+  const viewed = usePullRequestFilesViewed(route.params, commit ? null : detail.data);
+  const reviewDraft = usePullRequestReviewDraft(route.params);
+  const [pickedPath, setJumpPath] = useState<string | null>(null);
   const filter = usePullRequestFileFilter(
     result.parsed,
-    linkPath ?? (route.params.focus ? route.params.path : undefined),
+    viewed,
+    linkPath ?? (route.params.path || undefined),
+    [
+      ...reviewDraft.draft.comments.map((comment) => comment.path),
+      ...(pickedPath ? [pickedPath] : []),
+    ],
   );
   const { refresh: refreshDetail } = detail;
   useFocusEffect(refreshDetail);
   const { loadMore } = result;
   const parsed = filter.parsed;
-  const reviewDraft = usePullRequestReviewDraft(route.params);
-  const viewed = usePullRequestFilesViewed(route.params, commit ? null : detail.data);
   const [commitViewedPaths, setCommitViewedPaths] = useState<ReadonlySet<string>>(() => new Set());
   const permissions = usePullRequestPermissions(route.params, detail.data);
   const scopeKey = pullRequestReviewKey(route.params);
@@ -1052,7 +1058,6 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
       code: row.content ?? "",
     });
   };
-  const [pickedPath, setJumpPath] = useState<string | null>(null);
   const jumpPath = pickedPath ?? linkPath ?? route.params.path;
   const requestedFile =
     result.parsed.kind === "files"
@@ -1091,18 +1096,24 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
   const [collapsedIds, setCollapsedIds] = useState<readonly string[]>(EMPTY_IDS);
   const [visibleFileId, setVisibleFileId] = useState<string | null>(null);
   const nativeView = useRef<NativeReviewDiffViewHandle>(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 filters the current jump was made under. Hiding or showing files replaces the rows,
+  // so a jump made under other filters has to run again. Native applies the entry
   // destination before its first viewport, so the first one counts as made.
-  const jumpedUnder = useRef<boolean | null>(targetFile ? filter.hideTestFiles : null);
+  const filterChoice = {
+    hideTestFiles: filter.hideTestFiles,
+    hideViewedFiles: filter.hideViewedFiles,
+  };
+  const jumpedUnder = useRef<typeof filterChoice | null>(targetFile ? filterChoice : null);
   const hasTestFiles =
     result.parsed.kind === "files" &&
     result.parsed.files.some((file) => isTestChangedFile(file.path));
   const jumpStarted = () =>
-    jumpedUnder.current !== null && (!hasTestFiles || jumpedUnder.current === filter.hideTestFiles);
+    jumpedUnder.current !== null &&
+    (!hasTestFiles || jumpedUnder.current.hideTestFiles === filter.hideTestFiles) &&
+    (!filter.viewedCount || jumpedUnder.current.hideViewedFiles === filter.hideViewedFiles);
   const scrollToFile = (fileId: string) => {
     if (!nativeView.current) return;
-    jumpedUnder.current = filter.hideTestFiles;
+    jumpedUnder.current = filterChoice;
     void nativeView.current.scrollToFile(fileId, false).catch(() => {
       jumpedUnder.current = null;
     });
@@ -1147,7 +1158,7 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
       nativeView,
       jumpStarted,
       setJumpStarted: (started) => {
-        jumpedUnder.current = started ? filter.hideTestFiles : null;
+        jumpedUnder.current = started ? filterChoice : null;
       },
       onDebug: bridge.onDebug,
       loadMore,
@@ -1160,19 +1171,26 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
     );
   };
   const visibleFile = data.files.find((file) => file.id === visibleFileId) ?? targetFile;
-  const toggleTestFiles = () => {
+  const keepVisibleFile = () => {
     setJumpPath(visibleFile?.path ?? route.params.path);
     setSelectedRowId(null);
+  };
+  const toggleTestFiles = () => {
+    keepVisibleFile();
     filter.toggle();
   };
+  const toggleViewedFiles = () => {
+    keepVisibleFile();
+    filter.toggleViewedFiles();
+  };
   const title = commit ? "Commit changes" : "Changes";
   const seekingFile = !targetFile && result.nextCursor !== null;
   const loadingDiff = (result.isPending && !data.rows.length) || (seekingFile && !result.error);
   return (
     <View className="flex-1 android:bg-header ios:bg-screen">
       <AppBar
         title={title}
-        subtitle={`${commit ? commit.slice(0, 7) : `#${route.params.number}`} · ${data.files.length} ${data.files.length === 1 ? "file" : "files"}${filter.hideTestFiles && filter.hiddenCount ? ` · ${filter.hiddenCount} ${filter.hiddenCount === 1 ? "test" : "tests"} hidden` : ""}`}
+        subtitle={`${commit ? commit.slice(0, 7) : `#${route.params.number}`} · ${data.files.length} ${data.files.length === 1 ? "file" : "files"}${filter.hideTestFiles && filter.hiddenCount ? ` · ${filter.hiddenCount} ${filter.hiddenCount === 1 ? "test" : "tests"} hidden` : ""}${filter.hiddenViewedCount ? ` · ${filter.hiddenViewedCount} viewed hidden` : ""}`}
         onBack={() => navigation.goBack()}
         menus={[
           {
@@ -1221,6 +1239,17 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
                 selected: filter.hideTestFiles,
                 onPress: toggleTestFiles,
               },
+              ...(filter.canHideViewedFiles
+                ? [
+                    {
+                      id: "hide-viewed",
+                      title: "Hide viewed files",
+                      icon: "eye.slash",
+                      selected: filter.hideViewedFiles,
+                      onPress: toggleViewedFiles,
+                    },
+                  ]
+                : []),
               ...(selectedRow && selectedFile && canCommentOnDiff
                 ? [
                     {
@@ -1363,6 +1392,25 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
                 contentResetKey={`pull-request:${route.params.environmentId}:${route.params.repository}:${route.params.number}:${commit ?? "all"}`}
               />
             </View>
+          ) : filter.hiddenViewedCount > 0 &&
+            !data.rows.length &&
+            !result.isPending &&
+            !result.error &&
+            !seekingFile ? (
+            <View style={{ paddingTop: contentTopInset }}>
+              <EmptyState
+                title="All files are hidden"
+                detail="Viewed files are hidden from this review."
+                action={
+                  <Button
+                    label="Show viewed files"
+                    tone="contrast"
+                    onPress={filter.showViewedFiles}
+                  />
+                }
+                variant="plain"
+              />
+            </View>
           ) : filter.hiddenCount > 0 &&
             !data.rows.length &&
             !result.isPending &&

apps/mobile/src/features/pull-requests/pullRequestFileFilters.test.ts

@@ -1,7 +1,11 @@
 import { describe, expect, it } from "vite-plus/test";
 import { buildNativeReviewDiffData } from "../review/nativeReviewDiffAdapter";
 import { buildReviewParsedDiff } from "../review/reviewModel";
-import { filterPullRequestTestFiles } from "./pullRequestFileFilters";
+import {
+  filterPullRequestTestFiles,
+  filterPullRequestViewedFiles,
+  viewedFilePaths,
+} from "./pullRequestFileFilters";
 
 function patch(path: string) {
   return `diff --git a/${path} b/${path}\n--- a/${path}\n+++ b/${path}\n@@ -1 +1 @@\n-old\n+new\n`;
@@ -42,6 +46,31 @@ describe("mobile PR test file filter", () => {
     expect(visible.files[1]).toBe(parsed.files[2]);
   });
 
+  it("hides viewed files except those kept on screen", () => {
+    const parsed = buildReviewParsedDiff(
+      ["src/read.ts", "src/opened.ts", "src/drafted.ts", "src/unread.ts"].map(patch).join("\n"),
+      "pr",
+    );
+    const paths = viewedFilePaths(
+      parsed,
+      (path) => path !== "src/unread.ts",
+      (path) => path === "src/opened.ts" || path === "src/drafted.ts",
+    );
+    expect(paths).toEqual({
+      viewed: ["src/read.ts", "src/opened.ts", "src/drafted.ts"],
+      hidden: ["src/read.ts"],
+    });
+    const visible = filterPullRequestViewedFiles(parsed, new Set(paths.hidden));
+    if (parsed.kind !== "files" || visible.kind !== "files") throw new Error("Expected files");
+    expect(visible.files.map((file) => file.path)).toEqual([
+      "src/opened.ts",
+      "src/drafted.ts",
+      "src/unread.ts",
+    ]);
+    expect(visible.fileCount).toBe(3);
+    expect(filterPullRequestViewedFiles(parsed, new Set())).toBe(parsed);
+  });
+
   it("uses the current path when a file is renamed out of a test directory", () => {
     const parsed = buildReviewParsedDiff(
       "diff --git a/tests/helper.ts b/src/helper.ts\nsimilarity index 100%\nrename from tests/helper.ts\nrename to src/helper.ts\n",

apps/mobile/src/features/pull-requests/pullRequestFileFilters.ts

@@ -1,15 +1,9 @@
 import { isTestChangedFile } from "@t3tools/shared/changedFileKind";
 import type { ReviewParsedDiff } from "../review/reviewModel";
 
-export function filterPullRequestTestFiles(
-  parsed: ReviewParsedDiff,
-  hideTestFiles: boolean,
-  includePath?: string,
-): ReviewParsedDiff {
-  if (!hideTestFiles || parsed.kind !== "files") return parsed;
-  const files = parsed.files.filter(
-    (file) => file.path === includePath || !isTestChangedFile(file.path),
-  );
+function keepFiles(parsed: ReviewParsedDiff, keep: (path: string) => boolean): ReviewParsedDiff {
+  if (parsed.kind !== "files") return parsed;
+  const files = parsed.files.filter((file) => keep(file.path));
   if (files.length === parsed.files.length) return parsed;
   return {
     ...parsed,
@@ -19,3 +13,30 @@ export function filterPullRequestTestFiles(
     deletions: files.reduce((total, file) => total + file.deletions, 0),
   };
 }
+
+export function filterPullRequestTestFiles(
+  parsed: ReviewParsedDiff,
+  hideTestFiles: boolean,
+  includePath?: string,
+): ReviewParsedDiff {
+  if (!hideTestFiles) return parsed;
+  return keepFiles(parsed, (path) => path === includePath || !isTestChangedFile(path));
+}
+
+/** Viewed files among `parsed`, and those the filter hides: every one not kept on screen. */
+export function viewedFilePaths(
+  parsed: ReviewParsedDiff,
+  isViewed: (path: string) => boolean,
+  isKept: (path: string) => boolean,
+) {
+  const viewed =
+    parsed.kind === "files" ? parsed.files.map((file) => file.path).filter(isViewed) : [];
+  return { viewed, hidden: viewed.filter((path) => !isKept(path)) };
+}
+
+export function filterPullRequestViewedFiles(
+  parsed: ReviewParsedDiff,
+  hiddenPaths: ReadonlySet<string>,
+): ReviewParsedDiff {
+  return keepFiles(parsed, (path) => !hiddenPaths.has(path));
+}