MuckRock / documentcloud-frontend

DocumentCloud's front end source code - Please report bugs, issues and feature requests to info@documentcloud.org
https://www.documentcloud.org
GNU Affero General Public License v3.0
15 stars 5 forks source link

[sveltekit] Text view #532

Closed eyeseast closed 2 months ago

eyeseast commented 2 months ago

https://deploy-preview-532.muckcloud.com/documents/20006357-reportersguide/?mode=text

Screenshot 2024-04-26 at 12 04 21 PM
netlify[bot] commented 2 months ago

Deploy Preview for documentcloud-frontend ready!

Name Link
Latest commit ba89e3fdb34c8aba0015b3e4fedd4fbde707dce5
Latest deploy log https://app.netlify.com/sites/documentcloud-frontend/deploys/663128c6173aa80008a31628
Deploy Preview https://deploy-preview-532.muckcloud.com
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

allanlasser commented 2 months ago

Awesome start. Initial thoughts:

Do we need to render notes, redactions, or other edits onto the text?

allanlasser commented 2 months ago

https://www.figma.com/file/KVRuo8Ss3TvzreCmT6HQke/DC-SvelteKit?type=design&node-id=1522%3A13384&mode=dev

github-actions[bot] commented 2 months ago

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 37.9% 2892 / 7630
🔵 Statements 37.9% 2892 / 7630
🔵 Functions 26.87% 43 / 160
🔵 Branches 47.53% 106 / 223
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
src/lib/api/documents.ts 82.29% 79.31% 83.33% 82.29% 27-55, 61-90, 120-121, 125-126, 132-133, 166-167, 228
src/lib/api/types.d.ts 0% 0% 0% 0% 1-226
src/lib/components/documents/TextPage.svelte 0% 0% 0% 0% 1-55
src/routes/app/+page.svelte 0% 0% 0% 0% 1-119
src/routes/app/add-ons/+page.svelte 0% 0% 0% 0% 1-131
src/routes/app/projects/+page.svelte 0% 0% 0% 0% 1-64
src/routes/app/projects/[id]-[slug]/+page.svelte 0% 0% 0% 0% 1-112
src/routes/documents/[id]-[slug]/+layout.ts 0% 0% 0% 0% 1-43
src/routes/documents/[id]-[slug]/+page.svelte 0% 0% 0% 0% 1-177
src/routes/documents/[id]-[slug]/+page.ts 0% 0% 0% 0% 1-27
Generated in workflow #89
eyeseast commented 2 months ago

If the text view is a state of the document, then it may be better as a search param than a path. If the view is a resource of the document, then having /text in the path is better. Both approaches are equivalent, so having a stated reason will help with https://github.com/MuckRock/documentcloud-frontend/issues/522

Text has to be fetched on its own, after we fetch the document data (because we need document.asset_url), so that makes me think it's better giving it a dedicated URL so we can fetch it server-side.

We have four document modes:

Those could all be at the same URL with query params to switch. Text is the only resource that needs to be fetched on its own.

eyeseast commented 2 months ago

Let's merge this and then deal with the page hash when we do pagination, which I think will end up addressing that.

I'm not sure what's happening with mode and the initial state.