feat: support sub grid feature and integration tests. #789
Conversation
- Marked Phase 9 ✅ complete and removed the “deferred/future” roadmap (CSS_GRID_PLAN.md:708) - Added Phase 9 file/test breakdown (14 files, 68 tests) and updated totals to 91 files / 597 tests (CSS_GRID_PLAN.md:209) - Updated coverage tables + success metrics and bumped “Last Updated” to 2026-01-06 (CSS_GRID_PLAN.md:101)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds full CSS Grid Subgrid support: parsing/serialization (GridSubgrid), rendering/layout integration with subgrid contexts and intrinsic contributions, computed-style serialization, and 14 new integration test suites covering subgrid behavior across many scenarios. Changes
Sequence DiagramsequenceDiagram
participant Dev as CSS input / Tests
participant Parser as grid.dart (Parser)
participant Model as GridSubgrid (css model)
participant Serializer as computed_style_declaration.dart
participant Renderer as rendering/grid.dart
participant Layout as Final Layout
Dev->>Parser: parse "grid-template-(columns|rows): subgrid [line-names]?"
activate Parser
Parser->>Model: create GridSubgrid(lineNameGroups)
deactivate Parser
Model->>Serializer: serialize computed style
activate Serializer
Serializer->>Dev: output "subgrid [line-names]"
deactivate Serializer
Dev->>Renderer: attach element with subgrid tracks
activate Renderer
Renderer->>Renderer: _setSubgridParentContext(columns/rows)
Renderer->>Renderer: _configureSubgridContextForGridItem()
Renderer->>Renderer: resolve track sizing & intrinsic contributions
Renderer->>Renderer: place items respecting subgrid tracks/gaps/named lines
Renderer->>Layout: produce positioned boxes
deactivate Renderer
Layout-->>Dev: rendered snapshot / measurements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom Pre-merge Checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)webf/**/*.dart📄 CodeRabbit inference engine (webf/CLAUDE.md)
Files:
{bridge/**/*.{cc,h},webf/**/*.dart}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (10)📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
📚 Learning: 2025-12-13T16:32:47.644ZApplied to files:
📚 Learning: 2025-12-08T23:28:00.818ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (1)
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: 1
🤖 Fix all issues with AI Agents
In @integration_tests/specs/css/css-grid/subgrid/basic.ts:
- Line 8: The test sets grid.style.width = '400px' which exceeds the testing
window max of 340px; update the assignment in the test to use a width <= '340px'
(e.g., '340px' or smaller) and, if needed, adjust the gridTemplateColumns values
in the same test (references: grid.style.width and
grid.style.gridTemplateColumns) so the column definitions still fit and preserve
the test's intent.
🧹 Nitpick comments (7)
integration_tests/specs/css/css-grid/subgrid/alignment.ts (1)
122-125: Consider removing unused variables.Lines 123-125 retrieve bounding rectangles but only
items[1]is used in the subsequent assertion. Variablesitem1RectandsubgridRectare never referenced.🔎 Proposed cleanup
- const items = Array.from(subgrid.children) as HTMLElement[]; - const item1Rect = items[0].getBoundingClientRect(); - const item2Rect = items[1].getBoundingClientRect(); - const subgridRect = subgrid.getBoundingClientRect(); - - expect(items[1].getBoundingClientRect().width).toBe(60); + const items = Array.from(subgrid.children) as HTMLElement[]; + expect(items[1].getBoundingClientRect().width).toBe(60);webf/lib/src/css/grid.dart (1)
533-549: Consider stricter token boundary validation for thesubgridkeyword.The check at line 534 uses
trimmed.toLowerCase().startsWith('subgrid'), which could theoretically match strings like"subgridfunction"or"subgrid-custom". While the subsequent line-name token validation (lines 543-544) correctly rejects these cases by returning an empty list, it would be more explicit and spec-compliant to verify that"subgrid"is a complete token—either followed by whitespace or at the end of the input.Example: Add a check like:
final int endIdx = 7; if (trimmed.length > 7 && !trimmed[7].contains(RegExp(r'\s'))) { // "subgrid" is not a complete token return _parseTrackListInternal(trimmed, renderStyle, propertyName, axis); }This makes the intent clearer and aligns with CSS tokenization rules, though the current implementation works correctly in practice due to downstream validation.
integration_tests/specs/css/css-grid/subgrid/abspos.ts (1)
54-104: Add a comment explaining why this test is skipped.The test is skipped using
xit()without any explanation. Adding a comment would help other developers understand whether this is a known limitation, a work-in-progress feature, or a test that needs fixing.🔎 Suggested comment format
+ // TODO: This test is skipped because [reason - e.g., "grid-column placement + // for absolutely positioned items in subgrids is not yet supported"] xit('absolutely positioned item with grid-column placement in subgrid', async () => {Based on coding guidelines.
integration_tests/specs/css/css-grid/subgrid/mixed-tracks.ts (1)
1-254: Comprehensive mixed-track subgrid coverage; consider staying within 340px where possibleThese six tests exercise subgrid behavior with fixed+fr, minmax+auto, intrinsic+fr, repeat patterns, fit-content, and complex mixed tracks. They:
- Use async/await with
waitForFrame()andsnapshot(),- Clean up via
parent.remove(),- Assert widths via
getBoundingClientRect()in a way that matches the configured tracks.The only minor nit is that some parents use explicit widths (400–500px), while the harness notes a 340px max window width. Layout will still work (overflow is fine), but if you see snapshot cropping, you might want to tune these widths down closer to 340px in a follow-up.
webf/lib/src/rendering/grid.dart (3)
43-55: Subgrid axis context plumbing is sound; add a brief comment for future maintainersThe
_SubgridAxisContextstruct plus_pendingSubgridColumns/_pendingSubgridRowsand_setSubgridParentContext/_takeSubgrid*Contextgive a clear, single-consumer handoff from parent grid to subgrid._configureSubgridContextForGridItemcorrectly:
- Detects
grid-template-rows/columns: subgrid,- Slices the parent track sizes and line-name maps by the item’s span, and
- Marks when row sizes are not yet definite so a second layout pass can use final heights.
This design looks correct and contained. A short doc comment on
_setSubgridParentContextexplaining that it must be called by the parent grid just before laying out a subgrid child would make the ownership/ordering constraints obvious when someone revisits this code.Also applies to: 113-148, 264-323
2599-2719: Intrinsic width contribution logic (including nested subgrids) is correct but denseThe
resolveIntrinsicWidthContribution/applyIntrinsicWidthContributionToColumnspair, together with the nested-subgrid path, implement:
- Per-item min/max intrinsic width contributions (respecting
white-space,min-width,width: min/max-content, and aspect-ratio),- Distribution of those contributions across auto/min-content/max-content and flex tracks,
- Special handling for
fit-content()andminmax(<intrinsic>, <fixed>)range tracks, and- A second, inner auto-placement run to derive per-column intrinsic contributions for
subgridcolumns, including padding/border insets and sliced line-name maps.Semantically this lines up with how browsers size auto/content-based and flex tracks, and the nested-subgrid branch correctly forwards contributions from subgrid items up into the parent’s columns rather than treating the subgrid container as opaque.
Given how much logic is embedded in this one block, you might consider extracting these helpers (and possibly the nested-subgrid intrinsic loop) into a small internal helper class or separate private methods to keep
_performGridLayoutmore navigable. Behaviorally, though, it looks sound.Also applies to: 2726-2890, 3027-3230
3607-3766: Row-size propagation from subgrids and relayout flags are reasonable; minor clarity tweak possibleThe new flow:
- Tracks whether any subgrid row context had non-definite sizes via
relayoutForSubgridRows,- Captures per-row contributions from subgrid children into
_subgridRowSizeContributionson the subgrid,- Lets the parent, during Pass 2, either:
- Consume those contributions into
implicitRowHeights(when the child is asubgridin rows), or- Fall back to the existing marginBox-based spanning logic, and
- Forces a second child relayout pass when either implicit-row stretch or subgrid rows require updated constraints, rewiring contexts via
_configureSubgridContextForGridItembefore the secondlayout().This should avoid double-counting contributions (via
skipSelfRowContribution) and give subgrid rows a chance to pick up final heights without perturbing non-subgrid grids.The only suggestion is to add a short comment near
relayoutForSubgridRowsexplaining that it is used solely to trigger the second-pass child layout with final row sizes, and that the first pass is responsible for populating the parent’s row/implicit heights from subgrid contributions.Also applies to: 4172-4255, 4436-4445
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (72)
integration_tests/snapshots/css/css-grid/grid-definition/template-areas-advanced.ts.790ccafa2.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/grid-definition/template-areas-advanced.ts.bfcda8ab1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/placement/area-based.ts.20700f2d1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/abspos.ts.844e9ce31.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/abspos.ts.9cc463101.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/abspos.ts.e4bf40821.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/alignment.ts.706783171.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/alignment.ts.7c6d606a1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/alignment.ts.8874d64c1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/alignment.ts.d7ffb7ea1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/auto-placement.ts.503709cf1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/auto-placement.ts.5ceaf30b1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/auto-placement.ts.7e8b75f21.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/auto-placement.ts.972d30c01.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/axis-combinations.ts.4502d2f61.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/axis-combinations.ts.58ac4ee41.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/axis-combinations.ts.873f9db01.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/axis-combinations.ts.9156da731.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/axis-combinations.ts.b1d919721.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/axis-combinations.ts.b2fbd7bd1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic-inheritance.ts.189290721.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic-inheritance.ts.434ce9411.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic-inheritance.ts.651b3ae01.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic-inheritance.ts.ab8dc5a81.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic-inheritance.ts.cfd039121.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic-inheritance.ts.dccd66681.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic.ts.978250541.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/basic.ts.f94b85c51.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/explicit-placement.ts.339e0d361.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/explicit-placement.ts.6dafdb201.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/explicit-placement.ts.aba104b01.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/explicit-placement.ts.d4f55b061.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/gap-inheritance.ts.11a28ad91.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/gap-inheritance.ts.1d4f65861.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/gap-inheritance.ts.2cde28b31.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/gap-inheritance.ts.8fb5465e1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/gap-inheritance.ts.a28a34c71.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/gap-inheritance.ts.b6b1a7a71.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/intrinsic-sizing.ts.37f5d62f1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/intrinsic-sizing.ts.42f0e6ed1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/intrinsic-sizing.ts.806bcfba1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/intrinsic-sizing.ts.c996c5521.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/intrinsic-sizing.ts.e074960f1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/intrinsic-sizing.ts.f768ec3c1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/mixed-tracks.ts.41c3e35c1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/mixed-tracks.ts.bc24fdd61.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/mixed-tracks.ts.cb0702dc1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/mixed-tracks.ts.cf169f9e1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/mixed-tracks.ts.e140b0ad1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/mixed-tracks.ts.e9c2cf981.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/named-lines.ts.9b07aaee1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/named-lines.ts.a98fb59b1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/named-lines.ts.e76ed0c41.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/named-lines.ts.ef9167261.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/named-lines.ts.f5f5f8e41.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/nested-subgrids.ts.52688aa11.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/nested-subgrids.ts.6e2d65331.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/nested-subgrids.ts.7a8f862e1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/nested-subgrids.ts.ab8df25d1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/spanning-subgrid.ts.179cccfd1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/spanning-subgrid.ts.2ba7bfb11.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/spanning-subgrid.ts.2edb0a211.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/spanning-subgrid.ts.71239b5d1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/spanning-subgrid.ts.80010a071.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/spanning-subgrid.ts.d74ca5171.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/track-sizing-contribution.ts.11fe95b71.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/track-sizing-contribution.ts.3c9f6df01.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/track-sizing-contribution.ts.9a7fe5be1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/track-sizing-contribution.ts.a2df471a1.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/track-sizing-contribution.ts.c86b1b921.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/subgrid/track-sizing-contribution.ts.f38600a81.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-grid/template-areas.ts.455eb0b51.pngis excluded by!**/*.png
📒 Files selected for processing (19)
docs/CSS_GRID_PLAN.mdintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.tsintegration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tswebf/lib/src/css/computed_style_declaration.dartwebf/lib/src/css/grid.dartwebf/lib/src/rendering/grid.dart
🧰 Additional context used
📓 Path-based instructions (6)
integration_tests/specs/**/*.ts
📄 CodeRabbit inference engine (integration_tests/CLAUDE.md)
integration_tests/specs/**/*.ts: Place tests in appropriate directories underspecs/(css/, dom/, or window/)
Use TypeScript (.ts extension) for test files
Usedone()callback for async tests
Usesnapshot()for visual regression tests to capture current rendering
UsesimulateClickwith coordinates for hit testing tests
Test assets should reference files inassets/directory
Usefdescribe()instead ofdescribe()to run only specific test specs (Jasmine feature)
Usefit()instead ofit()to run only specific test cases
Snapshots are stored as images for comparison and failed snapshots generate diff images
The max width of testing window is 340px
Test specs will always pass if there are no existing snapshots
Group related tests in describe blocks
Each test should be independent
Remove created elements after tests (test cleanup)
Use clear, descriptive test names
Test behavior, not implementation
Include edge cases and error conditions in tests
Batch DOM operations to minimize reflows
Use async/await and proper async patterns for asynchronous operations in tests
Measure operations usingperformance.now()for timing in performance-critical tests
Files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Clarify that WebF builds Flutter apps, not web apps in documentation
Provide complete, runnable examples in documentation
Include WebFControllerManager setup in code examples within documentation
Structure documentation information hierarchically
Test all code examples provided in documentation
Files:
docs/CSS_GRID_PLAN.md
webf/**/*.dart
📄 CodeRabbit inference engine (webf/CLAUDE.md)
webf/**/*.dart: Follow rules in webf/analysis_options.yaml for Dart code style
Use single quotes for strings in Dart code
File names must use snake_case in Dart
Class names must use PascalCase in Dart
Variables and functions must use camelCase in Dart
Prefer final fields when applicable in Dart code
Lines should be max 120 characters in Dart code
Always free allocated memory in Dart FFI using malloc.free() for toNativeUtf8() allocations
Free FFI allocated memory in finally blocks to ensure cleanup on exceptions
Track ownership of allocated pointers in FFI callbacks
Free NativeValue pointers after converting with fromNativeValue in FFI code
Document memory ownership clearly in FFI async callbacks
Implement WidgetElement to create custom Flutter widgets integrated into WebF's DOM tree
Register custom WidgetElements using WidgetElementRegistry.register(tagName, builder)
Override buildWidget(BuildContext context) method in WidgetElement to build the Flutter widget
Call updateWidget() when attributes change in WidgetElement implementations
Dispose controllers and subscriptions in WidgetElement for memory management
Follow W3C event standards when dispatching events from WidgetElement
Minimize widget rebuilds in WidgetElement for performance optimization
Implement ARIA attributes in WidgetElement when applicable for accessibilityDart code in webf module must follow naming conventions: snake_case for file names, PascalCase for classes, and camelCase for class members
webf/**/*.dart: Always free allocated memory in Dart FFI
Usemalloc.free()fortoNativeUtf8()allocations in Dart FFI
Track pointer ownership in callbacks within Dart FFI
Files:
webf/lib/src/css/computed_style_declaration.dartwebf/lib/src/css/grid.dartwebf/lib/src/rendering/grid.dart
webf/lib/src/css/**/*.dart
📄 CodeRabbit inference engine (webf/CLAUDE.md)
Use CSSRenderStyle for style computation and storage in Dart CSS code
Files:
webf/lib/src/css/computed_style_declaration.dartwebf/lib/src/css/grid.dart
{bridge/**/*.{cc,h},webf/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
Document memory ownership clearly in FFI implementations
Files:
webf/lib/src/css/computed_style_declaration.dartwebf/lib/src/css/grid.dartwebf/lib/src/rendering/grid.dart
webf/lib/src/rendering/**/*.dart
📄 CodeRabbit inference engine (webf/CLAUDE.md)
Use RenderBoxModel as base class for layout in Dart rendering code
Files:
webf/lib/src/rendering/grid.dart
🧠 Learnings (25)
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Place tests in appropriate directories under `specs/` (css/, dom/, or window/)
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsdocs/CSS_GRID_PLAN.mdintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Batch DOM operations to minimize reflows
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsdocs/CSS_GRID_PLAN.mdintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : The max width of testing window is 340px
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use `snapshot()` for visual regression tests to capture current rendering
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Test behavior, not implementation
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Measure operations using `performance.now()` for timing in performance-critical tests
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Group related tests in describe blocks
Applied to files:
integration_tests/specs/css/css-grid/subgrid/track-sizing-contribution.tsintegration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/gap-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Snapshots are stored as images for comparison and failed snapshots generate diff images
Applied to files:
integration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Include edge cases and error conditions in tests
Applied to files:
integration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/subgrid/abspos.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Each test should be independent
Applied to files:
integration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use clear, descriptive test names
Applied to files:
integration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/basic.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/mixed-tracks.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.tsintegration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Remove created elements after tests (test cleanup)
Applied to files:
integration_tests/specs/css/css-grid/subgrid/nested-subgrids.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/named-lines.tsintegration_tests/specs/css/css-grid/subgrid/explicit-placement.tsintegration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.ts
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/integration_tests/specs/**/*.dart : Use snapshot tests in integration_tests/specs/ for visual regression testing with await snapshot()
Applied to files:
integration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsdocs/CSS_GRID_PLAN.md
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Test specs will always pass if there are no existing snapshots
Applied to files:
integration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use async/await and proper async patterns for asynchronous operations in tests
Applied to files:
integration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.tsintegration_tests/specs/css/css-grid/subgrid/auto-placement.tsintegration_tests/specs/css/css-grid/subgrid/axis-combinations.tsintegration_tests/specs/css/css-grid/subgrid/alignment.tsintegration_tests/specs/css/css-grid/subgrid/basic-inheritance.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use `simulateClick` with coordinates for hit testing tests
Applied to files:
integration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Test assets should reference files in `assets/` directory
Applied to files:
integration_tests/specs/css/css-grid/subgrid/abspos.ts
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/test/**/*_test.dart : Access render objects in tests to verify layout calculations
Applied to files:
docs/CSS_GRID_PLAN.md
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Use widget unit tests to verify rendering changes with: cd webf && flutter test test/src/rendering/
Applied to files:
docs/CSS_GRID_PLAN.md
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/test/**/*_test.dart : Use WebFWidgetTestUtils.prepareWidgetTest() to test HTML/CSS rendering in widget unit tests
Applied to files:
docs/CSS_GRID_PLAN.md
📚 Learning: 2025-12-08T23:28:11.651Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:28:11.651Z
Learning: Applies to webf/test/**/*.dart : Dart/Flutter widget and unit tests must be placed in `webf/test/` and use `WebFWidgetTestUtils` and `pumpAndSettle()` where needed
Applied to files:
docs/CSS_GRID_PLAN.md
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/lib/src/rendering/**/*.dart : Use RenderBoxModel as base class for layout in Dart rendering code
Applied to files:
docs/CSS_GRID_PLAN.mdwebf/lib/src/css/grid.dartwebf/lib/src/rendering/grid.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/lib/src/css/**/*.dart : Use CSSRenderStyle for style computation and storage in Dart CSS code
Applied to files:
docs/CSS_GRID_PLAN.mdwebf/lib/src/css/computed_style_declaration.dartwebf/lib/src/css/grid.dartwebf/lib/src/rendering/grid.dart
📚 Learning: 2025-12-08T23:28:11.651Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:28:11.651Z
Learning: Applies to cli/test/**/*.test.ts : TypeScript test files must use `.test.ts` extension and target at least 70% code coverage for core modules
Applied to files:
docs/CSS_GRID_PLAN.md
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Follow rules in webf/analysis_options.yaml for Dart code style
Applied to files:
webf/lib/src/css/computed_style_declaration.dart
🧬 Code graph analysis (1)
integration_tests/specs/css/css-grid/subgrid/abspos.ts (1)
bridge/core/dom/node.cc (1)
parent(691-691)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build_windows_bridge (Debug)
- GitHub Check: build_bridge
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (16)
integration_tests/specs/css/css-grid/absolute-positioning/mixed-positioning.ts (1)
208-208: Verify the need for the snapshot delay.The added 0.5-second delay before snapshot capture suggests a rendering timing issue. Since
waitForFrame()is already called on Line 207, the additional delay may indicate that the overlay positioning requires extra time to stabilize with the new subgrid implementation.Consider investigating whether this delay is masking an underlying rendering issue or if it's truly necessary for the overlay test case. If the delay is required, document why this specific test needs more time than others.
webf/lib/src/css/computed_style_declaration.dart (2)
929-933: LGTM! GridSubgrid line name counting implemented correctly.The addition correctly handles GridSubgrid by iterating through its lineNameGroups and counting occurrences of the target line name. This aligns with the new subgrid support introduced in this PR.
967-976: LGTM! GridSubgrid CSS serialization implemented correctly.The special-case handling for a single GridSubgrid track correctly serializes it as "subgrid" followed by its line-name groups in CSS format. The logic appropriately skips empty groups and formats the output according to the CSS Grid Level 2 subgrid syntax.
integration_tests/specs/css/css-grid/subgrid/spanning-subgrid.ts (1)
1-267: LGTM! Comprehensive subgrid spanning test suite.The test suite effectively covers subgrid spanning scenarios across columns, rows, and both axes. The implementation follows integration test best practices:
- Clear, descriptive test names
- Proper async patterns with
waitForFrame()andsnapshot()- Batched DOM operations to minimize reflows
- Independent tests with proper cleanup via
parent.remove()- Appropriate use of
toBeCloseTo()with tolerance for dimension assertionsBased on learnings and coding guidelines.
integration_tests/specs/css/css-grid/subgrid/intrinsic-sizing.ts (1)
1-250: LGTM! Thorough intrinsic sizing test coverage.The test suite comprehensively validates subgrid intrinsic sizing behavior across various parent track configurations including min-content, max-content, fit-content, and mixed track types. The implementation demonstrates:
- Well-chosen test scenarios covering different intrinsic sizing modes
- Appropriate assertions matching each scenario's constraints
- Proper handling of edge cases like text wrapping
- Consistent test structure with proper cleanup
- Good use of
whiteSpace: 'nowrap'where content should not wrapBased on learnings and coding guidelines.
integration_tests/specs/css/css-grid/subgrid/nested-subgrids.ts (1)
1-204: LGTM! Well-structured nested subgrid test suite.The test suite effectively validates nested subgrid behavior across multiple nesting levels and configurations. Notable strengths include:
- Progressive testing from simple (2-level) to complex (3-level) nesting
- Coverage of both full-span and partial-span nested subgrids
- Validation of track inheritance across both axes
- Clear visual distinction between nesting levels using decreasing padding values
- Accurate dimensional assertions accounting for inherited track sizing and padding
Based on learnings and coding guidelines.
integration_tests/specs/css/css-grid/subgrid/explicit-placement.ts (1)
1-253: LGTM!The explicit placement tests are well-structured, follow coding guidelines, and provide comprehensive coverage of subgrid placement scenarios. The inline comments (lines 55, 119) helpfully explain the padding behavior being tested.
integration_tests/specs/css/css-grid/subgrid/gap-inheritance.ts (1)
1-239: LGTM!The gap inheritance tests provide excellent coverage of various gap scenarios, including edge cases like zero gaps and percentage-based gaps. The test at line 121 specifically validates the important behavior that a subgrid's own gap declaration doesn't override the inherited gap.
integration_tests/specs/css/css-grid/subgrid/auto-placement.ts (1)
1-195: LGTM!The auto-placement tests comprehensively cover subgrid auto-placement behavior across multiple scenarios. The inline comments (lines 35-36, 82) provide helpful context for understanding the expected padding behavior.
integration_tests/specs/css/css-grid/subgrid/abspos.ts (1)
1-1: LGTM!The active tests for absolutely positioned elements with subgrids are well-structured and cover important layout scenarios including containing block relationships and positioning behavior.
Also applies to: 2-52, 106-215
docs/CSS_GRID_PLAN.md (2)
16-18: Subgrid moved into in-scope features and Phase 9 marked completeThe updated non-goals, next-steps wording, and TODO checklist correctly treat Subgrid as an in-scope Level 2 feature while keeping masonry and other advanced modules out of scope. The Phase 9 checkbox and description are consistent with the rest of the document and with the new subgrid test files in this PR. No changes needed here.
Also applies to: 78-84, 85-96
209-226: Phase 9/Subgrid coverage numbers and phase summary look internally consistentI rechecked the arithmetic:
- Phase breakdown (including initial 11 files) sums to 91 files and 597 tests.
- The Subgrid section lists 14 files and 68 tests, which aligns with the Phase 9 row and the updated WPT distribution table (175 WPT Subgrid tests vs 68 covered here).
- The coverage notes, success metrics, and “All phases complete” bullets are consistent with the counts and file lists.
This documentation accurately reflects the new subgrid integration tests and overall grid coverage.
Also applies to: 234-247, 708-727, 791-806, 809-820
integration_tests/specs/css/css-grid/subgrid/axis-combinations.ts (1)
1-261: Well-structured subgrid axis-combination tests with good behavioral assertionsThe six tests cover key axis combinations (columns-only, rows-only, both axes, auto rows, regular+subgrid siblings, and spanning subset of tracks), follow the harness patterns (
await waitForFrame(),await snapshot(), and cleanup viaparent.remove()), and assert on concrete geometry rather than implementation details. This is a solid, behavior-focused suite and should give good regression signal around the new subgrid axis logic.webf/lib/src/rendering/grid.dart (3)
1271-1291: Line-name slicing/merging for subgrids appears correct
_sliceLineNameMapand_mergeLineNameMapWithSubgridGroups:
- Rebase line indices into a local 0-based space for a subgrid span,
- Preserve ordering and deduplicate indices per name, and
- Respect
lineCountbounds when merging subgridlineNameGroups.Using an inclusive
endLineIndexhere (and passingstart .. start + spanorstart + sizes.length) matches the fact that there aretrackCount + 1grid lines. This should give subgrids the right view of named lines while keeping parent and child maps independent.Also applies to: 1293-1322
2008-2021: Subgrid detection and fallback behavior look robustThe
_performGridLayoutchanges:
- Detect
grid-template-*-list == [GridSubgrid]via_isSubgridTrackList,- Pull contexts from the parent with
_takeSubgrid*Context(),- Override
colGap/rowGapusing the inherited subgrid gaps, and- Materialize “virtual” fixed or auto tracks based on whether inherited sizes are definite, including padding/border insets on the first/last tracks.
If a grid declares
subgridbut the parent didn’t provide a context, you clearcolsDef/rowsDefand effectively treat it as a non-subgrid grid, which is a sane, fail-soft behavior. This all looks good and should not disturb non-subgrid grids.Also applies to: 2041-2058, 2069-2114, 2141-2187, 2199-2223
2344-2411: Pre-pass for implicit columns referenced by definite placements is a good fixThe new row-flow-only pre-pass that scans
placementChildrenand uses_resolveLineRequirementIndexto determinerequiredColumns, followed by_ensureImplicitColumns(...), ensures:
- Implicit columns required by definite line-based placements exist before auto placement runs, and
- The auto-placement cursor doesn’t wrap too early in row-flow when later items reference implicit columns.
The logic respects both start and end line requirements and minimum spans, and it’s guarded by
colSizes.isNotEmptyand row-flow, so it shouldn’t regress existing layouts.
| grid.style.gridTemplateColumns = '[start] 100px [mid] 1fr [end] 80px'; | ||
| grid.style.gridTemplateRows = '40px 40px'; | ||
| grid.style.columnGap = '12px'; | ||
| grid.style.width = '400px'; |
There was a problem hiding this comment.
Reduce grid width to comply with testing window constraints.
The grid width is set to 400px, but the coding guidelines specify that the max width of the testing window is 340px. This may cause rendering issues or test inconsistencies.
🔎 Proposed fix
- grid.style.width = '400px';
+ grid.style.width = '340px';You may also need to adjust the gridTemplateColumns values to fit within the new width constraint while maintaining the test's intent.
Based on coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| grid.style.width = '400px'; | |
| grid.style.width = '340px'; |
🤖 Prompt for AI Agents
In @integration_tests/specs/css/css-grid/subgrid/basic.ts at line 8, The test
sets grid.style.width = '400px' which exceeds the testing window max of 340px;
update the assignment in the test to use a width <= '340px' (e.g., '340px' or
smaller) and, if needed, adjust the gridTemplateColumns values in the same test
(references: grid.style.width and grid.style.gridTemplateColumns) so the column
definitions still fit and preserve the test's intent.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.