PDF Component Library loose ends

[carolinepaulic]

Legitimate tech debt that needs to be fixed. At this point it's a list of pending good to haves.

DONE :)

Bonus/nice to haves

• [ ] Unit tests for Reader. Components have tests, missing an integration test for the Reader - 3 days
• [ ] Unit tests for rotate.ts - 3 days
• [ ] Add optional callback props to PDF load handlers https://github.com/allenai/pdf-component-library/pull/34#discussion_r714192686. Might be nice to have a callback to handle onError events. - 3 day
• [ ] DocumentWrapper- use more specific props https://github.com/allenai/pdf-component-library/pull/34#discussion_r714192686
• [ ] Look into use of FunctionComponent vs StaticFunctionalComponent
• [ ] Establish import sorting rules for JS files (https://github.com/allenai/pdf-component-library/pull/17#discussion_r679359274)
• [ ] Add PR template using the following as an example- https://github.com/allenai/scholar/blob/main/.github/pull_request_template.md
• [ ] PageRotation: move default/Rotate0 case to top, add comments showing rotation visually
• [ ] Update errorUtils.ts getErrorMessage parameter type and remove line that disables linting
  • https://github.com/allenai/pdf-component-library/pull/43#discussion_r732063623
• [ ] Update JSON formatter to leave EOF newline on save
• [ ] Hot reload for demo app
• [ ] Answer performance related questions based on metrics created in #30640
  • [ ] How does placement of setting outline context variable affect performance? Is there a difference if we set the outline in DocumentWrapper.tsx onPdfLoadSuccess callback vs setting in Outline.tsx
  • [ ] Does O(n^3) bounding box rendering time complexity significantly affect performance?

Done

• [x] Unit tests for new context providers
  • Relevant comments from old PR: https://github.com/allenai/pdf-component-library/pull/17#discussion_r683689566, https://github.com/allenai/pdf-component-library/pull/17#discussion_r683687874
  • New PR: https://github.com/allenai/pdf-component-library/pull/19
• [x] Log message when context setters aren't instantiated
  • Relevant comment from old PR: https://github.com/allenai/pdf-component-library/pull/17#discussion_r683696944
  • New PR: https://github.com/allenai/pdf-component-library/pull/20
• [x] Zoom control: replace child component props with context vars where possible
  • PR: https://github.com/allenai/pdf-component-library/pull/21
• [x] Look into Header ' vs '
  • Solution no longer needed because this line was removed from Header altogether, but I did some research and learned more about when escaping chars is and isn't needed. In this case I think it would have been ok as '.
• [x] Drawer/Outline: move into own component and replace child component props with context vars where possible (contains bugfix for outline not opening)
  • PR: https://github.com/allenai/pdf-component-library/pull/24
• [x] Header: replace child component props with context vars where possible
  • PR: https://github.com/allenai/pdf-component-library/pull/25
• [x] PageWrapper: replace child component props with context vars where possible
  • PR: https://github.com/allenai/pdf-component-library/pull/37
• [x] allenai/scholar#29449
• [x] allenai/scholar#29577
• [x] allenai/scholar#29467
  • https://github.com/allenai/pdf-component-library/pull/17#discussion_r679378238
  • SimpleZoomControl (https://github.com/allenai/pdf-component-library/pull/17#discussion_r679384569),
  • Header (https://github.com/allenai/pdf-component-library/pull/17#discussion_r683670343)
  • Possible solution: https://jotai.pmnd.rs/
• [x] Pull page width computation functions from Overlay and PageWrapper into shared util
• [x] allenai/scholar#29578
• [x] allenai/scholar#29696