documentcloud-frontend icon indicating copy to clipboard operation
documentcloud-frontend copied to clipboard

Refactor Viewer

Open allanlasser opened this issue 1 year ago β€’ 2 comments

This isn't fully where I want it to be, but wanted to get it up for visibilty. Additional things, like notes improvements, can be follow-ups.

Goals are cleaning up code, consolidating redundancies, extracting pure functions, and increased test coverage.

Changes include:

  • Adding ViewerContext that makes it easier to set and access data for variety of sub-components
  • Removes props passing in favor of having components access ViewerContext directly
  • Renaming *Pane into *Layer
  • Consolidating read-only NotesPane with AnnotationsPane to deduplicate rendering logic
  • Consolidating Text and TextPage
  • Loading the PDF in the ViewerContext, instead of the conditionally-rendered PDF
  • Keeping viewer directory as flat as possible. Toolbars are now alongside their layers, e.g. AnnotationLayer and AnnotationToolbar are siblings.
  • Refactors stateless functions into tested utility functions in lib/utils/viewer
  • Adds src/lib/components/viewer/README.md with a high-level overview of how it works (yay documentation)

Still to do:

  • Move zoom and scale state into ViewerContext and mode-conversion logic into src/lib/utils/viewer.ts
  • ViewerContext.activeNote is probably redundant and can be removed. Whether it’s open or not is state local to that note, navigation is handled through link to the #hash, and we can update a note to be open if its hash is in $page.url
  • Consolidate afterNavigate and onHashChange into ViewerContext from src/routes/(app)/documents/[id]-[slug]/+page.svelte and src/routes/embed/documents/[id]-[slug]/+page.svelte
  • Try to consolidate PDF rendering between PDFPage.svelte and Note.svelteβ€”there's a duplication of logic there. The render functions could also be made pure and assume it's operating on a canvas node that's passed as an argument.

allanlasser avatar Oct 11 '24 13:10 allanlasser

Coverage Report

Status Category Percentage Covered / Total
πŸ”΅ Lines 23.65% 7454 / 31514
πŸ”΅ Statements 23.65% 7454 / 31514
πŸ”΅ Functions 41.14% 151 / 367
πŸ”΅ Branches 60.66% 418 / 689
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
src/lib/api/documents.ts 97.96% 88.88% 100% 97.96% 156-157, 174-180, 184-185, 484-486
src/lib/api/notes.ts 84.72% 78.57% 83.33% 84.72% 25-37, 43-60
src/lib/components/embeds/DocumentEmbed.svelte 0% 0% 0% 0% 1-97
src/lib/components/forms/ConfirmRedaction.svelte 0% 0% 0% 0% 1-88
src/lib/components/forms/EditNote.svelte 0% 0% 0% 0% 1-106
src/lib/components/layouts/ContentLayout.svelte 0% 0% 0% 0% 1-59
src/lib/components/layouts/DocumentLayout.svelte 0% 0% 0% 0% 1-121
src/lib/components/layouts/EmbedLayout.svelte 0% 0% 0% 0% 1-109
src/lib/components/viewer/AnnotationLayer.svelte 0% 0% 0% 0% 1-377
src/lib/components/viewer/Note.svelte 0% 0% 0% 0% 1-393
src/lib/components/viewer/NoteTab.svelte 100% 100% 100% 100%
src/lib/components/viewer/Notes.svelte 0% 0% 0% 0% 1-74
src/lib/components/viewer/PDF.svelte 0% 0% 0% 0% 1-478
src/lib/components/viewer/PDFPage.svelte 0% 0% 0% 0% 1-341
src/lib/components/viewer/Page.svelte 0% 0% 0% 0% 1-590
src/lib/components/viewer/PageActions.svelte 0% 0% 0% 0% 1-145
src/lib/components/viewer/Text.svelte 0% 0% 0% 0% 1-68
src/lib/components/viewer/Viewer.svelte 0% 0% 0% 0% 1-101
src/lib/components/viewer/ViewerContext.svelte 0% 0% 0% 0% 1-194
src/lib/components/viewer/Zoom.svelte 0% 0% 0% 0% 1-62
src/lib/load/document.ts 0% 0% 0% 0% 1-38
src/lib/utils/search.ts 94.8% 75% 75% 94.8% 14-15, 76-77
src/lib/utils/viewer.ts 91.66% 100% 77.77% 91.66% 100-109, 115-121
src/routes/(app)/documents/[id]-[slug]/+page.svelte 0% 0% 0% 0% 1-58
src/routes/(app)/documents/[id]-[slug]/+page.ts 0% 0% 0% 0% 1
src/routes/embed/documents/[id]-[slug]/+page.svelte 0% 0% 0% 0% 1-41
src/routes/embed/documents/[id]-[slug]/+page.ts 0% 0% 0% 0% 1
Generated in workflow #613 for commit 2e51a01 by the Vitest Coverage Report Action

github-actions[bot] avatar Oct 11 '24 13:10 github-actions[bot]

Deploy Preview for documentcloud-frontend ready!

Name Link
Latest commit 2e51a01ec87c1996d9f15528a91ff4f0ed1e6dc8
Latest deploy log https://app.netlify.com/sites/documentcloud-frontend/deploys/67190d728d0c130008f8de2c
Deploy Preview https://deploy-preview-777.muckcloud.com
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 39
Accessibility: 79
Best Practices: 92
SEO: 85
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Oct 11 '24 13:10 netlify[bot]