Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
71 changes: 48 additions & 23 deletions apps/dashboard/src/components/layouts/dashboard-tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ interface DashboardTabsProps {

export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {
const openTabs = useTabs();
const pathname = useRouterState({ select: (s) => s.location.pathname });
const dragTabRef = useRef<string | null>(null);
const [contextTab, setContextTab] = useState<{
tab: Tab;
Expand Down Expand Up @@ -130,25 +129,12 @@ export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {
dragTabRef.current = null;
}, []);

// biome-ignore lint/correctness/useExhaustiveDependencies: pathname is intentionally used as a trigger to re-run when the route changes
useEffect(() => {
const container = scrollRef.current;
if (!container) return;
const activeTab = container.querySelector<HTMLElement>(".active");
if (!activeTab) return;

const { left: cLeft, right: cRight } = container.getBoundingClientRect();
const { left: tLeft, right: tRight } = activeTab.getBoundingClientRect();

if (tLeft < cLeft || tRight > cRight) {
activeTab.scrollIntoView({ inline: "nearest", block: "nearest" });
updateScrollState();
}
}, [pathname, scrollRef, updateScrollState]);

// Read pathname imperatively in event handlers (rerender-defer-reads)
// so the callbacks are stable and don't bust memo on DetailTab.
const handleCloseTab = useCallback(
(id: string, tabUrl: string) => {
const isActive = pathname === tabUrl;
const currentPath = routerRef.current.state.location.pathname;
const isActive = currentPath === tabUrl;
const index = openTabs.findIndex((tab) => tab.id === id);
const nextTab =
index === -1 ? undefined : (openTabs[index + 1] ?? openTabs[index - 1]);
Expand All @@ -157,7 +143,7 @@ export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {
void routerRef.current.navigate({ to: nextTab?.url ?? "/" });
}
},
[openTabs, pathname, routerRef],
[openTabs, routerRef],
);

const handleContextClose = useCallback(() => {
Expand All @@ -167,20 +153,22 @@ export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {

const handleContextCloseOthers = useCallback(() => {
if (!contextTab) return;
if (pathname !== contextTab.tab.url) {
const currentPath = routerRef.current.state.location.pathname;
if (currentPath !== contextTab.tab.url) {
void routerRef.current.navigate({ to: contextTab.tab.url });
}
removeOtherTabs(contextTab.tab.id);
}, [contextTab, pathname, routerRef]);
}, [contextTab, routerRef]);

const handleContextCloseRight = useCallback(() => {
if (!contextTab) return;
removeTabsToRight(contextTab.tab.id);
}, [contextTab]);

const handleContextCloseMerged = useCallback(() => {
const currentPath = routerRef.current.state.location.pathname;
const activeTabWillClose = openTabs.find(
(t) => pathname === t.url && isMergedTab(t),
(t) => currentPath === t.url && isMergedTab(t),
);
removeMergedTabs();
if (activeTabWillClose) {
Expand All @@ -189,7 +177,7 @@ export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {
to: remaining[0]?.url ?? "/",
});
}
}, [openTabs, pathname, routerRef]);
}, [openTabs, routerRef]);

const hasMergedTabs = openTabs.some(isMergedTab);

Expand Down Expand Up @@ -229,6 +217,10 @@ export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {
onMouseEnter={updateScrollState}
className="no-scrollbar flex w-0 min-w-full items-center gap-0.5 overflow-x-auto"
>
<ScrollActiveTabIntoView
scrollRef={scrollRef}
updateScrollState={updateScrollState}
/>
{openTabs.map((tab, index) => {
const Icon = tabIconMap[tab.type];
return (
Expand Down Expand Up @@ -286,6 +278,39 @@ export function DashboardTabs({ tabsReady, routerRef }: DashboardTabsProps) {
);
}

/**
* Isolated component that subscribes to pathname changes to scroll the active
* tab into view. Extracted so the parent DashboardTabs doesn't re-render on
* every navigation — only this tiny renderless component does.
*/
function ScrollActiveTabIntoView({
scrollRef,
updateScrollState,
}: {
scrollRef: React.RefObject<HTMLDivElement | null>;
updateScrollState: () => void;
}) {
const pathname = useRouterState({ select: (s) => s.location.pathname });

// biome-ignore lint/correctness/useExhaustiveDependencies: pathname triggers scroll-into-view on route change
useEffect(() => {
const container = scrollRef.current;
if (!container) return;
const activeTab = container.querySelector<HTMLElement>(".active");
if (!activeTab) return;

const { left: cLeft, right: cRight } = container.getBoundingClientRect();
const { left: tLeft, right: tRight } = activeTab.getBoundingClientRect();

if (tLeft < cLeft || tRight > cRight) {
activeTab.scrollIntoView({ inline: "nearest", block: "nearest" });
updateScrollState();
}
}, [pathname, scrollRef, updateScrollState]);

return null;
}

const DetailTab = memo(function DetailTab({
tab,
icon: Icon,
Expand Down
84 changes: 35 additions & 49 deletions apps/dashboard/src/components/pulls/review/review-page.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
FileIcon,
GitPullRequestIcon,
PanelLeftIcon,
SearchIcon,
} from "@diffkit/icons";
import { FileIcon, GitPullRequestIcon, PanelLeftIcon } from "@diffkit/icons";
import {
Drawer,
DrawerContent,
Expand All @@ -30,13 +25,13 @@ import {
memo,
Suspense,
useCallback,
useDeferredValue,
useMemo,
useRef,
useState,
useSyncExternalStore,
} from "react";
import { getPrStateConfig } from "#/components/pulls/detail/pull-detail-header";
import { FileSearchCard } from "#/components/shared/file-search-card";
import { getPullFiles, submitPullReview } from "#/lib/github.functions";
import {
githubPullFileSummariesQueryOptions,
Expand Down Expand Up @@ -66,11 +61,10 @@ import {
import { ReviewSubmitPopover } from "./review-submit-popover";
import type {
ActiveCommentForm,
FileTreeNode,
PendingComment,
ReviewEvent,
} from "./review-types";
import { buildFileTree } from "./review-utils";
import { buildFileTree, encodeFileId } from "./review-utils";

const routeApi = getRouteApi("/_protected/$owner/$repo/review/$pullId");
const PULL_FILES_PAGE_SIZE = 25;
Expand Down Expand Up @@ -736,54 +730,46 @@ const ReviewSidebar = memo(function ReviewSidebar({
activeFileStore: ActiveFileStore;
onFileClick: (path: string) => void;
}) {
const [fileFilter, setFileFilter] = useState("");
const deferredFileFilter = useDeferredValue(fileFilter);

const fileTree = useMemo(() => buildFileTree(sidebarFiles), [sidebarFiles]);

const filteredTree = useMemo(() => {
if (!deferredFileFilter) return fileTree;
const lower = deferredFileFilter.toLowerCase();

function filterNodes(nodes: FileTreeNode[]): FileTreeNode[] {
return nodes
.map((node) => {
if (node.type === "file") {
return node.name.toLowerCase().includes(lower) ? node : null;
}

const filteredChildren = filterNodes(node.children);
return filteredChildren.length > 0
? { ...node, children: filteredChildren }
: null;
})
.filter(Boolean) as FileTreeNode[];
}
const searchEntries = useMemo(
() =>
sidebarFiles.map((f) => ({
path: f.filename,
name: f.filename.split("/").pop() ?? f.filename,
type: "file" as const,
})),
[sidebarFiles],
);

return filterNodes(fileTree);
}, [deferredFileFilter, fileTree]);
const activeFile = useSyncExternalStore(
activeFileStore.subscribe,
activeFileStore.get,
);

const handleSearchSelect = useCallback(
(entry: { path: string }) => {
onFileClick(entry.path);
const hash = `#${encodeFileId(entry.path)}`;
if (window.location.hash !== hash) {
history.replaceState(null, "", hash);
}
},
[onFileClick],
);

return (
<div className="flex h-full flex-col">
<div className="px-3 py-2">
<div className="relative flex items-center rounded-md border bg-surface-0 px-2.5 py-1.5">
<SearchIcon
size={13}
strokeWidth={2}
className="shrink-0 text-muted-foreground"
/>
<input
type="text"
placeholder="Filter files..."
value={fileFilter}
onChange={(event) => setFileFilter(event.target.value)}
className="ml-2 w-full bg-transparent text-xs outline-none placeholder:text-muted-foreground"
/>
</div>
</div>
<FileSearchCard
entries={searchEntries}
onSelect={handleSearchSelect}
activePath={activeFile ?? undefined}
placeholder="Search files..."
shortcutKey="f"
/>

<div className="flex-1 overflow-auto py-1">
{filteredTree.map((node) => (
{fileTree.map((node) => (
<ReviewFileTreeNode
key={node.path}
node={node}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function BranchSelector({
size="sm"
onMouseEnter={prefetchBranches}
onFocus={prefetchBranches}
className="max-w-[220px]"
className="max-w-[220px] border border-border dark:border-transparent"
>
<GitPullRequestIcon size={14} />
<span className="truncate">{currentRef}</span>
Expand Down
Loading
Loading