mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
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.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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<Element>
|
||||
|
||||
@@ -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'
|
||||
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<HTMLElement | null>,
|
||||
@@ -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,
|
||||
|
||||
@@ -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<HTMLDivElement>(null)
|
||||
const githubListScrollTopRef = useRef(0)
|
||||
const pendingGithubScrollRestoreRef = useRef<number | null>(null)
|
||||
const githubListRestoreWriteRef = useRef<GitHubListRestoreWrite | null>(null)
|
||||
const [paginationLoading, setPaginationLoading] = useState(false)
|
||||
const [loadingTargetPage, setLoadingTargetPage] = useState<number | null>(null)
|
||||
const [countedTotalPages, setCountedTotalPages] = useState<number | null>(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
|
||||
|
||||
Reference in New Issue
Block a user