Closed yensung closed 2 years ago
Ok, that's all the comments I have for this PR. Just get a few minor items wrapped up and this will be good to go.
This was a tricky bug to fix but I think you did a great job of providing a well thought out solution. Especially since it covers edge cases such as page rotation!
Update: Added several unit test cases into ContextProvider.test.tsx
and scroll.test.ts
.
For whether using className
prop in Outline
and OutlineItem
, I'll leave them to be addressed in this ticket. Currently the expand button and annotations are not implemented in Outline. I think we'll have a more clear sight to it with those components implemented.
Changes are merged in this PR: https://github.com/allenai/pdf-component-library/pull/83
Description
https://github.com/allenai/scholar/issues/29954
The Outline component provided by react-pdf doesn't pass
dest
information to its event handler when it is being clicked. Without the argument, it is hard to scroll a pdf to specific section precisely. This PR addresses the issue by creating customized Outline component and using it for displaying table of contents as well as section redirection. Works include:<Outline>
and<OutlineItem>
components. When any titles are clicked in TOC, thedest
would be passed to handler function so we can use it for redirection.<DocumentContext>
so it can storepdfDocProxy
object from<Document>
and pass it to<Outline>
.Test plan
Manually test. When paper is scaled larger or smaller, it can still scroll to section precisely. When paper is rotated, it also works, but may need to add offsets for showing titles.
Screenshot