Formatted text prototype#26228
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a formatted text prototype that extends the existing basic text functionality with support for character-level formatting and inline objects. It is designed to match Quill's formatting requirements.
Changes:
- Added new
FormattedTextAsTreenamespace with support for character formatting (bold, italic, underline, font, size) and line formatting (HTML tags like h1-h5, li) - Exported
charactersFromStringfunction fromtextDomain.tsfor reuse in formatted text implementation - Added comprehensive test coverage including schema compatibility tests and functional tests for formatting operations
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/dds/tree/src/text/textDomainFormatted.ts | New file implementing formatted text with character/line formatting and inline objects support |
| packages/dds/tree/src/text/textDomain.ts | Exported charactersFromString function for reuse |
| packages/dds/tree/src/text/index.ts | Exported FormattedTextAsTree namespace |
| packages/dds/tree/src/index.ts | Re-exported FormattedTextAsTree from public API |
| packages/dds/tree/src/test/text/textDomainFormatted.spec.ts | Added comprehensive tests for formatted text functionality |
| packages/dds/tree/src/test/text/textDomain.spec.ts | Added schema compatibility test for basic text |
| packages/dds/tree/src/test/domain-schema-compatibility-snapshots/text/2.81.0.json | Schema snapshot for basic text compatibility |
| packages/dds/tree/src/test/domain-schema-compatibility-snapshots/formattedText/2.81.0.json | Schema snapshot for formatted text compatibility |
| }); | ||
| } | ||
|
|
||
| public charactersFormatted(): Iterable<FormattedTextAsTree.StringAtom> { |
There was a problem hiding this comment.
Nit: "formattedCharacters"? Both because it's more like natural English, and because this phrasing could be confused for a shorthand of "are characters formatted".
There was a problem hiding this comment.
When we have multiple related members on an interface which are different versions of the same thing (in this the inherited characters and charactersFormatted) I like for them to start with the same string and have the disambiguater at the end as it helps discoverability a lot (for example if someone is using characters it makes it clear there is an alternative which has formatting information).
SharedObjectCore's reSubmitCore vs reSubmitSquashed are examples of this as is our exportConcise and exportVerbose APIs.
If we want to better align with English, maybe we could do charactersWithFormatting?
There was a problem hiding this comment.
I like charactersWithFormatting
| * Using larger atoms and splitting them as needed is NOT a recommended approach, since this will result in poor merge behavior for concurrent edits. | ||
| * Instead atoms should always be the smallest unit of text which will be independently selected, moved or formatted. |
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
Description
Minimal prototype for formatted text with inline objects.
This is targeting matching what Quill needs for formatting, see #26217
Reviewer Guidance
The review process is outlined on this wiki page.