Skip to content

Conversation

matthewlipski
Copy link
Collaborator

@matthewlipski matthewlipski commented Dec 5, 2024

This PR fixes a few issues:

fix: Files don't have src attribute in HTML export

Problem

The src attribute for file blocks was not added when exporting to external HTML, since the src would only be set after the download URL was resolved.

Solution

The src attribute is now set to block.props.url if editor.resolveFileUrl isn't defined, which will at least fix the issue for blocks which are not uploaded via the editor. Since editor.resolveFileUrl is async, we don't have a way of getting the correct URL when converting the block, which is done synchronously.

fix: Images/videos throw error exporting to HTML using server editor

Problem

Exporting file blocks using the server editor would throw an error. This was because of a typing issue, as even though _tiptapEditor is not defined for the ServerBlockNoteEditor, you can still use all of its functions as if it was. The error was being thrown as the image block render function was trying to access _tiptapEditor.view.dom.

Solution

Image/video block max widths are now constrained by CSS instead of the editor DOMRect, meaning they no longer need to access _tiptapEditor.

Also, the typing has been adjusted such that at least _tiptapEditor.view can be undefined, as this can even happen for the regular client editor. This is a small improvement, but we should look into a more robust typing fix in the future.

feat: Improved image/video block resize handling

Problem

The width of image/video blocks is constrained to the width of the editor. This was done by checking if the width of an image/video block was wider than the editor DOMRect when rendering or resizing, then updating it if necessary. These updates were causing a number of issues.

First, it was not great code-wise as clamping the width of image/video blocks could be accomplished using CSS instead.

Second, upon loading, images outside the viewport horizontally would become invisible, as the editor DOMRect width would be 0. So any images/videos would also also have their widths set to 0 to not be wider than the editor.

Finally, image/video caption widths would only update once the user stopped resizing the image/video itself, leading to bad UX.

Solution

As mentioned in the previous issue, constraining the image/video block widths is now done with CSS. The previewWidth of file blocks is no longer limited to the width of the editor. Instead, the blocks' widths are now capped using a CSS max-width rule. Since previewWidth no longer necessarily represents the visual width of a file block, the block's referenceRect is used to get the actual width when resizing.

This also fixes the second issue as we're no longer checking any DOMRects on initial render - we just set the width to previewWidth and CSS figures out if it needs to be clamped.

Finally, the FileAndCaptionWrapper & ResizeHandlesWrapper elements/components have been removed & restructured into FileBlockWrapper & ResizableFileBlockWrapper. DefaultFilePreview has also been renamed to FileNameWithIcon for better clarity. The changes made both fix the caption resizing issue, and streamline the rendered output by getting rid of a few wrapper divs.

TODO

  • Update existing test snapshots
  • Add/update tests for ServerBlockNoteEditor exporting file blocks to HTML

Closes #1036
Closes #1049
Closes #1309

Copy link

vercel bot commented Dec 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Dec 9, 2024 7:45pm
blocknote-website ✅ Ready (Inspect) Visit Preview Dec 9, 2024 7:45pm

- Renamed `DefaultFilePreview` to `FileNameWithIcon`
- Made file `src` get set straight away only if `resolveFileUrl` is undefined
Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

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

awesome progress!

Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

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

Looks good if there are no major changes in the moved files (can't really easily review that as the diff doesn't show)

I liked the convenience of the default resolveFileUrl, but don't think it's worth implementing a different option (e.g.: hasCustomResolveFileUrl: boolean) as that doesn't seem great either (unless you have a better idea :) )

@matthewlipski matthewlipski merged commit c689e4b into main Dec 10, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants