feat: add a gif node to markup, with serialization and viewer support - #25
Open
shanzez wants to merge 1 commit into
Open
feat: add a gif node to markup, with serialization and viewer support#25shanzez wants to merge 1 commit into
shanzez wants to merge 1 commit into
Conversation
Markup had no way to represent an animated GIF. Images exist, but the image node is disabled in both message composers via kitOptions, so a GIF sent in chat had nowhere to live. This adds a first-class `gif` node rather than re-enabling images, which keeps the composers' existing image policy untouched. What changed: - `MarkupNodeType.gif` plus a GifNode tiptap extension, registered in the server kit as well as the client so stored markup round-trips. - Markdown and HTML serialization both ways. The HTML case reads `src` OR a blob reference; an earlier draft read only `src` and silently produced `src="undefined"` for blob-backed GIFs. - `insertGif` on the editor handler, implemented across every editor that exposes one so no composer is silently missing it. - The node enabled in both message composers. - Rendering in BOTH markup viewers. `NodeContent.svelte` had a branch for image and none for gif, so a GIF rendered as the literal text `unknown node: "gif"`; `LiteNodeContent.svelte` rendered nothing at all. Two things worth calling out for reviewers: `gif` is added to `nonEmptyNodes` in text-core. Without it a message containing only a GIF counts as empty, so `canSubmit` stays false and the send button never enables. `emoji` is already on that list for the same reason. This is easy to miss because serialization tests all pass while the feature is unusable. `markupViewerCoverage.test.ts` is a structural test: it reads the viewer sources and asserts each composable node type has a branch. It exists because the missing-branch bug above passed every serialization test in the suite. Reading files needs node types, hence the tsconfig `types` addition in `packages/presentation`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Markup has no way to represent an animated GIF. Images exist, but the image node is disabled in both message composers via
kitOptions(AttachmentRefInput.svelteand communication'sTextInput.svelte), so a GIF sent in chat has nowhere to live.This adds a first-class
gifnode rather than re-enabling images, which leaves the composers' existing image policy untouched.It is deliberately source-agnostic: the node accepts either a blob reference or an external
src, so a workspace-hosted GIF and a third-party one both work. No provider is included here.What changed
MarkupNodeType.gifintext-core, plus aGifNodetiptap extension registered in the server kit as well as the client, so stored markup round-trips through the server.srcor a blob reference — an earlier draft read onlyattrs.srcand silently emittedsrc="undefined"for blob-backed GIFs.insertGifonTextEditorHandler, implemented in every editor that exposes one. There are five call sites, not four:TextEditor.svelteholds the low-level export the others delegate to.defaultRefActionsand communication'sdefaultMessageInputActions) fanning out to ~10 surfaces, so a blanket swap would have changed editors that should not gain the action.NodeContent.sveltehad a branch for image and none for gif, so a GIF rendered as the literal textunknown node: "gif".LiteNodeContent.svelterendered nothing at all.Two things worth a reviewer's attention
gifis added tononEmptyNodes. Without it, a message containing only a GIF counts as empty, socanSubmitstays false and the send button never enables — the feature looks broken in exactly the case people use it for.emojiis already on that list for the same reason. This is easy to miss because every serialization test passes while the feature is unusable.markupViewerCoverage.test.tsis a structural test. It reads the viewer sources and asserts that each composable node type has a branch, because the missing-branch bug above passed the entire serialization suite. Reading files needs node types, hence thetypesaddition topackages/presentation's tsconfig. If you would rather not have a source-reading test, say so and I will drop it — but something needs to catch that class of bug.Verification
Built and exercised on a self-hosted instance running this node in production chat, including the GIF-only message case.
rush validateandrush svelte-checkpass on the branch this was developed on. This PR is a clean cherry-pick ontodevelop; CI here is the first run against this base.