search

allenai / pdf-component-library

44 stars 4 forks source link

Fix scroll zoom bug #160

Closed ericmarsh995 closed 2 years ago

ericmarsh995 commented 2 years ago

Description

Fix known bug in react-pdf where zooming in/out causes the current scroll to jump to vastly different pages. This is most notable when at the bottom of the document

Reviewer Instructions

Semantic Reader has an issue where the scroll observers arent loaded correctly, but is fixed when the TOC is opened. To fix this, I reset scroll observers on pdfDocProxy change inside of document wrapper.

Note: this fix is reliant on visiblePageRatios which we will be tweaking in the future to be more consistent

Testing Plan

Zoom in and out on first page, last page, middle page. Should zoom into the current page.

Output / Screenshots

Before

https://user-images.githubusercontent.com/108751850/182953931-8993be71-e19e-4f31-bb76-012254dbf3c4.mov

After

https://user-images.githubusercontent.com/108751850/182953939-e525de0b-7979-458e-8e3b-189b7829793e.mov

A11y

N/A

huytr1995 commented 2 years ago

@ericmarsh995 since u created this new function in ScrollContext remember to add it in unit test.

huytr1995 commented 2 years ago

@ericmarsh995 can u also test this in Prod to make sure. Because if we dont test in prod and we found any bug we have to do the whole publish npm again which can be quite a hassle.

ericmarsh995 commented 2 years ago

closing in favor of a better solution