fix(app): avoid unnecessary git diff header auto-scroll on collapse
This commit is contained in:
@@ -7,6 +7,7 @@ import {
|
|||||||
Pressable,
|
Pressable,
|
||||||
FlatList,
|
FlatList,
|
||||||
Platform,
|
Platform,
|
||||||
|
type LayoutChangeEvent,
|
||||||
type NativeSyntheticEvent,
|
type NativeSyntheticEvent,
|
||||||
type NativeScrollEvent,
|
type NativeScrollEvent,
|
||||||
} from "react-native";
|
} from "react-native";
|
||||||
@@ -40,6 +41,7 @@ import { useHorizontalScrollOptional } from "@/contexts/horizontal-scroll-contex
|
|||||||
import { useExplorerSidebarAnimation } from "@/contexts/explorer-sidebar-animation-context";
|
import { useExplorerSidebarAnimation } from "@/contexts/explorer-sidebar-animation-context";
|
||||||
import { Fonts } from "@/constants/theme";
|
import { Fonts } from "@/constants/theme";
|
||||||
import { getNowMs, isPerfLoggingEnabled, perfLog } from "@/utils/perf";
|
import { getNowMs, isPerfLoggingEnabled, perfLog } from "@/utils/perf";
|
||||||
|
import { shouldAnchorHeaderBeforeCollapse } from "@/utils/git-diff-scroll";
|
||||||
import {
|
import {
|
||||||
DropdownMenu,
|
DropdownMenu,
|
||||||
DropdownMenuContent,
|
DropdownMenuContent,
|
||||||
@@ -518,6 +520,8 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
|||||||
const [isManualRefresh, setIsManualRefresh] = useState(false);
|
const [isManualRefresh, setIsManualRefresh] = useState(false);
|
||||||
const [expandedByPath, setExpandedByPath] = useState<Record<string, boolean>>({});
|
const [expandedByPath, setExpandedByPath] = useState<Record<string, boolean>>({});
|
||||||
const diffListRef = useRef<FlatList<DiffFlatItem>>(null);
|
const diffListRef = useRef<FlatList<DiffFlatItem>>(null);
|
||||||
|
const diffListScrollOffsetRef = useRef(0);
|
||||||
|
const diffListViewportHeightRef = useRef(0);
|
||||||
const headerHeightByPathRef = useRef<Record<string, number>>({});
|
const headerHeightByPathRef = useRef<Record<string, number>>({});
|
||||||
const bodyHeightByPathRef = useRef<Record<string, number>>({});
|
const bodyHeightByPathRef = useRef<Record<string, number>>({});
|
||||||
const defaultHeaderHeightRef = useRef<number>(44);
|
const defaultHeaderHeightRef = useRef<number>(44);
|
||||||
@@ -622,6 +626,18 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
|||||||
bodyHeightByPathRef.current[path] = height;
|
bodyHeightByPathRef.current[path] = height;
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
const handleDiffListScroll = useCallback((event: NativeSyntheticEvent<NativeScrollEvent>) => {
|
||||||
|
diffListScrollOffsetRef.current = event.nativeEvent.contentOffset.y;
|
||||||
|
}, []);
|
||||||
|
|
||||||
|
const handleDiffListLayout = useCallback((event: LayoutChangeEvent) => {
|
||||||
|
const height = event.nativeEvent.layout.height;
|
||||||
|
if (!Number.isFinite(height) || height <= 0) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
diffListViewportHeightRef.current = height;
|
||||||
|
}, []);
|
||||||
|
|
||||||
const computeHeaderOffset = useCallback(
|
const computeHeaderOffset = useCallback(
|
||||||
(path: string): number => {
|
(path: string): number => {
|
||||||
const defaultHeaderHeight = defaultHeaderHeightRef.current;
|
const defaultHeaderHeight = defaultHeaderHeightRef.current;
|
||||||
@@ -645,9 +661,19 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
|||||||
const isCurrentlyExpanded = expandedByPath[path] ?? false;
|
const isCurrentlyExpanded = expandedByPath[path] ?? false;
|
||||||
const nextExpanded = !isCurrentlyExpanded;
|
const nextExpanded = !isCurrentlyExpanded;
|
||||||
const targetOffset = isCurrentlyExpanded ? computeHeaderOffset(path) : null;
|
const targetOffset = isCurrentlyExpanded ? computeHeaderOffset(path) : null;
|
||||||
|
const headerHeight = headerHeightByPathRef.current[path] ?? defaultHeaderHeightRef.current;
|
||||||
|
const shouldAnchor =
|
||||||
|
isCurrentlyExpanded &&
|
||||||
|
targetOffset !== null &&
|
||||||
|
shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset: targetOffset,
|
||||||
|
headerHeight,
|
||||||
|
viewportOffset: diffListScrollOffsetRef.current,
|
||||||
|
viewportHeight: diffListViewportHeightRef.current,
|
||||||
|
});
|
||||||
|
|
||||||
// Anchor to the clicked header before collapsing so visual context is preserved.
|
// Anchor to the clicked header before collapsing so visual context is preserved.
|
||||||
if (isCurrentlyExpanded && targetOffset !== null) {
|
if (shouldAnchor && targetOffset !== null) {
|
||||||
diffListRef.current?.scrollToOffset({
|
diffListRef.current?.scrollToOffset({
|
||||||
offset: targetOffset,
|
offset: targetOffset,
|
||||||
animated: false,
|
animated: false,
|
||||||
@@ -931,6 +957,9 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
|||||||
style={styles.scrollView}
|
style={styles.scrollView}
|
||||||
contentContainerStyle={styles.contentContainer}
|
contentContainerStyle={styles.contentContainer}
|
||||||
testID="git-diff-scroll"
|
testID="git-diff-scroll"
|
||||||
|
onLayout={handleDiffListLayout}
|
||||||
|
onScroll={handleDiffListScroll}
|
||||||
|
scrollEventThrottle={16}
|
||||||
onRefresh={handleRefresh}
|
onRefresh={handleRefresh}
|
||||||
refreshing={isManualRefresh && isDiffFetching}
|
refreshing={isManualRefresh && isDiffFetching}
|
||||||
// Mixed-height rows (header + potentially very large body) are prone to clipping artifacts.
|
// Mixed-height rows (header + potentially very large body) are prone to clipping artifacts.
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { shouldAnchorHeaderBeforeCollapse } from "./git-diff-scroll";
|
||||||
|
|
||||||
|
describe("shouldAnchorHeaderBeforeCollapse", () => {
|
||||||
|
it("skips anchor when header is fully visible in viewport", () => {
|
||||||
|
expect(
|
||||||
|
shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset: 120,
|
||||||
|
headerHeight: 44,
|
||||||
|
viewportOffset: 80,
|
||||||
|
viewportHeight: 400,
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips anchor when header is partially visible in viewport", () => {
|
||||||
|
expect(
|
||||||
|
shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset: 60,
|
||||||
|
headerHeight: 44,
|
||||||
|
viewportOffset: 80,
|
||||||
|
viewportHeight: 300,
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("anchors when header is above viewport", () => {
|
||||||
|
expect(
|
||||||
|
shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset: 200,
|
||||||
|
headerHeight: 44,
|
||||||
|
viewportOffset: 500,
|
||||||
|
viewportHeight: 300,
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("anchors when header is below viewport", () => {
|
||||||
|
expect(
|
||||||
|
shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset: 600,
|
||||||
|
headerHeight: 44,
|
||||||
|
viewportOffset: 100,
|
||||||
|
viewportHeight: 300,
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("anchors when viewport metrics are unavailable", () => {
|
||||||
|
expect(
|
||||||
|
shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset: 0,
|
||||||
|
headerHeight: 44,
|
||||||
|
viewportOffset: 0,
|
||||||
|
viewportHeight: 0,
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,36 @@
|
|||||||
|
interface AnchorVisibilityInput {
|
||||||
|
headerOffset: number;
|
||||||
|
headerHeight: number;
|
||||||
|
viewportOffset: number;
|
||||||
|
viewportHeight: number;
|
||||||
|
edgeThreshold?: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function shouldAnchorHeaderBeforeCollapse({
|
||||||
|
headerOffset,
|
||||||
|
headerHeight,
|
||||||
|
viewportOffset,
|
||||||
|
viewportHeight,
|
||||||
|
edgeThreshold = 1,
|
||||||
|
}: AnchorVisibilityInput): boolean {
|
||||||
|
if (
|
||||||
|
!Number.isFinite(headerOffset) ||
|
||||||
|
!Number.isFinite(headerHeight) ||
|
||||||
|
!Number.isFinite(viewportOffset) ||
|
||||||
|
!Number.isFinite(viewportHeight) ||
|
||||||
|
viewportHeight <= 0
|
||||||
|
) {
|
||||||
|
// Preserve current behavior when metrics are unavailable.
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
const clampedThreshold = Math.max(0, edgeThreshold);
|
||||||
|
const clampedHeaderHeight = Math.max(0, headerHeight);
|
||||||
|
const headerStart = headerOffset;
|
||||||
|
const headerEnd = headerOffset + clampedHeaderHeight;
|
||||||
|
const viewportStart = viewportOffset + clampedThreshold;
|
||||||
|
const viewportEnd = viewportOffset + viewportHeight - clampedThreshold;
|
||||||
|
const headerVisible = headerEnd > viewportStart && headerStart < viewportEnd;
|
||||||
|
|
||||||
|
return !headerVisible;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user