Merge pull request #28 from getpaseo/gitif/header-scroll-rule-pr
fix(app): avoid unnecessary git diff header auto-scroll on collapse
This commit is contained in:
@@ -7,6 +7,7 @@ import {
|
||||
Pressable,
|
||||
FlatList,
|
||||
Platform,
|
||||
type LayoutChangeEvent,
|
||||
type NativeSyntheticEvent,
|
||||
type NativeScrollEvent,
|
||||
} from "react-native";
|
||||
@@ -40,6 +41,7 @@ import { useHorizontalScrollOptional } from "@/contexts/horizontal-scroll-contex
|
||||
import { useExplorerSidebarAnimation } from "@/contexts/explorer-sidebar-animation-context";
|
||||
import { Fonts } from "@/constants/theme";
|
||||
import { getNowMs, isPerfLoggingEnabled, perfLog } from "@/utils/perf";
|
||||
import { shouldAnchorHeaderBeforeCollapse } from "@/utils/git-diff-scroll";
|
||||
import {
|
||||
DropdownMenu,
|
||||
DropdownMenuContent,
|
||||
@@ -519,6 +521,8 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
const [isManualRefresh, setIsManualRefresh] = useState(false);
|
||||
const [expandedByPath, setExpandedByPath] = useState<Record<string, boolean>>({});
|
||||
const diffListRef = useRef<FlatList<DiffFlatItem>>(null);
|
||||
const diffListScrollOffsetRef = useRef(0);
|
||||
const diffListViewportHeightRef = useRef(0);
|
||||
const headerHeightByPathRef = useRef<Record<string, number>>({});
|
||||
const bodyHeightByPathRef = useRef<Record<string, number>>({});
|
||||
const defaultHeaderHeightRef = useRef<number>(44);
|
||||
@@ -623,6 +627,18 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
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(
|
||||
(path: string): number => {
|
||||
const defaultHeaderHeight = defaultHeaderHeightRef.current;
|
||||
@@ -646,9 +662,19 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
const isCurrentlyExpanded = expandedByPath[path] ?? false;
|
||||
const nextExpanded = !isCurrentlyExpanded;
|
||||
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.
|
||||
if (isCurrentlyExpanded && targetOffset !== null) {
|
||||
if (shouldAnchor && targetOffset !== null) {
|
||||
diffListRef.current?.scrollToOffset({
|
||||
offset: targetOffset,
|
||||
animated: false,
|
||||
@@ -932,6 +958,9 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
style={styles.scrollView}
|
||||
contentContainerStyle={styles.contentContainer}
|
||||
testID="git-diff-scroll"
|
||||
onLayout={handleDiffListLayout}
|
||||
onScroll={handleDiffListScroll}
|
||||
scrollEventThrottle={16}
|
||||
onRefresh={handleRefresh}
|
||||
refreshing={isManualRefresh && isDiffFetching}
|
||||
// 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