fix(review): force-load diffs up to selected file for accurate scroll (#403)

This commit is contained in:
João Salavisa
2026-03-10 20:56:10 +00:00
committed by GitHub
parent 42cdb56889
commit 76ad8d17a2
2 changed files with 28 additions and 26 deletions
+6 -19
View File
@@ -132,7 +132,6 @@ function computeCommentCounts(
type ReviewDialogHandlerOptions = {
allFiles: ReviewFile[];
fileRefs: Map<string, React.RefObject<HTMLDivElement | null>>;
markReviewed: (path: string, hash: string) => void;
markUnreviewed: (path: string) => void;
onSendComments: ReviewDialogProps["onSendComments"];
@@ -141,15 +140,7 @@ type ReviewDialogHandlerOptions = {
};
function useReviewDialogHandlers(opts: ReviewDialogHandlerOptions) {
const {
allFiles,
fileRefs,
markReviewed,
markUnreviewed,
onSendComments,
onOpenChange,
sessionId,
} = opts;
const { allFiles, markReviewed, markUnreviewed, onSendComments, onOpenChange, sessionId } = opts;
const { discard } = useGitOperations(sessionId);
const { toast } = useToast();
@@ -159,14 +150,11 @@ function useReviewDialogHandlers(opts: ReviewDialogHandlerOptions) {
window.dispatchEvent(new CustomEvent("diff-view-mode-change", { detail: mode }));
}, []);
const handleSelectFile = useCallback(
(path: string, setSelectedFile: (p: string) => void) => {
setSelectedFile(path);
const ref = fileRefs.get(path);
if (ref?.current) ref.current.scrollIntoView({ behavior: "smooth", block: "start" });
},
[fileRefs],
);
const handleSelectFile = useCallback((path: string, setSelectedFile: (p: string) => void) => {
setSelectedFile(path);
// Note: scrolling is now handled by FileDiffSection when isSelected changes
// This ensures proper timing after the section expands
}, []);
const handleToggleReviewed = useCallback(
(path: string, reviewed: boolean) => {
@@ -280,7 +268,6 @@ function useReviewDialogState(props: ReviewDialogProps) {
const handlers = useReviewDialogHandlers({
allFiles,
fileRefs,
markReviewed,
markUnreviewed,
onSendComments,
@@ -47,9 +47,11 @@ export const ReviewDiffList = memo(function ReviewDiffList({
fileRefs,
}: ReviewDiffListProps) {
const scrollContainerRef = useRef<HTMLDivElement | null>(null);
// Find index of selected file - we need to force-load all files up to it
const selectedIndex = selectedFile ? files.findIndex((f) => f.path === selectedFile) : -1;
return (
<div ref={scrollContainerRef} className="overflow-y-auto h-full">
{files.map((file) => (
{files.map((file, index) => (
<FileDiffSection
key={file.path}
file={file}
@@ -59,6 +61,7 @@ export const ReviewDiffList = memo(function ReviewDiffList({
autoMarkOnScroll={autoMarkOnScroll}
wordWrap={wordWrap}
isSelected={selectedFile === file.path}
forceLoad={selectedIndex >= 0 && index <= selectedIndex}
onToggleReviewed={onToggleReviewed}
onDiscard={onDiscard}
onOpenFile={onOpenFile}
@@ -78,6 +81,7 @@ type FileDiffSectionProps = {
autoMarkOnScroll: boolean;
wordWrap: boolean;
isSelected?: boolean;
forceLoad?: boolean;
onToggleReviewed: (path: string, reviewed: boolean) => void;
onDiscard: (path: string) => void;
onOpenFile?: (filePath: string) => void;
@@ -274,6 +278,7 @@ function FileDiffSection({
autoMarkOnScroll,
wordWrap,
isSelected,
forceLoad,
onToggleReviewed,
onDiscard,
onOpenFile,
@@ -282,12 +287,22 @@ function FileDiffSection({
}: FileDiffSectionProps) {
const [collapsed, setCollapsed] = useState(false);
const handleToggleCollapse = useCallback(() => setCollapsed((v) => !v), []);
// Force-expand when the user explicitly navigates to this file
useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect -- syncing collapsed state from parent selection prop
if (isSelected) setCollapsed(false);
}, [isSelected]);
const { isVisible, sentinelRef } = useLazyVisible(scrollContainer);
// Force load when: visible via intersection observer, or forceLoad is true (all files up to selected)
const shouldRenderContent = isVisible || forceLoad;
useEffect(() => {
if (isSelected) {
// eslint-disable-next-line react-hooks/set-state-in-effect -- syncing collapsed state from parent selection prop
setCollapsed(false);
// Wait for all diffs above to fully render before scrolling
// Double rAF ensures layout is complete after React commit
requestAnimationFrame(() => {
requestAnimationFrame(() => {
sectionRef?.current?.scrollIntoView({ behavior: "smooth", block: "start" });
});
});
}
}, [isSelected, sectionRef]);
const scrollSentinelRef = useAutoMarkOnScroll({
autoMarkOnScroll,
isReviewed,
@@ -327,7 +342,7 @@ function FileDiffSection({
/>
<div ref={sentinelRef} />
{!collapsed &&
(isVisible && file.diff ? (
(shouldRenderContent && file.diff ? (
<div className="">
<FileDiffViewer
filePath={file.path}