Repository navigation
feat(web): add pull request file sidebar #6373
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.
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
Closed
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
42f38ba
feat(web): add pull request file sidebar
ShpetimA 09c904f
fix(web): move pull request files toggle to toolbar
ShpetimA b9c70f8
refactor(web): share Pierre tree theme
ShpetimA 84058bf
fix(web): keep pull request file sidebar compact
ShpetimA be68e6c
fix: Update apps/web/src/components/pullRequest/PullRequestDiffFileTr…
ShpetimA 0873cd2
fix(web): combine pull request collapse controls
ShpetimA 47a4eb2
fix: Update apps/web/src/components/pullRequest/PullRequestDiffFileTr…
ShpetimA 1815f3b
fix(web): reset pull request diff expansion on scope change
ShpetimA a4bb3c7
refactor(web): use React 19 ref prop for diff file tree
ShpetimA 7b7bca4
Merge remote-tracking branch 'upstream/main' into pr-files-sidebar
ShpetimA File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
273 changes: 179 additions & 94 deletions
273
apps/web/src/components/pullRequest/PullRequestCodeTab.tsx
Large diffs are not rendered by default.
Oops, something went wrong.
68 changes: 68 additions & 0 deletions
68
apps/web/src/components/pullRequest/PullRequestDiffFileTree.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import type { FileDiffMetadata } from "@pierre/diffs"; | ||
| import { renderToStaticMarkup } from "react-dom/server"; | ||
| import { describe, expect, it } from "vite-plus/test"; | ||
|
|
||
| import { PullRequestDiffFileTree } from "./PullRequestDiffFileTree"; | ||
|
|
||
| function changedFile(name: string): FileDiffMetadata { | ||
| return { name, type: "change", hunks: [] } as unknown as FileDiffMetadata; | ||
| } | ||
|
|
||
| describe("PullRequestDiffFileTree", () => { | ||
| it("shows loaded-file progress in the pagination action", () => { | ||
| const markup = renderToStaticMarkup( | ||
| <PullRequestDiffFileTree | ||
| files={[changedFile("src/a.ts"), changedFile("src/b.ts")]} | ||
| totalFileCount={80} | ||
| initiallyExpanded | ||
| hasMore | ||
| isLoadingMore={false} | ||
| loadMoreFailed={false} | ||
| onLoadMore={() => {}} | ||
| onSelectFile={() => {}} | ||
| />, | ||
| ); | ||
|
|
||
| expect(markup).toContain("Load more · 2 of 80 loaded"); | ||
| expect(markup).toContain("width:2.5%"); | ||
| expect(markup).toContain('aria-busy="false"'); | ||
| expect(markup).not.toContain("all folders"); | ||
| }); | ||
|
|
||
| it("keeps the current progress visible while the next page loads", () => { | ||
| const markup = renderToStaticMarkup( | ||
| <PullRequestDiffFileTree | ||
| files={[changedFile("src/a.ts"), changedFile("src/b.ts")]} | ||
| totalFileCount={80} | ||
| initiallyExpanded | ||
| hasMore | ||
| isLoadingMore | ||
| loadMoreFailed={false} | ||
| onLoadMore={() => {}} | ||
| onSelectFile={() => {}} | ||
| />, | ||
| ); | ||
|
|
||
| expect(markup).toContain("Loading · 2 of 80 loaded"); | ||
| expect(markup).toContain('aria-busy="true"'); | ||
| }); | ||
|
|
||
| it("does not present an exhausted reported count as complete while more files remain", () => { | ||
| const markup = renderToStaticMarkup( | ||
| <PullRequestDiffFileTree | ||
| files={[changedFile("src/a.ts"), changedFile("src/b.ts")]} | ||
| totalFileCount={2} | ||
| initiallyExpanded | ||
| hasMore | ||
| isLoadingMore={false} | ||
| loadMoreFailed={false} | ||
| onLoadMore={() => {}} | ||
| onSelectFile={() => {}} | ||
| />, | ||
| ); | ||
|
|
||
| expect(markup).toContain("Load more · 2 loaded"); | ||
| expect(markup).not.toContain("2 of 2 loaded"); | ||
| expect(markup).not.toContain("width:100%"); | ||
| }); | ||
| }); |
223 changes: 223 additions & 0 deletions
223
apps/web/src/components/pullRequest/PullRequestDiffFileTree.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| import type { FileDiffMetadata } from "@pierre/diffs"; | ||
| import type { GitStatusEntry } from "@pierre/trees"; | ||
| import { FileTree, useFileTree } from "@pierre/trees/react"; | ||
| import { | ||
| forwardRef, | ||
| useCallback, | ||
| useEffect, | ||
| useImperativeHandle, | ||
| useMemo, | ||
| useRef, | ||
| useState, | ||
| } from "react"; | ||
|
|
||
| import { useTheme } from "~/hooks/useTheme"; | ||
| import { resolveFileDiffPath } from "~/lib/diffRendering"; | ||
| import { T3_PIERRE_ICONS } from "~/pierre-icons"; | ||
| import { PIERRE_TREE_UNSAFE_CSS, pierreTreeStyle } from "~/pierre-tree-theme"; | ||
|
|
||
| import { Button } from "../ui/button"; | ||
| import { getPullRequestFileLoadState } from "./pullRequestDiff.logic"; | ||
|
|
||
| const NO_EXPANDED_DIRECTORIES: ReadonlyArray<string> = []; | ||
|
|
||
| function toGitStatus(file: FileDiffMetadata): GitStatusEntry { | ||
| const path = resolveFileDiffPath(file); | ||
| switch (file.type) { | ||
| case "new": | ||
| return { path, status: "added" }; | ||
| case "deleted": | ||
| return { path, status: "deleted" }; | ||
| case "rename-pure": | ||
| case "rename-changed": | ||
| return { path, status: "renamed" }; | ||
| case "change": | ||
| return { path, status: "modified" }; | ||
| } | ||
| } | ||
|
|
||
| function collectDirectoryPaths(paths: ReadonlyArray<string>): ReadonlyArray<string> { | ||
| const directories = new Set<string>(); | ||
| for (const path of paths) { | ||
| const segments = path.split("/"); | ||
| let directory = ""; | ||
| for (const segment of segments.slice(0, -1)) { | ||
| directory += `${segment}/`; | ||
| directories.add(directory); | ||
| } | ||
| } | ||
| return [...directories]; | ||
| } | ||
|
|
||
| /** The one bulk tree action coordinated by the pull request diff toolbar. */ | ||
| export interface PullRequestDiffFileTreeHandle { | ||
| /** Expands or collapses every directory currently loaded in the tree. */ | ||
| readonly setAllDirectoriesExpanded: (expanded: boolean) => void; | ||
| } | ||
|
|
||
| /** A path-first Pierre tree for the portion of a pull-request diff loaded so far. */ | ||
| export const PullRequestDiffFileTree = forwardRef< | ||
| PullRequestDiffFileTreeHandle, | ||
| { | ||
| readonly files: ReadonlyArray<FileDiffMetadata>; | ||
| /** Null when the selected host commit does not report its own aggregate file count. */ | ||
| readonly totalFileCount: number | null; | ||
| readonly hasMore: boolean; | ||
| readonly isLoadingMore: boolean; | ||
| readonly loadMoreFailed: boolean; | ||
| /** The persisted expansion used when the sidebar mounts. */ | ||
| readonly initiallyExpanded: boolean; | ||
| readonly onLoadMore: () => void; | ||
| readonly onSelectFile: (path: string) => void; | ||
| } | ||
| >(function PullRequestDiffFileTree( | ||
| { | ||
| files, | ||
| totalFileCount, | ||
| hasMore, | ||
| isLoadingMore, | ||
| loadMoreFailed, | ||
| initiallyExpanded, | ||
| onLoadMore, | ||
| onSelectFile, | ||
| }, | ||
| ref, | ||
| ) { | ||
| const { resolvedTheme } = useTheme(); | ||
| const paths = useMemo(() => files.map(resolveFileDiffPath), [files]); | ||
| const directoryPaths = useMemo(() => collectDirectoryPaths(paths), [paths]); | ||
| const gitStatus = useMemo(() => files.map(toGitStatus), [files]); | ||
| const filePathsRef = useRef<ReadonlySet<string>>(new Set(paths)); | ||
| const onSelectFileRef = useRef(onSelectFile); | ||
| const previousPathsRef = useRef<ReadonlyArray<string>>(paths); | ||
| const previousDirectoryPathsRef = useRef<ReadonlyArray<string>>(directoryPaths); | ||
| const newDirectoryExpansionRef = useRef<"open" | "closed">(initiallyExpanded ? "open" : "closed"); | ||
| const [hasRequestedMore, setHasRequestedMore] = useState(false); | ||
| const fileLoadState = getPullRequestFileLoadState(files.length, totalFileCount, hasMore); | ||
| const progressTotal = fileLoadState.knownTotalFileCount; | ||
| const progressPercent = | ||
| progressTotal === null || progressTotal === 0 ? null : (files.length / progressTotal) * 100; | ||
| const loadedFileCount = files.length.toLocaleString(); | ||
| const loadedFileProgress = | ||
| progressTotal === null | ||
| ? `${loadedFileCount} loaded` | ||
| : `${loadedFileCount} of ${progressTotal.toLocaleString()} loaded`; | ||
|
|
||
| useEffect(() => { | ||
| filePathsRef.current = new Set(paths); | ||
| onSelectFileRef.current = onSelectFile; | ||
| }, [onSelectFile, paths]); | ||
|
|
||
| const { model } = useFileTree({ | ||
| density: "compact", | ||
| flattenEmptyDirectories: true, | ||
| initialExpandedPaths: initiallyExpanded ? directoryPaths : NO_EXPANDED_DIRECTORIES, | ||
| initialExpansion: "closed", | ||
| icons: T3_PIERRE_ICONS, | ||
| onSelectionChange: (selectedPaths) => { | ||
| const path = selectedPaths.at(-1)?.replace(/\/$/, ""); | ||
| if (path && filePathsRef.current.has(path)) { | ||
| onSelectFileRef.current(path); | ||
| } | ||
| }, | ||
| paths, | ||
| search: false, | ||
| unsafeCSS: PIERRE_TREE_UNSAFE_CSS, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (previousPathsRef.current === paths) return; | ||
| const previousDirectoryPaths = previousDirectoryPathsRef.current; | ||
| const previousDirectoryPathSet = new Set(previousDirectoryPaths); | ||
| const previouslyExpandedPaths = new Set( | ||
| previousDirectoryPaths.filter((path) => { | ||
| const item = model.getItem(path); | ||
| return item !== null && "isExpanded" in item && item.isExpanded(); | ||
| }), | ||
| ); | ||
| const nextExpandedPaths = directoryPaths.filter((path) => | ||
| previousDirectoryPathSet.has(path) | ||
| ? previouslyExpandedPaths.has(path) | ||
| : newDirectoryExpansionRef.current === "open", | ||
| ); | ||
| previousPathsRef.current = paths; | ||
| previousDirectoryPathsRef.current = directoryPaths; | ||
| model.resetPaths(paths, { initialExpandedPaths: nextExpandedPaths }); | ||
| }, [directoryPaths, model, paths]); | ||
|
|
||
| useEffect(() => { | ||
| model.setGitStatus(gitStatus); | ||
| }, [gitStatus, model]); | ||
|
|
||
| const setAllDirectoriesExpanded = useCallback( | ||
| (expanded: boolean) => { | ||
| newDirectoryExpansionRef.current = expanded ? "open" : "closed"; | ||
| model.resetPaths(paths, { | ||
| initialExpandedPaths: expanded ? directoryPaths : NO_EXPANDED_DIRECTORIES, | ||
| }); | ||
| model.setGitStatus(gitStatus); | ||
| }, | ||
| [directoryPaths, gitStatus, model, paths], | ||
| ); | ||
| useImperativeHandle(ref, () => ({ setAllDirectoriesExpanded }), [setAllDirectoriesExpanded]); | ||
|
|
||
| return ( | ||
| <div className="flex min-h-0 flex-1 flex-col bg-background"> | ||
| <div | ||
| className="flex h-10 min-h-10 shrink-0 items-center gap-2 border-b border-border/60 bg-background px-3 text-xs text-muted-foreground" | ||
| data-surface-subheader | ||
| > | ||
| <span className="font-medium text-foreground">Files</span> | ||
| <span className="ml-auto min-w-6 shrink-0 text-right tabular-nums"> | ||
| {files.length} | ||
| {progressTotal !== null && progressTotal > files.length | ||
| ? ` of ${progressTotal}` | ||
| : fileLoadState.displayedCountIsLowerBound | ||
| ? "+" | ||
| : ""} | ||
| </span> | ||
| </div> | ||
| <FileTree | ||
| model={model} | ||
| aria-label="Pull request files" | ||
| className="min-h-0 flex-1 overflow-hidden" | ||
| style={pierreTreeStyle(resolvedTheme)} | ||
| /> | ||
| {hasMore ? ( | ||
| <div className="shrink-0 border-t border-border/60 p-2"> | ||
| <Button | ||
| type="button" | ||
| size="xs" | ||
| variant="outline" | ||
| aria-busy={isLoadingMore} | ||
| className="relative w-full overflow-hidden bg-transparent tabular-nums dark:bg-transparent" | ||
|
macroscopeapp[bot] marked this conversation as resolved.
Outdated
|
||
| disabled={isLoadingMore} | ||
| onClick={() => { | ||
| setHasRequestedMore(true); | ||
| onLoadMore(); | ||
| }} | ||
| > | ||
| {progressPercent === null ? null : ( | ||
| <span | ||
| aria-hidden | ||
| className="absolute inset-y-0 left-0 bg-primary/8 transition-[width] duration-200 motion-reduce:transition-none" | ||
| style={{ width: `${progressPercent}%` }} | ||
| /> | ||
| )} | ||
| <span className="relative z-10" aria-live="polite"> | ||
| {loadMoreFailed ? "Retry" : isLoadingMore ? "Loading" : "Load more"} | ||
| {` · ${loadedFileProgress}`} | ||
| </span> | ||
| </Button> | ||
| </div> | ||
| ) : hasRequestedMore ? ( | ||
| <div | ||
| role="status" | ||
| className="flex h-11 shrink-0 items-center justify-center border-t border-border/60 px-3 text-xs text-muted-foreground tabular-nums sm:h-10" | ||
| > | ||
| All {loadedFileCount} {files.length === 1 ? "file" : "files"} loaded | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.