feat(htmlcss): CSS Grid layout + box-shadow support#635
Merged
softmarshmallow merged 2 commits intomainfrom Apr 7, 2026
Merged
feat(htmlcss): CSS Grid layout + box-shadow support#635softmarshmallow merged 2 commits intomainfrom
softmarshmallow merged 2 commits intomainfrom
Conversation
Enable CSS Grid in the htmlcss direct-paint pipeline by wiring Stylo's grid properties through to Taffy's grid layout engine. - Enable Stylo's `layout.grid.enabled` pref (servo mode gates grid parsing behind this flag, defaulting to false) - Add grid IR types: GridAutoFlow, TrackBreadth, TrackSize, RepeatCount, GridTemplateEntry, GridPlacement - Collect grid container props from Stylo: grid-template-columns/rows, grid-auto-columns/rows, grid-auto-flow - Collect grid child placement: grid-column-start/end, grid-row-start/end - Convert IR to Taffy grid styles: track sizing functions (px, %, fr, minmax, fit-content), repeat(count/auto-fill/auto-fit), GridPlacement - Add 6 tests covering fixed columns, fr units, repeat(), span, dense auto-flow, and a structural layout assertion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Collect box-shadow from Stylo (offset, blur, spread, color, inset) via clone_box_shadow() on ComputedValues - Implement inset box-shadow painting using clip + EvenOdd PathBuilder frame (outer rect minus inner rect, blurred) - Multiple shadows (outer + inset) now render correctly - Add 4 tests: outer, inset, combined, and property collection assertion - Update docs to reflect full box-shadow support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughThis PR adds CSS Grid layout and enhanced box-shadow support to grida-canvas by integrating Stylo-based grid property parsing. Grid template properties, placement rules, and auto-flow behavior are extracted during style collection and mapped to Taffy layout computation. Inset box-shadow rendering is now fully implemented. New semantic types model grid concepts, and the StyledElement type is extended with grid-specific fields. Changes
Sequence Diagram(s)sequenceDiagram
participant HTML as HTML Input
participant Collect as Style Collection<br/>(Stylo)
participant Type as Type System<br/>(GridTemplateEntry,<br/>GridPlacement, etc.)
participant Layout as Layout Mapping<br/>(Taffy)
participant Paint as Paint Render<br/>(Canvas)
HTML->>Collect: Parse & compute styles
activate Collect
Collect->>Type: Extract grid-template-*<br/>grid-auto-flow<br/>grid-column/row
Type-->>Collect: GridTemplateEntry<br/>GridAutoFlow<br/>GridPlacement
Collect-->>Collect: Populate StyledElement<br/>grid fields
deactivate Collect
Collect->>Layout: Styled tree with<br/>grid properties
activate Layout
Layout->>Layout: Convert grid templates<br/>& placements to Taffy
Layout->>Layout: Map TrackSize,<br/>RepeatCount to<br/>Taffy grid format
Layout-->>Layout: Populate Taffy style<br/>grid properties
deactivate Layout
Layout->>Paint: Computed layout with<br/>grid cell positions
activate Paint
Paint->>Paint: Render grid cells<br/>with box-shadow
Paint->>Paint: For inset shadows:<br/>clip to bounds +<br/>EvenOdd hollow frame
Paint-->>HTML: Rendered grid layout
deactivate Paint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 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 |
![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
🤖 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/htmlcss/collect.rs`:
- Around line 1000-1003: The match arm for RepeatCount::Number currently narrows
rep.count with an unchecked `as` cast which can wrap large values; clamp the
integer to the valid range before converting (e.g. clamp to 0..=u16::MAX) and
then construct types::RepeatCount::Count with the clamped value; apply the same
clamping approach to the other similar casts around RepeatCount handling
(including the other occurrence referenced at lines ~1089-1095) so no
out-of-range repeat/grid integers wrap when narrowed.
- Around line 957-962: The code currently falls back to CGColor::BLACK when
s.base.color.as_absolute() is absent; change this to use the element's computed
text color (CSS currentColor) instead. Locate the assignment to color (the chain
s.base.color.as_absolute().map(|a|
abs_color_to_cg(a)).unwrap_or(CGColor::BLACK)) and replace the unwrap_or
fallback so it converts the element's computed text/current color via
abs_color_to_cg (i.e., obtain the element's computed foreground/text color from
the style object and use that as the fallback) rather than using CGColor::BLACK.
In `@crates/grida-canvas/src/htmlcss/paint.rs`:
- Around line 417-423: The inset cutout creation using Rect::from_xywh with
shadow.spread can produce negative width/height when spread > half of w/h; clamp
the computed inner width and height to a minimum of 0 (e.g. inner_w = (w -
spread*2.0).max(0.0), inner_h = (h - spread*2.0).max(0.0)) and adjust the
inner_rect origin accordingly (so it doesn't invert), and if either clamped
dimension is zero skip adding the hole entirely; apply the same guard to the
second cutout block around the code at the other occurrence (the block using
inner_rect and spread at lines ~439-452) so neither will produce inverted Rects
or add collapsed holes.
In `@docs/wg/feat-2d/htmlcss.md`:
- Around line 77-98: The two grid summaries conflict: the top row claims
`display: grid` is "full property support" while the `### Grid Layout` table
marks `grid-template-areas`, "Named grid lines", and `subgrid` as unsupported;
consolidate these into a single accurate summary by updating the `display: grid`
row and the Grid Layout table to reflect the true supported subset (keep
`grid-template-columns/rows`, `grid-auto-*`, `grid-column/grid-row`, and
`repeat(auto-*)` as ✅; mark `grid-template-areas`, Named grid lines, and
`subgrid` as ❌), remove the contradictory "full property support" phrasing, and
ensure both the top-level `display: grid` entry and the detailed `### Grid
Layout` table match exactly.
🪄 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: a8fd884f-1895-4b2b-a65f-b8183f147478
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
crates/grida-canvas/Cargo.tomlcrates/grida-canvas/src/htmlcss/collect.rscrates/grida-canvas/src/htmlcss/layout.rscrates/grida-canvas/src/htmlcss/mod.rscrates/grida-canvas/src/htmlcss/paint.rscrates/grida-canvas/src/htmlcss/style.rscrates/grida-canvas/src/htmlcss/types.rsdocs/wg/feat-2d/htmlcss.md
Comment on lines
+957
to
+962
| let color = s | ||
| .base | ||
| .color | ||
| .as_absolute() | ||
| .map(|a| abs_color_to_cg(a)) | ||
| .unwrap_or(CGColor::BLACK); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the file structure and locate the relevant code
wc -l crates/grida-canvas/src/htmlcss/collect.rsRepository: gridaco/grida
Length of output: 104
🏁 Script executed:
# Now let's read the code around lines 957-962 with more context
sed -n '940,980p' crates/grida-canvas/src/htmlcss/collect.rs | cat -nRepository: gridaco/grida
Length of output: 1533
🏁 Script executed:
# Let's also check what extract_box_shadow function looks like
sed -n '900,1000p' crates/grida-canvas/src/htmlcss/collect.rs | cat -nRepository: gridaco/grida
Length of output: 4073
🏁 Script executed:
# Search for other usages of get_inherited_text().color to understand the pattern
rg "get_inherited_text\(\)\.color" crates/grida-canvas/src/htmlcss/collect.rs -B 3 -A 3Repository: gridaco/grida
Length of output: 272
🏁 Script executed:
# Check if there's a function signature and where this code lives
sed -n '900,970p' crates/grida-canvas/src/htmlcss/collect.rs | head -80 | cat -nRepository: gridaco/grida
Length of output: 2501
🏁 Script executed:
# Let's check if there are other similar color fallback patterns in the code
rg "unwrap_or\(CGColor::" crates/grida-canvas/src/htmlcss/collect.rs -B 2 -A 2Repository: gridaco/grida
Length of output: 1144
🏁 Script executed:
# Check the type of style.get_inherited_text().color to ensure it's compatible
rg "get_inherited_text" crates/grida-canvas/src/htmlcss/collect.rs -B 5 -A 5Repository: gridaco/grida
Length of output: 650
🌐 Web query:
CSS box-shadow color default currentColor specification
💡 Result:
In the CSS Backgrounds and Borders Module Level 3 spec, each box-shadow item is a <shadow> that includes an optional <color>. If the color is omitted, it defaults to currentColor. [1]
currentColor is the keyword that resolves to the element’s computed color value (i.e., the current text color), so an unspecified box-shadow color will match the element’s color. [2][3]
Sources: W3C spec for box-shadow and <shadow> grammar/defaults [1], MDN box-shadow docs [2], MDN <color> / currentColor reference [3].
Change box-shadow color fallback from black to currentColor.
Per the CSS Backgrounds and Borders spec, box-shadow color defaults to currentColor (the element's text color) when omitted or unresolved. Falling back to black renders these shadows with incorrect color.
Suggested fix
let color = s
.base
.color
.as_absolute()
.map(|a| abs_color_to_cg(a))
- .unwrap_or(CGColor::BLACK);
+ .unwrap_or_else(|| abs_color_to_cg(&style.get_inherited_text().color));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-canvas/src/htmlcss/collect.rs` around lines 957 - 962, The code
currently falls back to CGColor::BLACK when s.base.color.as_absolute() is
absent; change this to use the element's computed text color (CSS currentColor)
instead. Locate the assignment to color (the chain
s.base.color.as_absolute().map(|a|
abs_color_to_cg(a)).unwrap_or(CGColor::BLACK)) and replace the unwrap_or
fallback so it converts the element's computed text/current color via
abs_color_to_cg (i.e., obtain the element's computed foreground/text color from
the style object and use that as the fallback) rather than using CGColor::BLACK.
Comment on lines
+1000
to
+1003
| let count = match rep.count { | ||
| RepeatCount::Number(n) => types::RepeatCount::Count(n as u16), | ||
| RepeatCount::AutoFill => types::RepeatCount::AutoFill, | ||
| RepeatCount::AutoFit => types::RepeatCount::AutoFit, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine the relevant lines
wc -l crates/grida-canvas/src/htmlcss/collect.rsRepository: gridaco/grida
Length of output: 104
🏁 Script executed:
# Read the code around the first issue (lines 1000-1003)
sed -n '995,1010p' crates/grida-canvas/src/htmlcss/collect.rsRepository: gridaco/grida
Length of output: 1034
🏁 Script executed:
# Read the code around the second issue (lines 1089-1095)
sed -n '1080,1100p' crates/grida-canvas/src/htmlcss/collect.rsRepository: gridaco/grida
Length of output: 790
🏁 Script executed:
# Search for other similar unsafe casts in the file
rg 'as (u16|i16|u32|i32)' crates/grida-canvas/src/htmlcss/collect.rs -n -B2 -A2Repository: gridaco/grida
Length of output: 1091
Clamp Stylo's grid integers before narrowing them.
Rust's as cast wraps/truncates out-of-range integers. A large repeat() count or grid-column: 40000 will silently turn into an unrelated u16/i16 value instead of saturating or being rejected.
Suggested fix
- RepeatCount::Number(n) => types::RepeatCount::Count(n as u16),
+ RepeatCount::Number(n) => {
+ types::RepeatCount::Count(n.clamp(1, u16::MAX as i32) as u16)
+ }
...
- let n = line.line_num.unsigned_abs() as u16;
+ let n = line.line_num.unsigned_abs().min(u16::MAX as u32) as u16;
return types::GridPlacement::Span(if n == 0 { 1 } else { n });
}
let num = line.line_num;
if num != 0 {
- return types::GridPlacement::Line(num as i16);
+ return types::GridPlacement::Line(num.clamp(i16::MIN as i32, i16::MAX as i32) as i16);
}Also applies to: 1089-1095
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-canvas/src/htmlcss/collect.rs` around lines 1000 - 1003, The
match arm for RepeatCount::Number currently narrows rep.count with an unchecked
`as` cast which can wrap large values; clamp the integer to the valid range
before converting (e.g. clamp to 0..=u16::MAX) and then construct
types::RepeatCount::Count with the clamped value; apply the same clamping
approach to the other similar casts around RepeatCount handling (including the
other occurrence referenced at lines ~1089-1095) so no out-of-range repeat/grid
integers wrap when narrowed.
Comment on lines
+417
to
+423
| let spread = shadow.spread; | ||
| let inner_rect = Rect::from_xywh( | ||
| shadow.offset_x + spread, | ||
| shadow.offset_y + spread, | ||
| w - spread * 2.0, | ||
| h - spread * 2.0, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code at lines 417-423 and surrounding context
wc -l crates/grida-canvas/src/htmlcss/paint.rsRepository: gridaco/grida
Length of output: 101
🏁 Script executed:
# Read the relevant section around lines 374-457
sed -n '374,457p' crates/grida-canvas/src/htmlcss/paint.rs | cat -nRepository: gridaco/grida
Length of output: 4071
🏁 Script executed:
# Also check the complete context of the function to understand the flow
sed -n '350,480p' crates/grida-canvas/src/htmlcss/paint.rs | cat -nRepository: gridaco/grida
Length of output: 5878
🌐 Web query:
skia-safe Rect::from_xywh negative width height behavior documentation
💡 Result:
In skia-safe, Rect::from_xywh(x, y, w, h) does not reject or normalize negative w / h: it literally constructs (left, top, right, bottom) as (x, y, x + w, y + h). So if w < 0 then right < left, and if h < 0 then bottom < top. [1]
That matches upstream Skia’s SkRect::MakeXYWH(x, y, w, h) implementation ({x, y, x + w, y + h}), i.e. negative sizes produce an unsorted rectangle. [2]
Consequences:
width()/height()become negative (since they areright - left/bottom - top). [3]is_empty()becomestruefor negative width/height, because Skia considers a rect non-empty only ifleft < right && top < bottom. [3]- If you want a canonical “positive-size” rectangle, call
rect.sort()(in-place) orrect.sorted()(returns a sorted copy), which swap edges as needed. [4]
Sources
- skia-safe
Rect::from_xywhimplementation (Rust) (rust-skia.github.io) - Skia
SkRect::MakeXYWHimplementation (C++) (api.skia.org) - Skia
SkRect::isEmpty,width,height(C++) (api.skia.org) - skia-safe
Rect::sort/Rect::sortedimplementations (Rust) (rust-skia.github.io)
Citations:
- 1: https://rust-skia.github.io/doc/src/skia_safe/core/rect.rs.html
- 2: https://api.skia.org/SkRect_8h_source.html
- 3: https://api.skia.org/SkRect_8h_source.html
- 4: https://rust-skia.github.io/doc/src/skia_safe/core/rect.rs.html
Guard the inset cutout against collapsed dimensions.
When a positive spread exceeds half the box dimensions, the inner cutout becomes negative-sized. Skia creates an inverted rectangle (left > right or top > bottom) in this case, producing unpredictable rendering with the EvenOdd fill rule. Examples like inset 0 0 0 9999px will render incorrectly. Clamp the cutout dimensions to non-negative and skip adding the hole when it collapses.
Suggested fix
let spread = shadow.spread;
- let inner_rect = Rect::from_xywh(
- shadow.offset_x + spread,
- shadow.offset_y + spread,
- w - spread * 2.0,
- h - spread * 2.0,
- );
+ let inner_width = (w - spread * 2.0).max(0.0);
+ let inner_height = (h - spread * 2.0).max(0.0);
+ let has_cutout = inner_width > 0.0 && inner_height > 0.0;
+ let inner_rect = Rect::from_xywh(
+ shadow.offset_x + spread,
+ shadow.offset_y + spread,
+ inner_width,
+ inner_height,
+ );
...
- if r.is_zero() {
+ if has_cutout && r.is_zero() {
builder.add_rect(inner_rect, None, None);
- } else {
+ } else if has_cutout {
// Shrink corner radii by spread for the inner cutout
let shrink = spread.max(0.0);
let inner_radii = [
skia_safe::Point::new((r.tl_x - shrink).max(0.0), (r.tl_y - shrink).max(0.0)),
skia_safe::Point::new((r.tr_x - shrink).max(0.0), (r.tr_y - shrink).max(0.0)),
skia_safe::Point::new((r.br_x - shrink).max(0.0), (r.br_y - shrink).max(0.0)),
skia_safe::Point::new((r.bl_x - shrink).max(0.0), (r.bl_y - shrink).max(0.0)),
];
let mut inner_rrect = skia_safe::RRect::new();
inner_rrect.set_rect_radii(inner_rect, &inner_radii);
builder.add_rrect(&inner_rrect, None, None);
}Also applies to: 439-452
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/grida-canvas/src/htmlcss/paint.rs` around lines 417 - 423, The inset
cutout creation using Rect::from_xywh with shadow.spread can produce negative
width/height when spread > half of w/h; clamp the computed inner width and
height to a minimum of 0 (e.g. inner_w = (w - spread*2.0).max(0.0), inner_h = (h
- spread*2.0).max(0.0)) and adjust the inner_rect origin accordingly (so it
doesn't invert), and if either clamped dimension is zero skip adding the hole
entirely; apply the same guard to the second cutout block around the code at the
other occurrence (the block using inner_rect and spread at lines ~439-452) so
neither will produce inverted Rects or add collapsed holes.
Comment on lines
+77
to
+98
| | `display: grid` | ✅ | Via Taffy `Display::Grid` — full property support | | ||
| | `display: list-item` | ✅ | Marker text generated (bullet/number) | | ||
| | `display: table` | ⚠️ | Falls back to block flow (no column grid) | | ||
| | `display: inline-block` | ⚠️ | Treated as inline | | ||
|
|
||
| ### Grid Layout | ||
|
|
||
| | CSS Property | Status | Notes | | ||
| | ------------------------- | ------ | -------------------------------------------------- | | ||
| | `grid-template-columns` | ✅ | px, %, fr, minmax(), fit-content(), repeat() | | ||
| | `grid-template-rows` | ✅ | px, %, fr, minmax(), fit-content(), repeat() | | ||
| | `grid-auto-columns` | ✅ | Implicit track sizing | | ||
| | `grid-auto-rows` | ✅ | Implicit track sizing | | ||
| | `grid-auto-flow` | ✅ | row, column, dense | | ||
| | `grid-column` (start/end) | ✅ | Line numbers, span | | ||
| | `grid-row` (start/end) | ✅ | Line numbers, span | | ||
| | `repeat(auto-fill, ...)` | ✅ | Via Taffy | | ||
| | `repeat(auto-fit, ...)` | ✅ | Via Taffy | | ||
| | `grid-template-areas` | ❌ | Not collected from Stylo (named areas not mapped) | | ||
| | Named grid lines | ❌ | Line names ignored; numeric placement only | | ||
| | `subgrid` | ❌ | Taffy does not support subgrid | | ||
|
|
There was a problem hiding this comment.
This grid section now contradicts the rest of the page.
It says display: grid has “full property support”, but this same section still marks grid-template-areas, named lines, and subgrid as unsupported, and the later detailed ### Grid Layout table still shows most grid properties as ❌. Please consolidate the two tables and tone this down to the supported subset.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/wg/feat-2d/htmlcss.md` around lines 77 - 98, The two grid summaries
conflict: the top row claims `display: grid` is "full property support" while
the `### Grid Layout` table marks `grid-template-areas`, "Named grid lines", and
`subgrid` as unsupported; consolidate these into a single accurate summary by
updating the `display: grid` row and the Grid Layout table to reflect the true
supported subset (keep `grid-template-columns/rows`, `grid-auto-*`,
`grid-column/grid-row`, and `repeat(auto-*)` as ✅; mark `grid-template-areas`,
Named grid lines, and `subgrid` as ❌), remove the contradictory "full property
support" phrasing, and ensure both the top-level `display: grid` entry and the
detailed `### Grid Layout` table match exactly.
3 tasks
Merged
Merged
4 tasks
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.
Summary
grid-template-columns/rows(px, %, fr, minmax, fit-content, repeat),grid-auto-columns/rows,grid-auto-flow(row/column/dense), andgrid-column/rowplacement (line numbers, span).box-shadowfrom Stylo'sComputedValues(offset, blur, spread, color, inset flag).layout.grid.enabledin Stylo's servo mode — without this,display: gridis silently ignored.Test plan
cargo check -p cg -p grida-canvas-wasm -p grida-dev— all crates compilecargo test -p cg -- htmlcss::tests— 23 tests pass (10 new: 6 grid + 4 box-shadow)cargo run --example golden_htmlcssongrid-template.htmlandpaint-shadow.htmlfixtures🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation