mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(ui): anchor detail pages to the top when opened from a list
React Router keeps the previous page's scroll offset, so opening an album from a scrolled list started the detail page mid-song-list. Artist pages had the same bug; it just shows less because the artist list is rarely long enough to scroll far. Keyed on the record id rather than mount, so detail-to-detail navigation (an album's artist link) resets too, and so the scroll waits for the record instead of firing against an empty page.
This commit is contained in:
parent
ff010f8db1
commit
40153bd434
@ -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, Title } from '../common'
|
||||
import { useResourceRefresh, useScrollToTop, Title } from '../common'
|
||||
|
||||
const useStyles = makeStyles(
|
||||
(theme) => ({
|
||||
@ -28,6 +28,7 @@ const AlbumShowLayout = (props) => {
|
||||
const { record } = context
|
||||
const classes = useStyles()
|
||||
useResourceRefresh('album', 'song')
|
||||
useScrollToTop(record?.id)
|
||||
|
||||
return (
|
||||
<>
|
||||
|
||||
@ -13,7 +13,12 @@ import subsonic from '../subsonic'
|
||||
import AlbumGridView from '../album/AlbumGridView'
|
||||
import MobileArtistDetails from './MobileArtistDetails'
|
||||
import DesktopArtistDetails from './DesktopArtistDetails'
|
||||
import { useAlbumsPerPage, useResourceRefresh, Title } from '../common/index.js'
|
||||
import {
|
||||
useAlbumsPerPage,
|
||||
useResourceRefresh,
|
||||
useScrollToTop,
|
||||
Title,
|
||||
} from '../common/index.js'
|
||||
import ArtistActions from './ArtistActions'
|
||||
import { makeStyles } from '@material-ui/core'
|
||||
|
||||
@ -85,6 +90,7 @@ const ArtistShowLayout = (props) => {
|
||||
const [, perPageOptions] = useAlbumsPerPage(width)
|
||||
const classes = useStyles()
|
||||
useResourceRefresh('artist', 'album')
|
||||
useScrollToTop(record?.id)
|
||||
|
||||
const maxPerPage = 90
|
||||
let perPage = 0
|
||||
|
||||
@ -28,6 +28,7 @@ export * from './useAlbumsPerPage'
|
||||
export * from './useGetHandleArtistClick'
|
||||
export * from './useInterval'
|
||||
export * from './useResourceRefresh'
|
||||
export * from './useScrollToTop'
|
||||
export * from './useRefreshOnEvents'
|
||||
export * from './useToggleLove'
|
||||
export * from './useTraceUpdate'
|
||||
|
||||
11
ui/src/common/useScrollToTop.jsx
Normal file
11
ui/src/common/useScrollToTop.jsx
Normal file
@ -0,0 +1,11 @@
|
||||
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])
|
||||
}
|
||||
46
ui/src/common/useScrollToTop.test.jsx
Normal file
46
ui/src/common/useScrollToTop.test.jsx
Normal file
@ -0,0 +1,46 @@
|
||||
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 })
|
||||
})
|
||||
|
||||
// Navigating straight from one detail page to another (an album's artist link, say) reuses the
|
||||
// component, so only the key change tells us we are looking at something new.
|
||||
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; scrolling then would
|
||||
// fire before the page has its content and read as a no-op.
|
||||
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)
|
||||
})
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user