Skip to content

Scroll results into view when paginating #1189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 28, 2024
Merged

Scroll results into view when paginating #1189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
21c173e
Scroll into view on paginate
jonkafton Jun 28, 2024
5418577
Scroll top of results to center page
jonkafton Jun 28, 2024
ef27a3e
Move scroll hook to top of results
jonkafton Jun 28, 2024
937b322
Provide scrollIntoView function for tests
jonkafton Jun 28, 2024
2d5f863
Correct ref type
jonkafton Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions frontends/jest-shared-setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ Object.defineProperty(window, "matchMedia", {
})),
})

Element.prototype.scrollIntoView = jest.fn()

/*
* This used to live in ol-ckeditor but we also need it now for NukaCarousel,
* so it's now here so it's available across the board.
Expand Down
15 changes: 12 additions & 3 deletions frontends/mit-open/src/page-components/SearchDisplay/SearchDisplay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo } from "react"
import React, { useMemo, useRef } from "react"
import {
styled,
Pagination,
Expand Down Expand Up @@ -503,6 +503,7 @@ const SearchDisplay: React.FC<SearchDisplayProps> = ({
setSearchParams,
}) => {
const [searchParams] = useSearchParams()
const scrollHook = useRef<HTMLDivElement>(null)
const activeTab =
TABS.find((t) => t.name === searchParams.get("tab")) ??
TABS.find((t) => t.defaultTab) ??
Expand Down Expand Up @@ -650,8 +651,8 @@ const SearchDisplay: React.FC<SearchDisplayProps> = ({
</StyledDrawer>
<MobileSortContainer>{sortDropdown}</MobileSortContainer>
</MobileFilter>

<StyledResultsContainer>
<div ref={scrollHook} />
{isLoading ? (
<PlainList itemSpacing={1.5}>
{Array(PAGE_SIZE)
Expand Down Expand Up @@ -680,7 +681,15 @@ const SearchDisplay: React.FC<SearchDisplayProps> = ({
<Pagination
count={getLastPage(data?.count ?? 0)}
page={page}
onChange={(_, newPage) => setPage(newPage)}
onChange={(_, newPage) => {
setPage(newPage)
setTimeout(() => {
scrollHook.current?.scrollIntoView({
block: "center",
behavior: "smooth",
})
}, 0)
Comment on lines +686 to +691
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki Jun 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity... why wrap this in setTimeout?

Copy link
Contributor Author

@jonkafton jonkafton Jun 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite some investigating, all I can say for sure is that it doesn't work without it. Likely some nuance around the dom element not being available during the old render cycle.

}}
renderItem={(item) => (
<PaginationItem
slots={{
Expand Down