Grida Canvas - canvas-native text editor#564
Merged
softmarshmallow merged 15 commits intomainfrom Mar 10, 2026
Merged
Conversation
- Updated the `TextEditSession` to be generic over the layout engine, allowing for more flexible integration of different layout implementations, such as `SkiaLayoutEngine` and `SimpleLayoutEngine`. - Introduced the `ManagedTextLayout` trait to encapsulate layout lifecycle management, providing methods for layout invalidation and updates. - Refactored the `Cargo.toml` to make the `skia-safe` dependency optional and organized feature flags for better modularity. - Adjusted the `wd_text_editor` example to utilize the new layout engine structure, improving clarity and maintainability. These changes aim to streamline the text editing architecture, enhancing flexibility and performance in the grida-text-edit project.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a full text-editing subsystem: WASM bindings and C ABI bridge, generic text-edit session backed by managed layout engines (Skia support), paragraph layout adapter, editor surface (WASM relay + DOM fallback), cache invalidation, devtools overlay rendering, IME/clipboard/undo/redo/styling APIs, and docs/tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Surface as SurfaceTextEditor
participant JS as JS/WASM Scene
participant App as UnknownTargetApplication
participant Session as TextEditSession
participant Layout as ParagraphCacheLayout
participant Skia as Skia
User->>Surface: focus / key / IME / pointer
Surface->>JS: text_edit_enter / command / ime_* / pointer_*
JS->>App: FFI -> text_edit_* calls
App->>Session: create/update session, apply commands
Session->>Layout: ensure_layout / rebuild_blocks_with_preedit
Layout->>Skia: build_paragraph / measure / geometry queries
Session-->>App: updated text / caret / selection
App->>JS: text_edit_get_* responses / exit -> commit text
App->>JS: request redraw / decorations
JS->>Surface: render overlay (selection/caret)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…iting - Removed unnecessary canvas clipping and saving in the `wd_text_editor` example, simplifying the drawing logic. - Enhanced the `TextEditSession` to correctly position the caret when an IME preedit is active, ensuring it reflects the end of the preedit text. - Added new methods to `GenericEditHistory` and `TextEditSession` for better access to undo states and history texts, facilitating integration with external history systems. These changes aim to streamline caret management and enhance the overall functionality of the text editing experience in the grida-text-edit project.
- Removed unused imports and streamlined the `draw_session` function in the `wd_text_editor` example for clarity. - Improved the `SkiaLayoutEngine` to support a consistent layout during IME preedit by implementing a strut style that prevents visual jumps. - Added a new method to rebuild the layout with preedit text, ensuring accurate metrics and selection handling. - Updated the cursor positioning logic to reflect the end of the preedit text correctly. These changes aim to enhance the text editing experience by improving layout stability and handling of IME preedit events in the grida-text-edit project.
- Added support for text editing sessions, allowing users to enter and exit editing mode for text nodes. - Implemented a new `ActiveTextEdit` structure to manage the state of text editing, including cursor positioning and selection handling. - Introduced a `ParagraphCacheLayout` to ensure consistent layout during editing, matching the rendering behavior of the Painter. - Enhanced the `Renderer` to update text content dynamically during editing, invalidating caches as necessary. - Added new methods for handling text editing commands, including undo and redo functionality. - Created a `TextEditDecorationOverlay` for rendering caret and selection highlights in the devtools overlay. These changes aim to provide a robust text editing experience within the grida-canvas framework, enhancing usability and functionality.
- Introduced a new document detailing research and comparative analysis of rich text persistency models, outlining design constraints and surveying how various systems represent styled text. - The document serves as a foundational reference for future schema design work, emphasizing CRDT compatibility, non-nested structures, and pragmatic evolution-friendly approaches. These changes aim to provide a comprehensive resource for understanding rich text persistency, aiding in the development of a robust model for Grida.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/wg/feat-text-editing/richtext-persistency.md (1)
85-91: Add language specifiers to fenced code blocks.Several code blocks lack language identifiers, which affects syntax highlighting and documentation tooling. Based on context:
- Lines 85-91: Appears to be a field list → use
textor leave as-is (it's a property list, not code)- Lines 109-118, 135-142: Figma schema → use
textorprotobuf- Lines 268-271: Peritext operations → use
text- Lines 354-357: Rust-like pseudocode → use
rustortext- Lines 490-503, 637-641, 647-655: Pseudocode → use
text📝 Example fix for lines 109-118
-``` +```text TextData { characters: string // flat plain textAlso applies to: 109-118, 135-142, 268-271, 354-357, 490-503, 637-641, 647-655
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wg/feat-text-editing/richtext-persistency.md` around lines 85 - 91, Update the fenced code blocks in richtext-persistency.md to include appropriate language specifiers: add ```text around the property list starting with "fontFamily, fontFamilyFallback, fontSize, fontWeight, fontStyle, ..." and use ```text or ```protobuf for the Figma schema blocks (the TextData / schema snippets), ```text for the Peritext operations blocks, ```rust for the Rust-like pseudocode block, and ```text for all remaining pseudocode blocks; ensure each fenced block is tagged consistently so tooling/syntax highlighting recognizes them.crates/grida-text-edit/src/skia_layout.rs (1)
1436-1442: Minor duplication: traitinvalidate()reimplements inherent method.The trait implementation duplicates the four lines from the inherent
invalidate()method (lines 1028-1033). Consider delegating to the inherent method for DRY.♻️ Proposed simplification
fn invalidate(&mut self) { - // Delegate to the existing invalidate method. - self.paragraph = None; - self.blocks.clear(); - self.cached_text.clear(); - self.cached_line_metrics = None; + SkiaLayoutEngine::invalidate(self); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-text-edit/src/skia_layout.rs` around lines 1436 - 1442, The trait implementation of invalidate() duplicates the inherent invalidate() behavior; replace the duplicated body in the trait impl with a delegation to the inherent method by calling the concrete type's inherent invalidate (i.e. invoke <ConcreteType>::invalidate(self>) from inside the trait impl so the four lines (clearing paragraph, blocks, cached_text and cached_line_metrics) are maintained in one place; locate the trait impl's invalidate() and the existing inherent invalidate() and perform this delegation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/grida-text-edit/src/skia_layout.rs`:
- Around line 1436-1442: The trait implementation of invalidate() duplicates the
inherent invalidate() behavior; replace the duplicated body in the trait impl
with a delegation to the inherent method by calling the concrete type's inherent
invalidate (i.e. invoke <ConcreteType>::invalidate(self>) from inside the trait
impl so the four lines (clearing paragraph, blocks, cached_text and
cached_line_metrics) are maintained in one place; locate the trait impl's
invalidate() and the existing inherent invalidate() and perform this delegation.
In `@docs/wg/feat-text-editing/richtext-persistency.md`:
- Around line 85-91: Update the fenced code blocks in richtext-persistency.md to
include appropriate language specifiers: add ```text around the property list
starting with "fontFamily, fontFamilyFallback, fontSize, fontWeight, fontStyle,
..." and use ```text or ```protobuf for the Figma schema blocks (the TextData /
schema snippets), ```text for the Peritext operations blocks, ```rust for the
Rust-like pseudocode block, and ```text for all remaining pseudocode blocks;
ensure each fenced block is tagged consistently so tooling/syntax highlighting
recognizes them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9324a23-566e-45b7-98cc-15c37346865a
📒 Files selected for processing (4)
crates/grida-canvas/src/text/paragraph_cache_layout.rscrates/grida-text-edit/src/skia_layout.rscrates/grida-text-edit/src/text_edit_session.rsdocs/wg/feat-text-editing/richtext-persistency.md
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/grida-canvas/src/text/paragraph_cache_layout.rs
- Added a detailed section on ProseMirror, highlighting its schema-driven document model, editing model, and relevance to rich text persistency. - Discussed the strengths and trade-offs of ProseMirror's approach, including its explicit transform pipeline and mark-based inline styling. - Updated the document structure to reflect the addition of ProseMirror, ensuring comprehensive coverage of various rich text models. These changes enhance the research document, providing valuable insights into ProseMirror's capabilities and its implications for future schema design.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/wg/feat-text-editing/richtext-persistency.md (3)
343-348: Add blank lines around the table.The table at line 344 should be surrounded by blank lines for proper rendering and consistency with Markdown best practices.
♻️ Proposed fix
**Mark behavior classification**: + | Mark type | Start anchor | End anchor | Boundary behavior | |---|---|---|---| | Bold, italic, etc. | `before` first char | `before` char after last | Grows at end, not at start | | Link, comment | `before` first char | `after` last char | Does not grow at either end | + **Conflict resolution**:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wg/feat-text-editing/richtext-persistency.md` around lines 343 - 348, The Markdown table under the "Mark behavior classification" header lacks surrounding blank lines; edit the section in richtext-persistency.md to insert one blank line immediately before the table and one blank line immediately after it so the table is isolated for proper Markdown rendering and consistent formatting.
339-340: Standardize emphasis style to asterisks.Lines 339-340 use underscore emphasis (
_before_,_after_) instead of the asterisk style used throughout the rest of the document. For consistency, change to asterisk emphasis.♻️ Proposed fix
-- `type: "before"` means the mark starts/ends in the gap _before_ the character. -- `type: "after"` means the mark starts/ends in the gap _after_ the character. +- `type: "before"` means the mark starts/ends in the gap *before* the character. +- `type: "after"` means the mark starts/ends in the gap *after* the character.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wg/feat-text-editing/richtext-persistency.md` around lines 339 - 340, Update the inline emphasis to use asterisks for consistency: in the lines describing `type: "before"` and `type: "after"` (the phrases currently written as `_before_` and `_after_`), replace the underscore-style emphasis with asterisk-style emphasis (`*before*` and `*after*`) so the wording matches the rest of the document.
85-91: Add language specifiers to fenced code blocks.Several code blocks are missing language identifiers, which affects syntax highlighting and documentation consistency. Add appropriate language tags (
dart,kiwi,javascript,typescript, or leave empty for pseudocode/plain text blocks where appropriate).📝 Examples of fixes
For the Flutter TextStyle fields block (lines 85-91), if it's pseudocode:
-``` +```text fontFamily, fontFamilyFallback, fontSize, fontWeight, fontStyle, ...For the Figma Kiwi schema (lines 109-117):
-``` +```kiwi TextData { ...For the Peritext anchor operations (lines 331-334):
-``` +```javascript addMark(opId, start: {type, charId}, end: {type, charId}, markType, ...)Apply similar fixes to blocks at lines 135-142, 249-255, 417-420, 553-561, 700-704, and 710-718.
Also applies to: 109-117, 135-142, 249-255, 331-334, 417-420, 553-561, 700-704, 710-718
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wg/feat-text-editing/richtext-persistency.md` around lines 85 - 91, Several fenced code blocks in the document (e.g., the Flutter TextStyle fields block listing "fontFamily, fontSize, ...", the Figma Kiwi schema block "TextData { ... }", the Peritext anchor operations block "addMark(opId, start: {type, charId}, ...)" and other blocks at the indicated ranges) are missing language specifiers; update each fenced code block to include an appropriate language tag (use `text` or `dart` for Flutter/TextStyle lists, `kiwi` for Figma schema blocks, `javascript`/`typescript` for Peritext/anchor op examples, or leave empty if truly pseudocode) so syntax highlighting and consistency are restored—locate the blocks by their contents (e.g., the TextStyle fields list, the "TextData" schema, and the "addMark(...)" anchor operation) and prepend the proper language identifier after the opening ``` fence for each listed block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/wg/feat-text-editing/richtext-persistency.md`:
- Around line 343-348: The Markdown table under the "Mark behavior
classification" header lacks surrounding blank lines; edit the section in
richtext-persistency.md to insert one blank line immediately before the table
and one blank line immediately after it so the table is isolated for proper
Markdown rendering and consistent formatting.
- Around line 339-340: Update the inline emphasis to use asterisks for
consistency: in the lines describing `type: "before"` and `type: "after"` (the
phrases currently written as `_before_` and `_after_`), replace the
underscore-style emphasis with asterisk-style emphasis (`*before*` and
`*after*`) so the wording matches the rest of the document.
- Around line 85-91: Several fenced code blocks in the document (e.g., the
Flutter TextStyle fields block listing "fontFamily, fontSize, ...", the Figma
Kiwi schema block "TextData { ... }", the Peritext anchor operations block
"addMark(opId, start: {type, charId}, ...)" and other blocks at the indicated
ranges) are missing language specifiers; update each fenced code block to
include an appropriate language tag (use `text` or `dart` for Flutter/TextStyle
lists, `kiwi` for Figma schema blocks, `javascript`/`typescript` for
Peritext/anchor op examples, or leave empty if truly pseudocode) so syntax
highlighting and consistency are restored—locate the blocks by their contents
(e.g., the TextStyle fields list, the "TextData" schema, and the "addMark(...)"
anchor operation) and prepend the proper language identifier after the opening
``` fence for each listed block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dff9935a-e266-4a56-bcc2-61dca845e0f4
📒 Files selected for processing (1)
docs/wg/feat-text-editing/richtext-persistency.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/grida-canvas-wasm/package.json`:
- Line 3: Bump the pinned consumer dependency: update the dependency entry for
"@grida/canvas-wasm" in the grida-canvas-sdk-render-figma package.json from
"0.90.0-canary.9" to "0.90.0-canary.10" so the consumer resolves the newly
published package (ensuring the new text-edit API is available).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18f08160-30a6-4279-8e89-b164f85deafc
⛔ Files ignored due to path filters (1)
crates/grida-canvas-wasm/lib/bin/grida_canvas_wasm.wasmis excluded by!**/*.wasm
📒 Files selected for processing (2)
crates/grida-canvas-wasm/lib/bin/grida-canvas-wasm.jscrates/grida-canvas-wasm/package.json
| { | ||
| "name": "@grida/canvas-wasm", | ||
| "version": "0.90.0-canary.9", | ||
| "version": "0.90.0-canary.10", |
There was a problem hiding this comment.
Update pinned consumers to 0.90.0-canary.10 in the same PR.
Line 3 bumps the published package, but packages/grida-canvas-sdk-render-figma/package.json Line 64-67 still pins @grida/canvas-wasm to 0.90.0-canary.9. That consumer will keep resolving the old artifact and miss the new text-edit API added here.
Suggested follow-up
--- a/packages/grida-canvas-sdk-render-figma/package.json
+++ b/packages/grida-canvas-sdk-render-figma/package.json
@@
- "@grida/canvas-wasm": "0.90.0-canary.9",
+ "@grida/canvas-wasm": "0.90.0-canary.10",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-canvas-wasm/package.json` at line 3, Bump the pinned consumer
dependency: update the dependency entry for "@grida/canvas-wasm" in the
grida-canvas-sdk-render-figma package.json from "0.90.0-canary.9" to
"0.90.0-canary.10" so the consumer resolves the newly published package
(ensuring the new text-edit API is available).
This was referenced Mar 28, 2026
Merged
Merged
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
1 participant
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.
Uh oh!
There was an error while loading. Please reload this page.