revert(ui): move detail-page scroll handling to its own branch

Reverts 40153bd43. Scroll position carrying over between pages is stock React
Router behaviour rather than something this branch introduced, so the fix does
not belong in an artwork PR that is otherwise ready for review.

The work continues on fix/scroll-restoration, off master, where it grew into
per-route restoration: a new page starts at the top and going back returns to
where you were, which the scroll-to-top could not do.
This commit is contained in:
Deluan 2026-08-04 09:39:00 -04:00
parent 0c06d3fcef
commit c1e4f1c9ae
5 changed files with 2 additions and 65 deletions

View File

@ -10,7 +10,7 @@ import { makeStyles } from '@material-ui/core/styles'
import AlbumSongs from './AlbumSongs'
import AlbumDetails from './AlbumDetails'
import AlbumActions from './AlbumActions'
import { useResourceRefresh, useScrollToTop, Title } from '../common'
import { useResourceRefresh, Title } from '../common'
const useStyles = makeStyles(
(theme) => ({
@ -28,7 +28,6 @@ const AlbumShowLayout = (props) => {
const { record } = context
const classes = useStyles()
useResourceRefresh('album', 'song')
useScrollToTop(record?.id)
return (
<>

View File

@ -13,12 +13,7 @@ import subsonic from '../subsonic'
import AlbumGridView from '../album/AlbumGridView'
import MobileArtistDetails from './MobileArtistDetails'
import DesktopArtistDetails from './DesktopArtistDetails'
import {
useAlbumsPerPage,
useResourceRefresh,
useScrollToTop,
Title,
} from '../common/index.js'
import { useAlbumsPerPage, useResourceRefresh, Title } from '../common/index.js'
import ArtistActions from './ArtistActions'
import { makeStyles } from '@material-ui/core'
@ -90,7 +85,6 @@ const ArtistShowLayout = (props) => {
const [, perPageOptions] = useAlbumsPerPage(width)
const classes = useStyles()
useResourceRefresh('artist', 'album')
useScrollToTop(record?.id)
const maxPerPage = 90
let perPage = 0

View File

@ -28,7 +28,6 @@ export * from './useAlbumsPerPage'
export * from './useGetHandleArtistClick'
export * from './useInterval'
export * from './useResourceRefresh'
export * from './useScrollToTop'
export * from './useRefreshOnEvents'
export * from './useToggleLove'
export * from './useTraceUpdate'

View File

@ -1,11 +0,0 @@
import { useEffect } from 'react'
// React Router keeps the previous page's scroll offset, so a detail page opened from a scrolled
// list starts mid-page. Keyed on the record id so detail-to-detail navigation resets too.
export const useScrollToTop = (key) => {
useEffect(() => {
if (key) {
window.scrollTo({ top: 0 })
}
}, [key])
}

View File

@ -1,44 +0,0 @@
import { renderHook } from '@testing-library/react-hooks'
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { useScrollToTop } from './useScrollToTop'
describe('useScrollToTop', () => {
beforeEach(() => {
window.scrollTo = vi.fn()
})
it('scrolls to the top on mount', () => {
renderHook(() => useScrollToTop('al-1'))
expect(window.scrollTo).toHaveBeenCalledWith({ top: 0 })
})
// Detail-to-detail navigation reuses the component, so only the key change signals a new page.
it('scrolls again when the key changes', () => {
const { rerender } = renderHook(({ id }) => useScrollToTop(id), {
initialProps: { id: 'al-1' },
})
expect(window.scrollTo).toHaveBeenCalledTimes(1)
rerender({ id: 'al-2' })
expect(window.scrollTo).toHaveBeenCalledTimes(2)
})
it('does not scroll again on a re-render with the same key', () => {
const { rerender } = renderHook(({ id }) => useScrollToTop(id), {
initialProps: { id: 'al-1' },
})
rerender({ id: 'al-1' })
expect(window.scrollTo).toHaveBeenCalledTimes(1)
})
// The record arrives after the first render, so the key starts undefined.
it('waits for a key rather than scrolling on an empty record', () => {
const { rerender } = renderHook(({ id }) => useScrollToTop(id), {
initialProps: { id: undefined },
})
expect(window.scrollTo).not.toHaveBeenCalled()
rerender({ id: 'al-1' })
expect(window.scrollTo).toHaveBeenCalledTimes(1)
})
})