feat(canvas): unify MarkdownEmbed to use htmlcss render pipeline#625
feat(canvas): unify MarkdownEmbed to use htmlcss render pipeline#625softmarshmallow merged 2 commits intomainfrom
Conversation
Replace the standalone 877-line markdown→Skia renderer (painter/markdown.rs) with the shared htmlcss pipeline: markdown is converted to HTML via pulldown-cmark, wrapped with grida-markdown.css, and rendered through Stylo CSS → Taffy layout → Skia paint — the same path as HTMLEmbedNode. Key changes: - Add `markdown_to_styled_html()` that converts GFM markdown to a self-contained HTML document with embedded CSS - Add `grida-markdown.css` fixture (element-selector-only stylesheet targeting pulldown-cmark's bare HTML output) - Add faux-table layout module: emulates CSS table via Taffy flex (table→flex-column, tr→flex-row, td→equal-width flex children) - Auto-height on resize: MarkdownEmbed and HTMLEmbed nodes re-measure content height when width changes in grida-dev - Measure content height at load time instead of using 100,000px - Rename MarkdownNodeRec → MarkdownEmbedNodeRec to align with HTMLEmbedNodeRec naming pattern - Delete painter/markdown.rs (877 lines removed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
WalkthroughThis PR migrates Markdown rendering from a direct Skia-based painter to an HTML/CSS pipeline: it renames the markdown node/layer variant to Changes
Sequence Diagram(s)sequenceDiagram
actor Painter
participant MD2HTML as markdown_to_styled_html()
participant HTMLCSS as htmlcss::render()
participant Fonts as FontRepository
Painter->>MD2HTML: markdown_to_styled_html(md)
MD2HTML->>MD2HTML: markdown_to_html + embed CSS
MD2HTML-->>Painter: styled_html
Painter->>HTMLCSS: render(styled_html, width, height, fonts)
HTMLCSS->>Fonts: measure/layout glyphs
alt render success
HTMLCSS-->>Painter: Ok(picture)
Painter->>Painter: draw(picture) with picture.cull_rect clip
else render error
HTMLCSS-->>Painter: Err(_)
Painter->>Painter: draw gray rectangle (width×height)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ce3669584
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// | ||
| /// Targets `.markdown-body` to scope styles and avoid leaking to other HTML. | ||
| pub static GITHUB_MARKDOWN_CSS: &str = | ||
| include_str!("../../assets/css/grida-markdown.css"); |
There was a problem hiding this comment.
This new stylesheet load depends on assets/css/grida-markdown.css being a real symlink target at checkout time. On Windows/dev environments where Git cannot create symlinks (core.symlinks=false), that file is checked out as plain text containing the target path, so include_str! embeds a path string instead of CSS and markdown renders unstyled at runtime even though compilation succeeds. Please store an actual CSS file in-crate (or copy it at build time) instead of depending on a symlink.
Useful? React with 👍 / 👎.
| Rect::from_xywh( | ||
| 0.0, | ||
| 0.0, | ||
| cull.width(), | ||
| cull.height(), |
There was a problem hiding this comment.
The markdown draw path now clips to picture.cull_rect() rather than the node's configured width/height. If content ends up taller than the node (for example after manual height resize, font-metric differences, or failed measurement), markdown text can bleed outside the node and overlap neighbors; the prior markdown renderer clipped to node bounds, so this is a functional regression introduced in this commit.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
crates/grida-canvas/src/layout/engine.rs (1)
2046-2053: Test correctly updated; consider renaming for consistency.The test properly uses
create_markdown_embed_node()andNode::MarkdownEmbed(md). The regression test comment accurately describes the prior bug (returninggrida_style_default()with no dimensions).Minor: The test function name
test_markdown_node_layout_preserves_sizecould be renamed totest_markdown_embed_node_layout_preserves_sizeto match the new variant naming convention used throughout the PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-canvas/src/layout/engine.rs` around lines 2046 - 2053, Rename the test function test_markdown_node_layout_preserves_size to test_markdown_embed_node_layout_preserves_size for naming consistency with the new node variant; update the function declaration and any references or test attributes accordingly so it still uses nf.create_markdown_embed_node() and Node::MarkdownEmbed(md) and preserves the same assertions and setup (e.g., md.size, md.transform, graph.append_child).crates/grida-dev/src/main.rs (1)
406-414: Consider extracting temp font repository creation to reduce duplication.The same pattern for creating a temporary
FontRepositorywith system fallback appears in bothscene_from_html_embed_path(lines 347-355) and here. This could be extracted to a helper function.♻️ Optional refactor
fn temp_font_repository_with_system_fallback() -> FontRepository { use cg::resources::ByteStore; use cg::runtime::font_repository::FontRepository; let mut repo = FontRepository::new( std::sync::Arc::new(std::sync::Mutex::new(ByteStore::new())) ); repo.enable_system_fallback(); repo }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-dev/src/main.rs` around lines 406 - 414, Extract the duplicated temp FontRepository creation into a helper function (e.g., temp_font_repository_with_system_fallback) that performs the ByteStore/FontRepository construction, wraps ByteStore in Arc<Mutex<_>>, calls repo.enable_system_fallback(), and returns the repo; then replace the inline blocks in scene_from_html_embed_path and the main.rs temp_fonts binding with calls to this helper (keep any required use/imports inside the new function to avoid changing other modules).crates/grida-canvas/src/htmlcss/mod.rs (1)
283-296: Make the table roundtrip test prove the faux-table layout.
assert!(pic.is_ok())only tells us the pipeline did not fail. A broken table mapping can still pass that check while stacking cells vertically. Please assert some layout property instead, such as distincttdX positions or widths, so this actually guards the new table behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-canvas/src/htmlcss/mod.rs` around lines 283 - 296, The test test_markdown_table currently only checks that render(&html, 600.0, 300.0, &fonts) returns Ok, which doesn't verify the faux-table layout; update the test to assert a concrete layout property from the rendered picture instead: after calling render and unwrapping pic, inspect the rendered layout (from the Picture/Frame returned by render) to locate the table cell boxes (e.g., elements corresponding to the first row's TDs) and assert their X positions or widths are distinct (for example assert td0.x < td1.x and td1.x < td2.x or assert widths differ), thereby proving horizontal placement rather than vertical stacking; reference the existing test_markdown_table, markdown_to_styled_html, and render to find where to extract those box metrics and add the assertions.crates/grida-canvas/src/painter/painter.rs (1)
2090-2155: Extract the shared embed rendering path.This arm now mirrors
PainterPictureLayer::HtmlEmbedalmost line-for-line. A small helper that takes the final HTML string would keep clipping, fallback, and future bug fixes from drifting between the two embed types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-canvas/src/painter/painter.rs` around lines 2090 - 2155, The MarkdownEmbed arm duplicates the HTML embed rendering logic; extract a helper method on Painter (e.g., render_html_embed or draw_embed_from_html) that accepts the final HTML string plus width and height and performs the clipping, picture rendering, and gray-rectangle fallback (using self.canvas, self.fonts, and the same clip/save/restore behavior). In PainterPictureLayer::MarkdownEmbed call crate::htmlcss::markdown_to_styled_html to get the HTML then delegate to the new helper; likewise refactor PainterPictureLayer::HtmlEmbed to call the same helper so both embed paths share clipping, render and fallback logic.
🤖 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/assets/css/grida-markdown.css`:
- Line 1: The file grida-markdown.css currently contains only a filesystem path
string instead of CSS rules, so replace that placeholder with valid CSS for the
markdown renderer (either paste the intended stylesheet rules or add a proper
`@import/`@use referencing the real CSS resource); ensure the file contains valid
selectors and properties needed by the renderer (e.g., base typography,
headings, code blocks, lists) and remove the path-only line so the renderer can
apply styles.
In `@crates/grida-canvas/src/htmlcss/mod.rs`:
- Around line 71-76: markdown_to_styled_html currently embeds raw HTML produced
by markdown_to_html (which uses html::push_html), allowing untrusted input via
pulldown-cmark Event::Html and Event::InlineHtml; update the markdown rendering
pipeline (in markdown_to_html or right before calling html::push_html) to
convert Event::Html and Event::InlineHtml into Event::Text (or otherwise
strip/sanitize them) so raw HTML/CSS is not emitted, or alternatively document
that MarkdownEmbed shares HTMLEmbed's trust model; reference
markdown_to_styled_html, markdown_to_html, html::push_html, and the
pulldown-cmark events Event::Html / Event::InlineHtml when making the change.
In `@crates/grida-dev/src/main.rs`:
- Around line 415-417: measure_content_height can return Ok(0.0) which your
current unwrap_or(600.0) does not catch, producing a zero-height node; update
the height calculation around cg::htmlcss::measure_content_height(&styled_html,
width, &temp_fonts) (and the local variable height/styled_html) to clamp the
result to a sensible minimum (e.g., assign the measured value then replace it
with a MIN_HEIGHT like 600.0 if it is 0.0 or use .max(MIN_HEIGHT)) so the final
height variable is never zero.
In `@fixtures/css/grida-markdown.css`:
- Line 71: The font-family declaration contains an unquoted multi-word family
name; update the declaration font-family: SF Mono, Menlo, Consolas, monospace;
(and the other occurrence noted) to quote the multi-word family name (e.g. "SF
Mono") so it satisfies stylelint's font-family-name-quotes rule and repeat the
same change at the other occurrence referenced in the comment.
---
Nitpick comments:
In `@crates/grida-canvas/src/htmlcss/mod.rs`:
- Around line 283-296: The test test_markdown_table currently only checks that
render(&html, 600.0, 300.0, &fonts) returns Ok, which doesn't verify the
faux-table layout; update the test to assert a concrete layout property from the
rendered picture instead: after calling render and unwrapping pic, inspect the
rendered layout (from the Picture/Frame returned by render) to locate the table
cell boxes (e.g., elements corresponding to the first row's TDs) and assert
their X positions or widths are distinct (for example assert td0.x < td1.x and
td1.x < td2.x or assert widths differ), thereby proving horizontal placement
rather than vertical stacking; reference the existing test_markdown_table,
markdown_to_styled_html, and render to find where to extract those box metrics
and add the assertions.
In `@crates/grida-canvas/src/layout/engine.rs`:
- Around line 2046-2053: Rename the test function
test_markdown_node_layout_preserves_size to
test_markdown_embed_node_layout_preserves_size for naming consistency with the
new node variant; update the function declaration and any references or test
attributes accordingly so it still uses nf.create_markdown_embed_node() and
Node::MarkdownEmbed(md) and preserves the same assertions and setup (e.g.,
md.size, md.transform, graph.append_child).
In `@crates/grida-canvas/src/painter/painter.rs`:
- Around line 2090-2155: The MarkdownEmbed arm duplicates the HTML embed
rendering logic; extract a helper method on Painter (e.g., render_html_embed or
draw_embed_from_html) that accepts the final HTML string plus width and height
and performs the clipping, picture rendering, and gray-rectangle fallback (using
self.canvas, self.fonts, and the same clip/save/restore behavior). In
PainterPictureLayer::MarkdownEmbed call crate::htmlcss::markdown_to_styled_html
to get the HTML then delegate to the new helper; likewise refactor
PainterPictureLayer::HtmlEmbed to call the same helper so both embed paths share
clipping, render and fallback logic.
In `@crates/grida-dev/src/main.rs`:
- Around line 406-414: Extract the duplicated temp FontRepository creation into
a helper function (e.g., temp_font_repository_with_system_fallback) that
performs the ByteStore/FontRepository construction, wraps ByteStore in
Arc<Mutex<_>>, calls repo.enable_system_fallback(), and returns the repo; then
replace the inline blocks in scene_from_html_embed_path and the main.rs
temp_fonts binding with calls to this helper (keep any required use/imports
inside the new function to avoid changing other modules).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 168c395f-35f9-400e-9312-d54edbf0efc5
📒 Files selected for processing (29)
crates/grida-canvas/assets/css/grida-markdown.csscrates/grida-canvas/examples/tool_io_grida.rscrates/grida-canvas/examples/tool_io_svg.rscrates/grida-canvas/src/cache/compositor/promotion.rscrates/grida-canvas/src/htmlcss/collect.rscrates/grida-canvas/src/htmlcss/faux_table.rscrates/grida-canvas/src/htmlcss/github_markdown.rscrates/grida-canvas/src/htmlcss/layout.rscrates/grida-canvas/src/htmlcss/mod.rscrates/grida-canvas/src/layout/engine.rscrates/grida-canvas/src/layout/into_taffy.rscrates/grida-canvas/src/node/factory.rscrates/grida-canvas/src/node/scene_graph.rscrates/grida-canvas/src/node/schema.rscrates/grida-canvas/src/painter/geometry.rscrates/grida-canvas/src/painter/layer.rscrates/grida-canvas/src/painter/markdown.rscrates/grida-canvas/src/painter/mod.rscrates/grida-canvas/src/painter/painter.rscrates/grida-canvas/src/painter/painter_debug_node.rscrates/grida-canvas/src/resources/mod.rscrates/grida-canvas/src/runtime/cost_prediction.rscrates/grida-canvas/src/runtime/scene.rscrates/grida-dev/src/bench/load_bench.rscrates/grida-dev/src/editor/document.rscrates/grida-dev/src/editor/mutation.rscrates/grida-dev/src/main.rsfixtures/css/README.mdfixtures/css/grida-markdown.css
💤 Files with no reviewable changes (2)
- crates/grida-canvas/src/painter/mod.rs
- crates/grida-canvas/src/painter/markdown.rs
| pub fn markdown_to_styled_html(markdown: &str) -> String { | ||
| let html_body = crate::io::io_markdown::markdown_to_html(markdown); | ||
| format!( | ||
| "<html><head><style>{}</style></head><body class=\"markdown-body\">{}</body></html>", | ||
| GITHUB_MARKDOWN_CSS, html_body | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does pulldown-cmark 0.13's html::push_html preserve raw HTML blocks and inline HTML by default, and what is the recommended way to disable or sanitize raw HTML when rendering markdown that should stay markdown-only?
💡 Result:
Yes, pulldown-cmark 0.13's html::push_html preserves raw HTML blocks (Event::Html) and inline HTML (Event::InlineHtml) by default, outputting them directly as raw HTML tags in the rendered output. This follows CommonMark specification compliance, where raw HTML is a standard feature parsed into dedicated events. To disable or sanitize raw HTML for markdown-only rendering: 1. Filter out HTML events before rendering: Transform Event::Html and Event::InlineHtml to Event::Text (escaping the HTML content) or skip them. Example from community sources: use pulldown_cmark::{Event, Parser}; let parser = Parser::new(input); let no_html = parser.into_iter.map(|event| match event { Event::Html(html) | Event::InlineHtml(html) => Event::Text(html.into), _ => event, }); pulldown_cmark::html::push_html(&mut output, no_html); This treats raw HTML as escaped text. 2. Post-render sanitization: Render with push_html, then sanitize the HTML output using a library like ammonia or html5ever to strip/escape dangerous tags. 3. Custom renderer: Implement a custom HTML renderer that skips or escapes Event::Html/Event::InlineHtml. No built-in flag exists to disable raw HTML parsing/rendering in pulldown-cmark, as it's core CommonMark behavior. For user-submitted content, filtering events or post-sanitization is recommended to prevent XSS.
Citations:
- 1: Disallowed Raw HTML (extension) pulldown-cmark/pulldown-cmark#608
- 2: https://docs.rs/pulldown-cmark/latest/pulldown_cmark/enum.Event.html
- 3: https://docs.rs/pulldown-cmark/latest/pulldown_cmark/
- 4: https://pulldown-cmark.github.io/pulldown-cmark/
- 5: https://github.com/raphlinus/pulldown-cmark
🏁 Script executed:
# Find and examine the markdown implementation
fd -t f "io_markdown.rs" crates/Repository: gridaco/grida
Length of output: 98
🏁 Script executed:
# Read the htmlcss/mod.rs file around the flagged lines
fd -t f "mod.rs" crates/grida-canvas/src/htmlcss/ --exec wc -l {} \;Repository: gridaco/grida
Length of output: 99
🏁 Script executed:
# Get the htmlcss mod.rs content
cat -n crates/grida-canvas/src/htmlcss/mod.rs | sed -n '60,90p'Repository: gridaco/grida
Length of output: 1390
🏁 Script executed:
# Get the io_markdown implementation
fd -t f "io_markdown.rs" crates/ --exec cat -n {} \;Repository: gridaco/grida
Length of output: 787
Sanitize raw HTML in markdown before rendering.
markdown_to_html() passes unfiltered output directly from html::push_html() to markdown_to_styled_html(), which embeds it in a complete HTML document with an inline <style> block. pulldown-cmark preserves raw HTML blocks and inline HTML by default (Event::Html and Event::InlineHtml). This allows arbitrary HTML/CSS injection through markdown input, collapsing the markdown feature boundary. Filter Event::Html and Event::InlineHtml to Event::Text before calling html::push_html(), or document that MarkdownEmbed shares HTMLEmbed's trust model and requires sanitized input.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-canvas/src/htmlcss/mod.rs` around lines 71 - 76,
markdown_to_styled_html currently embeds raw HTML produced by markdown_to_html
(which uses html::push_html), allowing untrusted input via pulldown-cmark
Event::Html and Event::InlineHtml; update the markdown rendering pipeline (in
markdown_to_html or right before calling html::push_html) to convert Event::Html
and Event::InlineHtml into Event::Text (or otherwise strip/sanitize them) so raw
HTML/CSS is not emitted, or alternatively document that MarkdownEmbed shares
HTMLEmbed's trust model; reference markdown_to_styled_html, markdown_to_html,
html::push_html, and the pulldown-cmark events Event::Html / Event::InlineHtml
when making the change.
| let styled_html = cg::htmlcss::markdown_to_styled_html(&md_source); | ||
| let height = | ||
| cg::htmlcss::measure_content_height(&styled_html, width, &temp_fonts).unwrap_or(600.0); |
There was a problem hiding this comment.
Potential zero-height scene when markdown/HTML produces no root element.
measure_content_height returns Ok(0.0) when the styled tree has no root element (see htmlcss/mod.rs:62). This would create a node with size: { width: 800.0, height: 0.0 }, which may render as invisible or cause layout issues.
Consider adding a minimum height fallback:
🛡️ Suggested fix
let height =
- cg::htmlcss::measure_content_height(&styled_html, width, &temp_fonts).unwrap_or(600.0);
+ cg::htmlcss::measure_content_height(&styled_html, width, &temp_fonts)
+ .unwrap_or(600.0)
+ .max(50.0); // Ensure minimum visible height🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-dev/src/main.rs` around lines 415 - 417, measure_content_height
can return Ok(0.0) which your current unwrap_or(600.0) does not catch, producing
a zero-height node; update the height calculation around
cg::htmlcss::measure_content_height(&styled_html, width, &temp_fonts) (and the
local variable height/styled_html) to clamp the result to a sensible minimum
(e.g., assign the measured value then replace it with a MIN_HEIGHT like 600.0 if
it is 0.0 or use .max(MIN_HEIGHT)) so the final height variable is never zero.
| /* ── Inline code ── */ | ||
|
|
||
| .markdown-body code { | ||
| font-family: SF Mono, Menlo, Consolas, monospace; |
There was a problem hiding this comment.
Quote multi-word font family names to satisfy stylelint.
SF Mono should be quoted in font-family declarations to pass font-family-name-quotes.
Proposed fix
-.markdown-body code {
- font-family: SF Mono, Menlo, Consolas, monospace;
+.markdown-body code {
+ font-family: "SF Mono", Menlo, Consolas, monospace;
@@
-.markdown-body pre code {
- font-family: SF Mono, Menlo, Consolas, monospace;
+.markdown-body pre code {
+ font-family: "SF Mono", Menlo, Consolas, monospace;Also applies to: 90-90
🧰 Tools
🪛 Stylelint (17.6.0)
[error] 71-71: Expected quotes around "SF Mono" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fixtures/css/grida-markdown.css` at line 71, The font-family declaration
contains an unquoted multi-word family name; update the declaration font-family:
SF Mono, Menlo, Consolas, monospace; (and the other occurrence noted) to quote
the multi-word family name (e.g. "SF Mono") so it satisfies stylelint's
font-family-name-quotes rule and repeat the same change at the other occurrence
referenced in the comment.
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/src/painter/painter.rs`:
- Around line 2137-2153: In the Err(_) branch of the render path in painter.rs
where the gray-rectangle fallback is drawn (the block that creates SkPaint and
calls self.canvas.draw_rect with Rect::from_xywh(md_layer.width,
md_layer.height)), capture the actual error instead of discarding it and emit a
debug/warn log entry including the error details before drawing the fallback;
use the project’s logging facility (or introduce the log crate) and format a
concise message like "render layer failed" with the error and layer id/size
context, then continue to draw the existing gray fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d46811dd-400d-4ec2-8f29-d1d133c4c12d
📒 Files selected for processing (3)
crates/grida-canvas/src/htmlcss/github_markdown.rscrates/grida-canvas/src/htmlcss/mod.rscrates/grida-canvas/src/painter/painter.rs
✅ Files skipped from review due to trivial changes (1)
- crates/grida-canvas/src/htmlcss/github_markdown.rs
| Err(_) => { | ||
| // Render error fallback: gray rectangle | ||
| let mut paint = SkPaint::default(); | ||
| paint.set_color(skia_safe::Color::from_rgb( | ||
| 200, 200, 200, | ||
| )); | ||
| paint.set_style(skia_safe::PaintStyle::Fill); | ||
| self.canvas.draw_rect( | ||
| Rect::from_xywh( | ||
| 0.0, | ||
| 0.0, | ||
| md_layer.width, | ||
| md_layer.height, | ||
| ), | ||
| &paint, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Consider logging the render error for debugging.
The error is silently discarded, making it difficult to diagnose rendering failures. Consider logging the error message at debug/warn level before showing the fallback.
🛠️ Suggested improvement
- Err(_) => {
- // Render error fallback: gray rectangle
+ Err(e) => {
+ // Render error fallback: gray rectangle
+ #[cfg(debug_assertions)]
+ eprintln!("MarkdownEmbed render error: {e}");
let mut paint = SkPaint::default();Alternatively, use a proper logging crate if available in the project.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-canvas/src/painter/painter.rs` around lines 2137 - 2153, In the
Err(_) branch of the render path in painter.rs where the gray-rectangle fallback
is drawn (the block that creates SkPaint and calls self.canvas.draw_rect with
Rect::from_xywh(md_layer.width, md_layer.height)), capture the actual error
instead of discarding it and emit a debug/warn log entry including the error
details before drawing the fallback; use the project’s logging facility (or
introduce the log crate) and format a concise message like "render layer failed"
with the error and layer id/size context, then continue to draw the existing
gray fallback.
1 participant
Summary
painter/markdown.rswith the sharedhtmlcssrender pipeline — markdown is now converted to HTML via pulldown-cmark, wrapped withgrida-markdown.css, and rendered through Stylo CSS → Taffy layout → Skia paint (same path asHTMLEmbedNode)display: tablevia Taffy flexbox so GFM tables render with cells side-by-side instead of vertically stackedMarkdownNodeRec→MarkdownEmbedNodeRecto align withHTMLEmbedNodeRecnamingTest plan
cargo check -p cg -p grida-canvas-wasm -p grida-dev— all 3 crates compilecargo test -p cg -- htmlcss— 13 tests pass (including 4 new markdown roundtrip tests).mdfile in grida-dev and verify rendering (headings, tables, code blocks, lists, blockquotes).htmlfile in embed mode and verify it still renders correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation