Closed smitar closed 2 years ago
Some problems we have with current structure:
<div class="reader__page-list">
and render all pdf pages under the scroll.ts
finds the <div>
that has its class attribute set to reader__page-list
to scroll. If there are some other Proposal:
<div class="reader__page-list>
in ui/library, so the library users can render all pdf pages simply by using this componentBenefits:
scroll.ts
would target on the component to scroll instead of using document.getElementsByClassName
to get the scrollable target. This will avoid failure when other Works to do:
PageList.tsx
) that renders all pdf pages in ui/library. It will basically encapsulate below lines
https://github.com/allenai/pdf-component-library/blob/166db4502e7ce7d1e2a639a79caacc4daee52d1d/ui/demo/components/Reader.tsx#L62-L77
(Also since pdf data is available on s2airs, and the citation data, scroll function, highlight function are available, I am thinking to drop the demo components under <Overlay>
)prop
scroll.ts
to let them accept a target element to scroll instead of querying it with document.getElementsByClassName
Blocked until we fix the double scroll bar. The PR creates a component to render all pdf pages but it also gonna be the scroll target for scroll functions to work on. However, we are keeping the outer scroll bar of the reader where the scroll target gonna be an element on S2 site, it conflicts with the PR. So, I’ll leave the PR open until Huy solves the double scroll bar, then we revisit it. Also, we may need to change the scroll functions in pdf-component-library if scroll target gonna be an element on S2 site.
No longer needed, we now have scroll context.
A single component that encapsulates Document Wrapper, outline, Page components etc. This might also fix the double scroll issue.