From bbcb7a199dff0ac130151f82b565cc4dfa9ce5cd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 1 Sep 2026 03:28:53 -0700 Subject: [PATCH] fix(task-page): restore the STA-5949 GitHub list scroll restore The split left this logic inline in use-task-page-github-detail.ts at its pre-STA-5949 shape: a 5s deadline whose expiry overwrote the remembered offset with the committed 0 -- the permanent-loss bug the extracted module's header says was removed -- observing only the children rather than the container, and with no MutationObserver, so late-mounting rows never retriggered a retry. The list scroll handler also blanket-returned while a restore was pending instead of classifying echoes, dropping a user's scroll during an unreachable restore. Promotes the module to a live path and wires the hook and scroll handler to it, which also gives the ten behavioral cases in the scroll-restore suite something live to assert against. --- .../src/components/TaskPageGitHubList.tsx | 19 +++-- ...sk-page-github-list-scroll-restore.test.ts | 2 +- ...> task-page-github-list-scroll-restore.ts} | 0 .../github/github-work-item-table.tsx | 2 +- .../hooks/use-task-page-github-list-state.ts | 2 +- .../components/use-task-page-github-detail.ts | 73 +++++-------------- .../use-task-page-github-list-state.ts | 4 + 7 files changed, 39 insertions(+), 63 deletions(-) rename src/renderer/src/components/{task-page/github/github-list-scroll-restore.ts => task-page-github-list-scroll-restore.ts} (100%) diff --git a/src/renderer/src/components/TaskPageGitHubList.tsx b/src/renderer/src/components/TaskPageGitHubList.tsx index aa53374ffa7..569db75502a 100644 --- a/src/renderer/src/components/TaskPageGitHubList.tsx +++ b/src/renderer/src/components/TaskPageGitHubList.tsx @@ -13,6 +13,7 @@ import { Button } from '@/components/ui/button' import { LoaderCircle } from 'lucide-react' import { TaskPageGitHubRows } from './TaskPageGitHubRows' import { PaginationBar } from './TaskPagePaginationBar' +import { supersedeGitHubListScrollRestore } from './task-page-github-list-scroll-restore' export function TaskPageGitHubList({ model }: { @@ -33,6 +34,7 @@ export function TaskPageGitHubList({ githubListScrollRef, githubListScrollTopRef, pendingGithubScrollRestoreRef, + githubListRestoreWriteRef, loadingTargetPage, taskListPositionRef, perRepoSourceState, @@ -61,14 +63,21 @@ export function TaskPageGitHubList({ }} onScroll={(event) => { const state = useAppStore.getState() - if ( - state.activeView !== 'tasks' || - state.taskPageData.openGitHubWorkItem || - pendingGithubScrollRestoreRef.current !== null - ) { + if (state.activeView !== 'tasks' || state.taskPageData.openGitHubWorkItem) { return } const scrollTop = event.currentTarget.scrollTop + // Why: a restore's own write must not be saved as the user's position, but a real + // user scroll has to supersede a pending restore or saving stays suppressed. + if ( + !supersedeGitHubListScrollRestore({ + scrollTop, + pendingRestoreRef: pendingGithubScrollRestoreRef, + restoreWriteRef: githubListRestoreWriteRef + }) + ) { + return + } githubListScrollTopRef.current = scrollTop taskListPositionRef.current = { contextKey: githubResumeContextKey, diff --git a/src/renderer/src/components/task-page-github-list-scroll-restore.test.ts b/src/renderer/src/components/task-page-github-list-scroll-restore.test.ts index 9f557a10b7d..785f503436f 100644 --- a/src/renderer/src/components/task-page-github-list-scroll-restore.test.ts +++ b/src/renderer/src/components/task-page-github-list-scroll-restore.test.ts @@ -6,7 +6,7 @@ import { startGitHubListScrollRestore, supersedeGitHubListScrollRestore, type GitHubListRestoreWrite -} from './task-page/github/github-list-scroll-restore' +} from './task-page-github-list-scroll-restore' type FakeResizeObserver = { targets: Set diff --git a/src/renderer/src/components/task-page/github/github-list-scroll-restore.ts b/src/renderer/src/components/task-page-github-list-scroll-restore.ts similarity index 100% rename from src/renderer/src/components/task-page/github/github-list-scroll-restore.ts rename to src/renderer/src/components/task-page-github-list-scroll-restore.ts diff --git a/src/renderer/src/components/task-page/github/github-work-item-table.tsx b/src/renderer/src/components/task-page/github/github-work-item-table.tsx index 0f6e1ab6e8e..c19a2d34201 100644 --- a/src/renderer/src/components/task-page/github/github-work-item-table.tsx +++ b/src/renderer/src/components/task-page/github/github-work-item-table.tsx @@ -27,7 +27,7 @@ import type { TaskPageGitHubWorkItemMutationRunner } from './github-work-item-mu import { supersedeGitHubListScrollRestore, type GitHubListRestoreWrite -} from './github-list-scroll-restore' +} from '@/components/task-page-github-list-scroll-restore' import { GithubWorkItemRows } from './github-work-item-rows' import { PaginationBar } from '../pagination/pagination-bar' diff --git a/src/renderer/src/components/task-page/hooks/use-task-page-github-list-state.ts b/src/renderer/src/components/task-page/hooks/use-task-page-github-list-state.ts index d85a7e3d0b4..19815e9da3f 100644 --- a/src/renderer/src/components/task-page/hooks/use-task-page-github-list-state.ts +++ b/src/renderer/src/components/task-page/hooks/use-task-page-github-list-state.ts @@ -9,7 +9,7 @@ import { sortWorkItemsByNumber } from '../../../../../shared/work-items' import type { GitHubWorkItem } from '../../../../../shared/github/work-item-types' import type { Repo } from '../../../../../shared/repo-types' import type { TaskViewPresetId } from '../../../../../shared/ui-chrome-types' -import type { GitHubListRestoreWrite } from '@/components/task-page/github/github-list-scroll-restore' +import type { GitHubListRestoreWrite } from '@/components/task-page-github-list-scroll-restore' import type { AppState } from '@/store/types' export function useTaskPageGitHubListState({ diff --git a/src/renderer/src/components/use-task-page-github-detail.ts b/src/renderer/src/components/use-task-page-github-detail.ts index 22e0b2a2f85..b2c61f64bde 100644 --- a/src/renderer/src/components/use-task-page-github-detail.ts +++ b/src/renderer/src/components/use-task-page-github-detail.ts @@ -11,6 +11,7 @@ import { import type { GitHubWorkItem } from '../../../shared/github/work-item-types' import type { GitLabWorkItem } from '../../../shared/gitlab-types' import { getTaskPageRepoCacheInput, getTaskPageRepoSourceContext } from './task-page-source-context' +import { startGitHubListScrollRestore } from './task-page-github-list-scroll-restore' function getTaskPageScrollTop( scrollRef: React.RefObject, @@ -38,6 +39,7 @@ export function useTaskPageGitHubDetail(model: TaskPageGitHubListStateModel) { githubListScrollRef, githubListScrollTopRef, pendingGithubScrollRestoreRef, + githubListRestoreWriteRef, taskListPositionRef } = model // Why: the dialog's "Use" button routes through the same direct-launch flow as the row-level "Use" CTA so behavior is consistent regardless of entry point. @@ -73,67 +75,28 @@ export function useTaskPageGitHubDetail(model: TaskPageGitHubListStateModel) { ? (cachedDialogWorkItem ?? githubTaskDrawerWorkItem) : null useLayoutEffect(() => { - const scrollTop = pendingGithubScrollRestoreRef.current - const scrollElement = githubListScrollRef.current - if (scrollTop === null || !scrollElement || !pages[currentPage]) { + const target = pendingGithubScrollRestoreRef.current + if (target === null || !githubListScrollRef.current || !pages[currentPage]) { return } - let frame: number | null = null - let timeout: number | null = null - let observer: ResizeObserver | null = null - const clearScheduledRestore = (): void => { - if (frame !== null) { - window.cancelAnimationFrame(frame) - frame = null - } - if (timeout !== null) { - window.clearTimeout(timeout) - timeout = null - } - observer?.disconnect() - } - const restore = (): void => { - const committedScrollElement = githubListScrollRef.current - if (!committedScrollElement || pendingGithubScrollRestoreRef.current !== scrollTop) { - return - } - committedScrollElement.scrollTop = scrollTop - githubListScrollTopRef.current = scrollTop - taskListPositionRef.current = { - contextKey: githubResumeContextKey, - page: currentPage, - scrollTop - } - if (Math.abs(committedScrollElement.scrollTop - scrollTop) < 1) { - pendingGithubScrollRestoreRef.current = null - clearScheduledRestore() - } - } - observer = new ResizeObserver(restore) - for (const child of scrollElement.children) { - observer.observe(child) - } - restore() - if (pendingGithubScrollRestoreRef.current === scrollTop) { - frame = window.requestAnimationFrame(restore) - timeout = window.setTimeout(() => { - if (pendingGithubScrollRestoreRef.current === scrollTop) { - const committedScrollTop = getTaskPageScrollTop(githubListScrollRef, 0) - githubListScrollTopRef.current = committedScrollTop - taskListPositionRef.current = { - contextKey: githubResumeContextKey, - page: currentPage, - scrollTop: committedScrollTop - } - pendingGithubScrollRestoreRef.current = null + return startGitHubListScrollRestore({ + target, + scrollElementRef: githubListScrollRef, + pendingRestoreRef: pendingGithubScrollRestoreRef, + restoreWriteRef: githubListRestoreWriteRef, + onScrollTopApplied: (scrollTop) => { + githubListScrollTopRef.current = scrollTop + taskListPositionRef.current = { + contextKey: githubResumeContextKey, + page: currentPage, + scrollTop } - clearScheduledRestore() - }, 5_000) - } - return clearScheduledRestore + } + }) }, [ currentPage, dialogWorkItem, + githubListRestoreWriteRef, githubListScrollRef, githubListScrollTopRef, githubResumeContextKey, diff --git a/src/renderer/src/components/use-task-page-github-list-state.ts b/src/renderer/src/components/use-task-page-github-list-state.ts index 7a5e6b31253..03ff48702c7 100644 --- a/src/renderer/src/components/use-task-page-github-list-state.ts +++ b/src/renderer/src/components/use-task-page-github-list-state.ts @@ -10,6 +10,7 @@ import { } from '@/components/task-page-github-resume-cache' import { sortWorkItemsByNumber } from '../../../shared/work-items' import { useAppStore } from '@/store' +import type { GitHubListRestoreWrite } from './task-page-github-list-scroll-restore' import { getTaskPageRepoSourceContext } from './task-page-source-context' export function useTaskPageGitHubListState(model: TaskPageProviderStateModel) { const { @@ -99,6 +100,7 @@ export function useTaskPageGitHubListState(model: TaskPageProviderStateModel) { const githubListScrollRef = useRef(null) const githubListScrollTopRef = useRef(0) const pendingGithubScrollRestoreRef = useRef(null) + const githubListRestoreWriteRef = useRef(null) const [paginationLoading, setPaginationLoading] = useState(false) const [loadingTargetPage, setLoadingTargetPage] = useState(null) const [countedTotalPages, setCountedTotalPages] = useState(null) @@ -218,6 +220,7 @@ export function useTaskPageGitHubListState(model: TaskPageProviderStateModel) { githubListScrollRef: typeof githubListScrollRef githubListScrollTopRef: typeof githubListScrollTopRef pendingGithubScrollRestoreRef: typeof pendingGithubScrollRestoreRef + githubListRestoreWriteRef: typeof githubListRestoreWriteRef paginationLoading: typeof paginationLoading setPaginationLoading: typeof setPaginationLoading loadingTargetPage: typeof loadingTargetPage @@ -275,6 +278,7 @@ export function useTaskPageGitHubListState(model: TaskPageProviderStateModel) { nextModel.githubListScrollRef = githubListScrollRef nextModel.githubListScrollTopRef = githubListScrollTopRef nextModel.pendingGithubScrollRestoreRef = pendingGithubScrollRestoreRef + nextModel.githubListRestoreWriteRef = githubListRestoreWriteRef nextModel.paginationLoading = paginationLoading nextModel.setPaginationLoading = setPaginationLoading nextModel.loadingTargetPage = loadingTargetPage