Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental pdf exports v2 #639

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Experimental pdf exports v2 #639

wants to merge 12 commits into from

Conversation

MTRNord
Copy link
Contributor

@MTRNord MTRNord commented Nov 20, 2024

Untitled-58.pdf

TODOs:

For our internal tracking

  • NEO-829 is fixed using the new calculation code for font sizes
  • NEO-819 is fixed because the lib supports image based emoji for that. It uses tweemoji currently
  • NEO-885 I believe is fixed. Hard to tell but so far not a single image failing
  • NEO-893 fixed
  • NEO-941 fixed and can be easily split into a new PR if we want to land this early

✔️ Checklist

  • A changeset describing the change and affected packages (more info).
  • Added or updated documentation.
  • Tests for new functionality and regression tests for bug fixes.
  • Screenshots or videos attached (for UI changes).
  • All your commits have a Signed-off-by line in the message (more info).

Copy link

changeset-bot bot commented Nov 20, 2024

⚠️ No Changeset found

Latest commit: 398c47b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@@ -40,17 +40,74 @@ export function image(element: ImageElement, base64content: string): Content {
};
}

function preprocessSvg(element: ImageElement, svg: string): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Canvas requires there to be a width and height on the svg element. Otherwise it wont draw it.

const svgString = atob(svg);

// We lied in the past about the mimetype, so we need to check if the svg is actually an svg and otherwise return the original string
if (!svgString.startsWith('<svg')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apaprently we had nwb files which lied and marked png files as svg. This is a sanity check for it...

}

// If we now end up with a png or jpeg image we can return it early
if (element.mimeType === 'image/png' || element.mimeType === 'image/jpeg') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the short-circuit for files which lied about being an svg.

>
{element.text && (
<Text
x={element.position.x + element.width / 2 - textWidth / 2}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason (bug?) react-sdk doesnt seem to position this correctly inside the rectangle so i had to do it myself.

);
})}
</Svg>
{imageElements.map((element, j) => (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react-pdf will not work if the image is inside of the svg.

Font.registerEmojiSource({
format: 'png',
// TODO: Replace with something we serve ourselves
url: 'https://cdnjs.cloudflare.com/ajax/libs/twemoji/14.0.2/72x72/',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Replace with something hosted by us inside of the neoboard.


import { renderPDF } from './pdf.local';

if (import.meta.hot) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a whole bunch of hacking because HMR for react in vite currently is totally messed up with webworkers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it explicitly error out which is intended here. Otherwise it would crash the webworker when using react.

import { WhiteboardDocumentExport } from '../../../state/export/whiteboardDocumentExport';

const renderPDF = async (exportData: WhiteboardDocumentExport) => {
const { pdf, Font } = await import('@react-pdf/renderer');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can not use any file level imports for things that interact with react in anyway as this will break HMR on the webworker due to vite bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the bundles are split correctly across the 2 worlds

@MTRNord MTRNord force-pushed the marcel/ex/react-pdf-export branch from 723eb8d to 56153b3 Compare November 20, 2024 01:15
@@ -23,7 +23,8 @@
"react-redux": "^9.1.2"
},
"resolutions": {
"fork-ts-checker-webpack-plugin": "^6.5.3"
"fork-ts-checker-webpack-plugin": "^6.5.3",
"@vitejs/plugin-react-swc": "git+https://github.com/vitejs/vite-plugin-react-swc.git"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required as we require vitejs/vite-plugin-react-swc@beb09db

@jaller94 jaller94 changed the title Experiemental pdf exports v2 Experimental pdf exports v2 Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant