Repository navigation
Replace the TipTap block's paragraph/heading options with configurable textBlocks - #6382
Conversation
|
@greptileai review |
|
|
@VPS-Andreas the lint workflow fails. |
27f0307 to
44ebebe
Compare
An alternative to #6390, solving the same problem with a smaller API. A block's `version` is currently a single counter, so only one party can advance it. That works while an application owns all of a block's migrations, but not when the library providing the block ships migrations as well. ## Examples where this is a problem: - `createTipTapRichTextBlock` with `migrateFromDraftJs` (currently we workaround the limitation by letting the vendor use version 1 and application migrations must start at 2) - future tiptap migrations (will be needed in #6382) - while keeping the option for project migrations - existing library blocks that have migrations: Youtube and ExternalLink currently can't have project migrations ## Proposed solution Add a second `migrateVendor`, a sibling of the `migrate` option the application fills. They count from 1 in `$$vendorVersion`, independently of the block's `version`, and run before the block's own migrations. `migrate` stays exactly as it is, and because the two options are separate, a library never merges its migrations into the application's object and neither side can overwrite the other's. example (in real world this would be split in library and project): ```ts createBlock(VideoBlockData, VideoBlockInput, { name: "Video", migrate: { //defined in project version: 1, migrations: typeSafeBlockMigrationPipe([ChangeTitleMigration]), }, migrateVendor: { //defined in library version: 1, migrations: typeSafeBlockMigrationPipe([ChangeAspectRatioMigration]), }, }); ``` ### naming I decided to name the new counter "vendor", because it is a migration from a 3rd party vendor. Other option would be "library". A single vendor counter is enough, any other package providing blocks should also use vendor migrations. ## Further information Differences to #6390: - There are exactly two chains instead of freely named scopes, and the library's chain is its own option rather than a nested `scopes` record inside the application's. - The application-facing API is untouched: `migrate` keeps its shape, and `$$version` its meaning — only `$$vendorVersion` is added next to it. - Vendor migrations run before the block's own migrations, instead of after. - The migrations Dextinity ships actually move into the new chain, which is what frees the block's versions for applications. Their legacy counter is split before any migration runs, so a chain can't seed itself from a counter another chain has just advanced. Note: extending eg. Youtube block is currently not possible because Data and Input classes are not exported. Session: https://claude.ai/code/session_01ByXXwcnSeVYzQfLujMknXH --------- Co-authored-by: Claude <noreply@anthropic.com>
44ebebe to
585aeb6
Compare
…e textBlocks The text block type select could only offer a fixed paragraph entry plus every enabled heading level, in a fixed order, so a design that puts a display headline above a regular heading 1 had no way to express it. textBlocks configures the types explicitly and lets several of them share a tag, told apart by the node's new textBlock attribute. A node written before that attribute existed is resolved by its tag, so existing content keeps working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…xtBlock attribute Resolving a node by its tag is ambiguous as soon as two text blocks share one: the first match wins, so which text block a node belongs to depends on the order of textBlocks. The migration writes the resolved name into the content once, which also repairs a name left stale by a migration that changed a node's tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two text blocks could not share a tag before this option existed, so no content out there is ambiguous today. The migration is for the step after: pinning the text block of existing content before a second one starts sharing its tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The type select sets the node's type and its text block together, but Mod-Alt-<level> changes only the type and carries the previous name along - a paragraph left with textBlock "heading-1" is content the API rejects, since a name has to belong to the node's tag. A plugin re-resolves such a name instead of rebuilding the shortcuts, which covers pasting and input rules as well. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A name that doesn't exist made the conversion fall back to whichever text block shares the DraftJS block's tag, silently and only once - the DraftJS content is gone afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test configured two text blocks on one tag and asserted that legacy content ends up on the first of them - the outcome the migration is meant to prevent. It now migrates with the configuration that is live first, then adds the second text block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1340bd7 to
ac55623
Compare
The content named its text block next to the tag and heading level it was stored as, which said the same thing twice: the tag belongs to the text block's configuration, and keeping it in the content meant a migration whenever that configuration changed. A single textBlock node carries the name alone, so the tag follows the configuration - and a name can no longer contradict the node it sits on, which the shortcuts used to manage. The tag fallback and buildApplyTextBlocksMigration are gone with the tags they resolved; a vendor migration converts the content written before this instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A site has the content but not the block's configuration, so it can't know which tag a text block was configured with - it maps the name to its own component instead. The default handler renders a paragraph, which is what a block without headings needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every paragraph and heading became one textBlock node, which left the content written before that without a node type the schema knows. A vendor migration converts it when the block is loaded, so no project has to carry the conversion itself. The vendor chain has to be gapless, so the migration takes version 2 where migrateFromDraftJs occupies version 1 and version 1 otherwise. It resolves the text block from the node's tag, which is unambiguous for this content: two text blocks could not share a tag before textBlocks existed. Content that is already converted passes through unchanged, because the DraftJS migration sits in the same chain and writes the current format. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ac55623 to
d6241bf
Compare
A node that names no text block takes the schema's default, and that default may well be a heading: lists only require a p text block to exist, not to be the default one. The check resolved a name and let a node without one through, so content that doesn't come from the editor could store a heading inside a list item after all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
VPS-Obi
left a comment
There was a problem hiding this comment.
The tag is not stored at all — it follows from the configuration.
If the tag isn't stored, why do we need to configure it in the Admin and API?
…DraftJs The chain has to be gapless, so the text block node migration counted as version 1 without the DraftJS migration and as version 2 with it. Every migration added later would have had to know which of the earlier ones a given configuration switched on. A migration that changes nothing holds the version instead, so the ones after it count the same however the block is configured. legacyVersions stays conditional: it says that the DraftJS migration once counted in $$version, which is not true for a block that never had it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@greptileai can you explain? |
|
The tag is configuration, not content data. The stored
Keeping the tag out of each node prevents contradictory data such as a |
It holds the version the DraftJS migration occupies, which is a detail of this block's vendor chain and not something another block is expected to need. Sitting next to the migrations it belongs to also keeps it out of the block factory, which is long enough already. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…6383) A text block style had to be configured in two places: globally in `textBlockStyles`, and again through an `appliesTo` listing the text block types it was allowed for. Which styles a text block offers could only be read by scanning every style's `appliesTo`, and two text blocks sharing a tag could not be told apart at all. A text block now carries its style definitions directly, and `textBlockStyles` is gone: ```tsx const headlineStyles: TipTapTextBlockStyle[] = [ { name: "headline300", label: "Headline 300", element: (props, Tag) => <Tag {...props} /> }, { name: "headline400", label: "Headline 400", element: (props, Tag) => <Tag {...props} /> }, ]; createTipTapRichTextBlock({ textBlocks: [ { name: "heading-1", label: "Heading 1", tag: "h1", styles: headlineStyles }, { name: "heading-2", label: "Heading 2", tag: "h2", styles: headlineStyles }, { name: "display", label: "Display", tag: "h1", element: (props, Tag) => <Tag style={{ fontSize: 64 }} {...props} /> }, ], orderedList: { styles: copyStyles }, }); ``` Text blocks that offer the same style share its definition, which makes the shared set explicit instead of implying it through repeated names. A style's `element` receives the tag of the text block it is applied to, so one definition works for several tags. A text block that needs no style choice carries its own `element` instead of `styles`; the two exclude each other in the types, so a text block either offers a choice or renders one way, and the styling select is only shown where there is something to choose. Lists take the same shape instead of `appliesTo: ["ordered-list", "unordered-list"]`. The stored format is unchanged — a style is still identified by its `name` in `textBlockStyle` — so no migration is needed. Second of four stacked pull requests, on top of #6382; #6384 adds `defaultStyle`, #6385 splits the block's Storybook stories by topic. ## Example The unit tests in `createTipTapRichTextBlock.test.ts`/`.test.tsx`, and in Storybook: - [Text Block Styles](https://tiptap-text-block-styles--69df3371c46abe69b5199825.chromatic.com/?path=/story/blocks-tiptaprichtextblock--text-block-styles) - [Text Block Style Interactions](https://tiptap-text-block-styles--69df3371c46abe69b5199825.chromatic.com/?path=/story/blocks-tiptaprichtextblock--text-block-style-interactions) - [List Text Block Styles](https://tiptap-text-block-styles--69df3371c46abe69b5199825.chromatic.com/?path=/story/blocks-tiptaprichtextblock--list-text-block-styles) - [Text Block Element](https://tiptap-text-block-styles--69df3371c46abe69b5199825.chromatic.com/?path=/story/blocks-tiptaprichtextblock--text-block-element) Session: https://claude.ai/code/session_f3c6ad37-c68b-4187-85b6-2abe5aa95c35 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
## Problem Since 10.8.0 (#6382), a TipTap rich text that ends with a list got an empty `bulletList` appended by the editor. A list without items is invalid, so the API rejected the content on save with "Validation failed". Deleting the list did not help: the empty list stayed in the content, rendered as an empty `<ul></ul>`, and editors could not see, select or remove it until they reloaded. Selecting all content and deleting it was affected too. It produced a `bulletList` with a `listItem` and a text block instead of one empty text block, and in a block that only allows headings it produced a `horizontalRule`. ProseMirror needs a default block type in some places, and no setting defines it: it is the first node of the `block` group in schema order (`schema.topNodeType.contentMatch.defaultType`). TipTap's `TrailingNode` inserts it after content that ends with a list or a child block, and ProseMirror's `createAndFill` fills an emptied document with it. The schema order follows the extensions' `priority`. TipTap's `Paragraph` has priority 1000, so the paragraph used to be the default block type. #6382 replaced the paragraph and heading nodes with the `textBlock` node, which had no priority and is registered after StarterKit's lists, so `bulletList` became the default block type. ## Solution `textBlock` now has `priority: 1000` in the Admin and in the API (the API schema must match the Admin schema). This restores what #6382 removed: `textBlock` takes the paragraph's place in the schema, so every place that uses the default block type gets a text block again, not only `TrailingNode`. The editor adds an empty text block after a trailing list, as it did before 10.8.0. Other approaches only fix part of the problem: - Setting the `node` option of `TrailingNode` to `textBlock` fixes `TrailingNode` only. An emptied document would still become a list. - Without `TrailingNode`, nothing follows a child block at the end of the content, so editors could not type after it. - Removing empty lists in the API hides the symptom. The editor would still create invalid content. - Registering `textBlock` before StarterKit gives the same schema order, but only as long as nobody changes the order of the extensions. `priority` is TipTap's way to define it, with the same value as TipTap's `Paragraph`. With `maxTextBlocks`, this empty text block must not be added when the content is already at the limit. Otherwise a list as the last allowed block is followed by a block over the limit, and the API rejects the content. 10.8.0 had the same problem for ordered lists. It only avoided it for bullet lists by accident, because `TrailingNode` never adds a block after its own default type. The `maxTextBlocks` extension now sets TipTap's `skipTrailingNodeMeta` on transactions that leave the content at or over the limit. A filter that rejects these transactions would also reject pastes over the limit instead of cutting them off. `@tiptap/extensions` exports `skipTrailingNodeMeta`, so it is now a direct dependency of `@dextinity/cms-admin`. StarterKit already depends on it. The "List Text Block Styles" story clicked the editor to put the caret at the end of the list item. With the empty text block after the list, that click lands below the list, so the story now clicks the list item text. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The styling select always offered a "Default" entry standing for "no
style", even where a design has no unstyled variant and every text block
is meant to carry one of the configured styles. Editors had to pick the
right style by hand on every new text block, and forgetting it produced
unstyled content.
A text block (or list) with a `defaultStyle` has no such state:
```tsx
createTipTapRichTextBlock({
textBlocks: [
{ name: "heading-1", label: "Heading 1", tag: "h1", styles: headlineStyles, defaultStyle: "headline300" },
{ name: "heading-2", label: "Heading 2", tag: "h2", styles: headlineStyles, defaultStyle: "headline400" },
],
});
```
The select drops its "Default" entry, and the style is applied to new
content and to every text block the editor creates without one —
pressing Enter at the end of a text block, the `Mod-Alt-<level>`
shortcuts, pasting. Rather than reimplementing each of those paths, a
ProseMirror plugin fills in a missing style on document change, so
opening older content leaves it untouched and does not mark the document
as changed. Switching the type keeps a style the new text block also
offers and falls back to its `defaultStyle` otherwise, and toggling a
list hands the text block to the list's styles or back — through the
toolbar buttons and `Mod-Shift-7`/`Mod-Shift-8` alike.
Because the default sits on the text block rather than on a tag, two
text blocks sharing a tag can have different defaults, and a text block
without one keeps its "Default" entry next to text blocks that have one.
Third of four stacked pull requests, on top of #6383 and #6382; #6385
splits the block's Storybook stories by topic.
## Example
`createTipTapRichTextBlock.test.ts`/`.test.tsx` cover the validation and
the content a default style produces. The Storybook story [Default Text
Block
Style](https://tiptap-text-block-default-style--69df3371c46abe69b5199825.chromatic.com/?path=/story/blocks-tiptaprichtextblock--default-text-block-style)
walks through a text block with a default, one without, and the styled
lists.
Session:
https://claude.ai/code/session_f3c6ad37-c68b-4187-85b6-2abe5aa95c35
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The text block type select could only offer a fixed paragraph entry plus every enabled heading level, in a fixed order. A design that wants two entries for the same tag — a display headline above a regular heading 1 — or a different order could not be configured.
textBlocksconfigures the entries explicitly and decouples a text block from the tag it is stored as:Two entries on the same tag only stay apart if the content records which one the editor picked, so paragraphs and headings are stored as a single
textBlocknode that names its entry. The tag is not stored at all — it follows from the configuration. The name is therefore the only thing content carries, and a node's type, its level and its text block can no longer drift apart.The site renders by that name instead of by heading level:
Content written before this change —
paragraphandheadingnodes carrying alevel— is converted when the block is loaded, by a vendor migration (#6401). A project needs no migration of its own. Its own migrations run after Dextinity's and therefore see the converted nodes, so one written against{ type: "heading", attrs: { level } }has to match{ type: "textBlock", attrs: { textBlock } }instead.Because every text block is the same node now, a list item's content expression can no longer keep a heading out of it. The editor takes the content out of the list when a heading is chosen there, turns a heading back into a paragraph when it is wrapped in a list, and the API rejects a heading text block inside a list item.
First of four stacked pull requests: #6383 moves the styles into the text blocks, #6384 adds
defaultStyle, #6385 splits the block's Storybook stories by topic.Example
createTipTapRichTextBlock.test.ts/.test.tsxcover the configuration, the stored format and what a list item accepts,buildTextBlockNodeMigration.test.tsthe conversion. In Storybook:Session: https://claude.ai/code/session_f3c6ad37-c68b-4187-85b6-2abe5aa95c35