Single PDF rendering component
Closed this issue · 3 comments
A single component that encapsulates Document Wrapper, outline, Page components etc.
This might also fix the double scroll issue.
Some problems we have with current structure:
- Developers who use this library have to define
<div class="reader__page-list">
and render all pdf pages under theelement by themselves. If it is not set properly, they would fail rendering pdf pages. This is not very intuitive and convenient to use. scroll.ts
finds the<div>
that has its class attribute set toreader__page-list
to scroll. If there are some otherthat use the same class name, the scroll function would fail.
Proposal:
- Create a new component that encapsulates
<div class="reader__page-list>
in ui/library, so the library users can render all pdf pages simply by using this component
Benefits:
- Easier and more intuitive for library users to render pdf pages
scroll.ts
would target on the component to scroll instead of usingdocument.getElementsByClassName
to get the scrollable target. This will avoid failure when otheruse the same class name as the scroll target.- Better code structure to maintain and use
Works to do:
- A new React component (planning to call it
PageList.tsx
) that renders all pdf pages in ui/library. It will basically encapsulate below lines
pdf-component-library/ui/demo/components/Reader.tsx
Lines 62 to 77 in 166db45
(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>
) - The component will have a field to receive citation data in
prop
- Update the functions in
scroll.ts
to let them accept a target element to scroll instead of querying it withdocument.getElementsByClassName
- Update unit tests
- Update demo app and S2 code
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.