feat: add responsive design field ingestion and rules#41
Merged
Conversation
- Add minWidth/maxWidth/minHeight/maxHeight, layoutGrow, constraints to AnalysisNode schema - Add layoutWrap, counterAxisSpacing, counterAxisAlignContent (wrap support) - Add clipsContent, overflowDirection (overflow/clip) - Extend LayoutModeSchema with "GRID" value - Map all new fields in figma-transformer.ts - Implement fixed-size-in-auto-layout rule (was return null) - Implement missing-min-width rule (was return null) - Implement missing-max-width rule (was return null) - Add transformer and rule tests https://claude.ai/code/session_018te5reJddNTD245p47kdKE
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds extraction and propagation of responsive layout metadata (min/max sizes, layoutGrow, constraints, wrap/counter-axis settings, grid container/child fields, overflow/clip) across the Figma plugin, transformer, Tailwind parser, schema contracts, layout rules, tests, and design-tree rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as Figma Plugin
participant Transformer as Figma→Analysis Transformer
participant Parser as Tailwind Parser
participant Rules as Rules Engine
participant Renderer as Design-tree Renderer
Plugin->>Transformer: Send raw node data (sizes, auto-layout, grid, constraints, overflow)
Transformer->>Parser: Enrich node from Tailwind classes (optional)
Parser-->>Transformer: Return extracted styles (min/max, wrap, gaps, overflow)
Transformer->>Rules: Provide AnalysisNode with responsive/grid/overflow fields
Rules->>Transformer: Query node fields for validations
Transformer->>Renderer: Supply enriched AnalysisFile
Renderer->>Renderer: Render layout as grid or flex using new fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Grid container: gridRowCount, gridColumnCount, gridRowGap, gridColumnGap, gridColumnsSizing, gridRowsSizing Grid child: gridChildHorizontalAlign, gridChildVerticalAlign, gridRowSpan, gridColumnSpan, gridRowAnchorIndex, gridColumnAnchorIndex https://claude.ai/code/session_018te5reJddNTD245p47kdKE
Plugin: - Map minWidth/maxWidth/minHeight/maxHeight, layoutGrow, constraints - Map layoutWrap, counterAxisSpacing, counterAxisAlignContent - Map grid container/child fields (12 properties) - Map clipsContent, overflowDirection - Convert Plugin API constraint enums to REST API format (MIN→LEFT, MAX→RIGHT, STRETCH→LEFT_RIGHT, etc.) MCP (Tailwind parser): - Parse min-w-*/max-w-*/min-h-*/max-h-* classes - Parse flex-wrap/flex-nowrap → layoutWrap - Parse gap-y-* → counterAxisSpacing, gap-x-* → itemSpacing - Parse overflow-hidden → clipsContent - Parse overflow-x/y-auto/scroll → overflowDirection - Enrich AnalysisNode with all new responsive fields https://claude.ai/code/session_018te5reJddNTD245p47kdKE
let-sunny
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/rules/layout/index.ts (1)
328-348:⚠️ Potential issue | 🟠 MajorLimit
missing-max-widthto actual auto-layout contexts.Unlike
missingMinWidth, this check no longer requires an auto-layout parent. Any node enriched withlayoutSizingHorizontal === "FILL"outside auto layout will now be flagged even thoughmaxWidthis not a relevant Figma fill constraint there. Please restore the parent guard and add a regression for the non-auto-layout case.Suggested fix
const missingMaxWidthCheck: RuleCheckFn = (node, context) => { // Only check containers and text-containing nodes if (!isContainerNode(node) && !hasTextContent(node)) return null; + if (!context.parent || !hasAutoLayout(context.parent)) return null; // Skip small elements if (node.absoluteBoundingBox) { const { width } = node.absoluteBoundingBox; if (width <= 200) return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/rules/layout/index.ts` around lines 328 - 348, The missingMaxWidthCheck currently flags nodes with layoutSizingHorizontal === "FILL" even when they're not inside an auto-layout parent; restore the same parent guard used by missingMinWidth so the rule only applies when the node has an auto-layout parent (e.g., check the parent's layoutMode or call the existing helper used by missingMinWidth like hasAutoLayoutParent(context) or isAutoLayoutParent(context)), and add a regression unit test asserting that a non-auto-layout node with layoutSizingHorizontal "FILL" and no maxWidth is NOT reported by missingMaxWidthCheck to prevent future regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/figma-plugin/src/main.ts`:
- Around line 231-237: The serializer currently omits the "NONE" enum when
copying overflowDirection, causing divergence from the REST contract; change the
logic in the block handling overflowDirection (references:
node.overflowDirection and result.overflowDirection) so that if
"overflowDirection" exists on node you always assign result.overflowDirection =
node.overflowDirection (including the "NONE" value) rather than skipping when it
equals "NONE", while still guarding for undefined/null as needed.
In `@src/core/adapters/tailwind-parser.ts`:
- Around line 93-100: The parser currently assigns gap-x/gap-y immediately in
parse logic (see token handling in src/core/adapters/tailwind-parser.ts around
the gap-* branches), which mismaps when flex direction is declared later; change
this by capturing/deferring gap values into temporary variables (e.g.,
pendingGapX, pendingGapY) when you encounter tokens starting with "gap-x-" or
"gap-y-" and do NOT set styles.itemSpacing or styles.counterAxisSpacing there,
then after all tokens are processed (and layoutMode is known, e.g., HORIZONTAL
for flex-row or VERTICAL for flex-col) map pendingGapX/pendingGapY to
styles.itemSpacing and styles.counterAxisSpacing according to the final
layoutMode (swap mapping for VERTICAL), and clear the pending variables. Ensure
resolveSpacing(token.slice(...)) is still used to compute px before storing
pending values.
In `@src/core/contracts/figma-node.ts`:
- Around line 39-40: You added "GRID" to LayoutModeSchema/LayoutMode but didn't
update consumers; before exporting GRID, update any code that branches on
AnalysisNode.layoutMode (the layout rendering path that currently does "if
layoutMode === 'VERTICAL' else ..." and the layout rule that checks for the
card-in-grid exception) to explicitly handle "GRID": change the renderer that
sets display:flex/flex-direction to instead produce CSS Grid behaviors for
"GRID", and update the layout rule (the function that exempts cards in grids) to
include the "GRID" case so the card-in-grid exception is applied; run/update
unit tests covering layout rendering and the card-in-grid rule.
---
Outside diff comments:
In `@src/core/rules/layout/index.ts`:
- Around line 328-348: The missingMaxWidthCheck currently flags nodes with
layoutSizingHorizontal === "FILL" even when they're not inside an auto-layout
parent; restore the same parent guard used by missingMinWidth so the rule only
applies when the node has an auto-layout parent (e.g., check the parent's
layoutMode or call the existing helper used by missingMinWidth like
hasAutoLayoutParent(context) or isAutoLayoutParent(context)), and add a
regression unit test asserting that a non-auto-layout node with
layoutSizingHorizontal "FILL" and no maxWidth is NOT reported by
missingMaxWidthCheck to prevent future regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 35a23c9a-ffe4-4251-adf0-3d1c3e3b2bed
📒 Files selected for processing (8)
app/figma-plugin/src/main.tssrc/core/adapters/figma-transformer.test.tssrc/core/adapters/figma-transformer.tssrc/core/adapters/tailwind-parser.test.tssrc/core/adapters/tailwind-parser.tssrc/core/contracts/figma-node.tssrc/core/rules/layout/index.tssrc/core/rules/layout/responsive-fields.test.ts
- Keep overflowDirection "NONE" in plugin serializer (match REST contract) - Defer gap-x/gap-y resolution until layoutMode is known (flex-col fix) - Handle GRID layoutMode in design-tree renderer (display: grid) - Add auto-layout parent guard to missing-max-width rule - Add regression tests for all fixes https://claude.ai/code/session_018te5reJddNTD245p47kdKE
Contributor
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 `@app/figma-plugin/src/main.ts`:
- Around line 231-237: The plugin currently only sets result.clipsContent and
result.overflowDirection inside the hasAutoLayout guard, causing mismatch with
the transformer; move the extraction of node.clipsContent and
node.overflowDirection out of the hasAutoLayout conditional so they are assigned
to result unconditionally (i.e., set result.clipsContent = node.clipsContent and
result.overflowDirection = node.overflowDirection outside/above the
hasAutoLayout block) and remove the now-duplicated assignments from inside the
hasAutoLayout branch; keep using the same property names (clipsContent,
overflowDirection), node and result variables as in the diff.
In `@src/core/adapters/tailwind-parser.ts`:
- Around line 19-28: Remove the unused "NONE" member from the
ExtractedStyles.overflowDirection union so the type precisely reflects only
produced values; update the type declaration that currently reads
overflowDirection?: "HORIZONTAL_SCROLLING" | "VERTICAL_SCROLLING" |
"HORIZONTAL_AND_VERTICAL_SCROLLING" | "NONE" to omit "NONE" and then run
TypeScript checks to fix any call sites or tests referencing
ExtractedStyles.overflowDirection to ensure they handle only the actual mapped
values.
In `@src/core/engine/design-tree.ts`:
- Around line 127-136: The GRID branch in render logic for node.layoutMode only
emits display and a generic gap; update the GRID branch (in design-tree.ts where
node.layoutMode === "GRID") to also emit grid-template-columns and
grid-template-rows from node.gridColumnsSizing and node.gridRowsSizing (e.g.
join/format each sizing token into a single string) and emit directional gaps
row-gap/column-gap from node.gridRowGap and node.gridColumnGap (only when those
values are present), keeping the existing generic gap as a fallback; reference
the properties node.gridColumnsSizing, node.gridRowsSizing, node.gridRowGap, and
node.gridColumnGap when adding these style strings.
In `@src/core/rules/layout/responsive-fields.test.ts`:
- Around line 48-52: Replace the unsafe indexed access issues[0]! with a safe
at(0) optional-chaining pattern: grab the first issue message via const
firstMessage = issues.at(0)?.violation.message and then assert it exists and
contains "Card" (e.g. expect(firstMessage).toBeDefined();
expect(firstMessage).toContain("Card")); update the test around result.issues /
issues and rule.definition.id accordingly to use
issues.at(0)?.violation.message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 952c1907-6a02-40c9-80fa-0639949abd0e
📒 Files selected for processing (6)
app/figma-plugin/src/main.tssrc/core/adapters/tailwind-parser.test.tssrc/core/adapters/tailwind-parser.tssrc/core/engine/design-tree.tssrc/core/rules/layout/index.tssrc/core/rules/layout/responsive-fields.test.ts
- Move clipsContent/overflowDirection extraction outside auto-layout guard in plugin (match transformer) - Remove unused "NONE" from ExtractedStyles.overflowDirection type - Enhance grid CSS output with template columns/rows and directional gaps - Use safer array access pattern (at(0)) in tests https://claude.ai/code/session_018te5reJddNTD245p47kdKE
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/adapters/tailwind-parser.ts (1)
95-104:⚠️ Potential issue | 🟡 MinorDefer generic
gap-*alongside directional variants for proper cross-axis spacing.Generic
gap-*is written directly tostyles.itemSpacing(line 103), bypassing the layout-aware resolution that follows. This breaks wrapped flex containers:flex-wrap gap-4losescounterAxisSpacing, and mixed directives likegap-4 gap-x-2lose the untouched axis when directional overrides apply. Instead, store the generic gap value and apply it to both axes during the layout-aware phase (after line 204), allowinggap-x-*andgap-y-*to selectively override each axis based on the finallayoutMode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/adapters/tailwind-parser.ts` around lines 95 - 104, The generic gap-* handling currently writes directly to styles.itemSpacing (in the token parsing block handling gap-y-, gap-x-, gap-), which bypasses layout-aware resolution; instead, when encountering token.startsWith("gap-") store its resolved spacing into a temporary variable (e.g., gap = resolveSpacing(val)) rather than setting styles.itemSpacing, and leave gapX and gapY handling as-is; then, in the layout-aware phase where layoutMode is known (the section after the existing parse loop that applies spacing based on layoutMode), apply the stored generic gap to both axes unless gapX or gapY were set by gap-x-* / gap-y-* (i.e., set styles.itemSpacing and/or counterAxisSpacing from the generic gap only if their axis-specific values are undefined).
♻️ Duplicate comments (1)
src/core/rules/layout/responsive-fields.test.ts (1)
121-126:⚠️ Potential issue | 🟡 MinorUse the same safe
at(0)pattern in the remaining rule assertions.These two positive cases still rely on
issues[0]!, so the file reintroduces thenoUncheckedIndexedAccessexception you already removed earlier. Grab the first message viaissues.at(0)?.violation.message, assert it exists, then check the content.As per coding guidelines: "This project uses TypeScript strict mode with noUncheckedIndexedAccess enabled. - Check for unchecked array/object index access (must guard against undefined)".
Also applies to: 197-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/rules/layout/responsive-fields.test.ts` around lines 121 - 126, Replace unchecked indexed access to the first issue (issues[0]!) with a safe at(0) pattern: retrieve the message via issues.at(0)?.violation.message, assert the result is defined (e.g., expect(message).toBeDefined()) and then perform the content assertion (expect(message).toContain("Content")). Apply the same change to the other assertion block that filters by rule.definition.id === "missing-min-width" (and the similar block at lines 197-202), ensuring all checks on the issues array use issues.at(0)? guarded access instead of direct indexed access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/figma-plugin/src/main.ts`:
- Around line 79-110: Replace the locally redefined AnalysisNode interface with
the canonical type from the shared contract: remove the local interface
declaration and import AnalysisNode from src/core/contracts/figma-node.ts so the
plugin uses the same enums/unions (LayoutWrap, GridChildAlign,
OverflowDirection, LayoutConstraint, etc.) and matches the Zod-validated schema;
update any usages in main.ts to reference the imported AnalysisNode type (and
adjust imports) so fields like layoutWrap, counterAxisAlignContent,
gridChildHorizontalAlign, gridChildVerticalAlign, overflowDirection and
constraints use the constrained types instead of bare string/object definitions.
In `@src/core/adapters/tailwind-parser.ts`:
- Around line 167-176: The parser currently lets the last overflow token win
because each branch sets styles.overflowDirection directly; instead, during the
token loop collect directional flags (e.g., hasOverflowX, hasOverflowY, and
hasOverflowAuto) when encountering "overflow-x-..." and "overflow-y-..." tokens
and keep handling non-directional tokens ("overflow-auto"/"overflow-scroll")
separately, then after the loop resolve and assign styles.overflowDirection
based on those flags (set to "HORIZONTAL_SCROLLING" if only x,
"VERTICAL_SCROLLING" if only y, "HORIZONTAL_AND_VERTICAL_SCROLLING" if both or a
global overflow-auto/scroll applies), update the token-processing block that
references token and styles.overflowDirection, and add a unit test "resolves
overflow-x/overflow-y correctly regardless of token order" to prevent
regressions.
In `@src/core/engine/design-tree.ts`:
- Around line 140-146: The current serialization in the else branch of the
layout block (where node.layoutMode, styles array) flattens wrapped auto-layouts
by only emitting a single gap and skipping wrap/alignment-on-content; update the
serializer to: when node.layoutWrap is truthy emit `flex-wrap: wrap` (or
`nowrap` accordingly), when node.counterAxisAlignContent is present emit
`align-content: ${mapAlign(node.counterAxisAlignContent)}`, and when
node.counterAxisSpacing is present emit the second-axis gap (use `row-gap` if
layout direction is column, otherwise `column-gap`) in addition to the primary
`gap` so both axes are preserved; make sure to reference and use
node.layoutMode, node.itemSpacing, node.counterAxisSpacing, node.layoutWrap,
node.primaryAxisAlignItems, node.counterAxisAlignItems,
node.counterAxisAlignContent and the mapAlign helper, and update docs
(README/CLAUDE/JSDoc) to reflect the wrapped-flex behavior change.
---
Outside diff comments:
In `@src/core/adapters/tailwind-parser.ts`:
- Around line 95-104: The generic gap-* handling currently writes directly to
styles.itemSpacing (in the token parsing block handling gap-y-, gap-x-, gap-),
which bypasses layout-aware resolution; instead, when encountering
token.startsWith("gap-") store its resolved spacing into a temporary variable
(e.g., gap = resolveSpacing(val)) rather than setting styles.itemSpacing, and
leave gapX and gapY handling as-is; then, in the layout-aware phase where
layoutMode is known (the section after the existing parse loop that applies
spacing based on layoutMode), apply the stored generic gap to both axes unless
gapX or gapY were set by gap-x-* / gap-y-* (i.e., set styles.itemSpacing and/or
counterAxisSpacing from the generic gap only if their axis-specific values are
undefined).
---
Duplicate comments:
In `@src/core/rules/layout/responsive-fields.test.ts`:
- Around line 121-126: Replace unchecked indexed access to the first issue
(issues[0]!) with a safe at(0) pattern: retrieve the message via
issues.at(0)?.violation.message, assert the result is defined (e.g.,
expect(message).toBeDefined()) and then perform the content assertion
(expect(message).toContain("Content")). Apply the same change to the other
assertion block that filters by rule.definition.id === "missing-min-width" (and
the similar block at lines 197-202), ensuring all checks on the issues array use
issues.at(0)? guarded access instead of direct indexed access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: efe2efb7-95f9-45e9-abf5-a7e63679667f
📒 Files selected for processing (4)
app/figma-plugin/src/main.tssrc/core/adapters/tailwind-parser.tssrc/core/engine/design-tree.tssrc/core/rules/layout/responsive-fields.test.ts
Comment on lines
+79
to
+110
| // Responsive / size constraints | ||
| minWidth?: number; | ||
| maxWidth?: number; | ||
| minHeight?: number; | ||
| maxHeight?: number; | ||
| layoutGrow?: 0 | 1; | ||
| constraints?: { horizontal: string; vertical: string }; | ||
|
|
||
| // Wrap | ||
| layoutWrap?: string; | ||
| counterAxisSpacing?: number; | ||
| counterAxisAlignContent?: string; | ||
|
|
||
| // Grid layout (container) | ||
| gridRowCount?: number; | ||
| gridColumnCount?: number; | ||
| gridRowGap?: number; | ||
| gridColumnGap?: number; | ||
| gridColumnsSizing?: string; | ||
| gridRowsSizing?: string; | ||
|
|
||
| // Grid layout (child) | ||
| gridChildHorizontalAlign?: string; | ||
| gridChildVerticalAlign?: string; | ||
| gridRowSpan?: number; | ||
| gridColumnSpan?: number; | ||
| gridRowAnchorIndex?: number; | ||
| gridColumnAnchorIndex?: number; | ||
|
|
||
| // Overflow / clip | ||
| clipsContent?: boolean; | ||
| overflowDirection?: string; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file structure
git ls-files | grep -E "(figma-plugin|contracts)" | head -20Repository: let-sunny/canicode
Length of output: 773
🏁 Script executed:
# Look for the main.ts file and read the problematic lines
if [ -f "app/figma-plugin/src/main.ts" ]; then
wc -l app/figma-plugin/src/main.ts
fiRepository: let-sunny/canicode
Length of output: 94
🏁 Script executed:
# Read the section around lines 79-110 of main.ts
if [ -f "app/figma-plugin/src/main.ts" ]; then
sed -n '60,130p' app/figma-plugin/src/main.ts
fiRepository: let-sunny/canicode
Length of output: 1749
🏁 Script executed:
# Look for the figma-node.ts contract file
find . -name "figma-node.ts" -o -name "*figma*" -type f | grep -E "\.(ts|tsx)$" | head -20Repository: let-sunny/canicode
Length of output: 387
🏁 Script executed:
# Read the complete figma-node.ts contract file
cat -n src/core/contracts/figma-node.tsRepository: let-sunny/canicode
Length of output: 7612
🏁 Script executed:
# Check for Zod usage and schema definitions in the codebase
rg "import.*zod" --type ts --type tsx | head -20Repository: let-sunny/canicode
Length of output: 89
🏁 Script executed:
# Look for schema definitions related to figma-node
rg "const.*Schema|export.*z\." src/core/contracts/ -A 3 | head -50Repository: let-sunny/canicode
Length of output: 2508
🏁 Script executed:
# Check the imports at the top of main.ts and understand the interface definition context
head -85 app/figma-plugin/src/main.ts | tail -30Repository: let-sunny/canicode
Length of output: 956
🏁 Script executed:
# Get the full AnalysisNode interface definition from main.ts
sed -n '40,115p' app/figma-plugin/src/main.tsRepository: let-sunny/canicode
Length of output: 1942
🏁 Script executed:
# Check imports in main.ts
head -30 app/figma-plugin/src/main.tsRepository: let-sunny/canicode
Length of output: 830
🏁 Script executed:
# Search for any schema usage or validation in main.ts
rg "AnalysisNodeSchema|z\.|parse|validate" app/figma-plugin/src/main.tsRepository: let-sunny/canicode
Length of output: 206
🏁 Script executed:
# Check if figma-node contract types are imported anywhere in main.ts
rg "from.*figma-node|import.*AnalysisNode" app/figma-plugin/src/main.tsRepository: let-sunny/canicode
Length of output: 44
Import and reuse AnalysisNode type from the shared contract instead of redefining it locally.
The local interface in lines 40–115 claims to "match src/core/contracts/figma-node.ts" but actually widens multiple fields to bare string types: layoutWrap, counterAxisAlignContent, gridChildHorizontalAlign, gridChildVerticalAlign, overflowDirection, and the constraints object. The shared schema constrains these to specific enums and unions (LayoutWrap, GridChildAlign, OverflowDirection, LayoutConstraint, etc.).
This divergence means the plugin—which is the data producer—can emit values that violate the shared contract, defeating compile-time safety and creating maintenance burden. Remove this local interface and import the actual AnalysisNode type from src/core/contracts/figma-node.ts to ensure type alignment with the Zod-validated schema.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/figma-plugin/src/main.ts` around lines 79 - 110, Replace the locally
redefined AnalysisNode interface with the canonical type from the shared
contract: remove the local interface declaration and import AnalysisNode from
src/core/contracts/figma-node.ts so the plugin uses the same enums/unions
(LayoutWrap, GridChildAlign, OverflowDirection, LayoutConstraint, etc.) and
matches the Zod-validated schema; update any usages in main.ts to reference the
imported AnalysisNode type (and adjust imports) so fields like layoutWrap,
counterAxisAlignContent, gridChildHorizontalAlign, gridChildVerticalAlign,
overflowDirection and constraints use the constrained types instead of bare
string/object definitions.
- Defer generic gap-* to both axes, allow directional gap-x/gap-y overrides - Defer overflow-x/overflow-y flags and combine after loop - Emit flex-wrap, align-content, counterAxisSpacing in design tree - Fix remaining issues[0]! → at(0)? in tests https://claude.ai/code/session_018te5reJddNTD245p47kdKE
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/adapters/tailwind-parser.ts`:
- Around line 170-180: The parser currently only tracks overflowX and overflowY
for scrolling and ignores overflow-x-hidden/overflow-y-hidden; update the token
handling in tailwind-parser to add axis-hidden flags (e.g., overflowXHidden,
overflowYHidden) alongside the existing overflowX/overflowY and handle tokens
"overflow-x-hidden" and "overflow-y-hidden" to set them, then resolve final
state so hidden on an axis wins over scrolling (set styles.clipsContent or
equivalent per-axis outcome) — update the logic where styles.clipsContent,
overflowX, and overflowY are finalized to consider
overflowXHidden/overflowYHidden, and add a unit test for the combination
"overflow-x-hidden overflow-y-auto" to assert the X axis is hidden while Y
remains scrollable to prevent regression.
In `@src/core/engine/design-tree.ts`:
- Around line 144-150: The current gap serialization in design-tree.ts uses the
shorthand `gap: ${node.itemSpacing}px ${node.counterAxisSpacing}px` which maps
to row-gap/column-gap (block/inline) and misassigns main/cross axis sizes for
flex; change the logic in the block handling
node.itemSpacing/node.counterAxisSpacing (where styles.push is used) to emit
explicit properties based on node.layoutMode: when layoutMode is "HORIZONTAL"
set column-gap to the main-axis spacing and row-gap to the counter-axis spacing
(or only column-gap if counterAxisSpacing is null), and when layoutMode is
"VERTICAL" set row-gap to the main-axis spacing and column-gap to the
counter-axis spacing (or only row-gap if counterAxisSpacing is null); keep using
node.itemSpacing, node.counterAxisSpacing, and layoutMode identifiers to locate
the relevant conditionals and styles.push calls.
- Around line 152-154: The code is emitting invalid CSS when
node.counterAxisAlignContent equals the Figma "AUTO" value because mapAlign
returns "AUTO"; modify the emission logic in the block that handles
counterAxisAlignContent (the lines using node.counterAxisAlignContent and
mapAlign) to skip pushing an align-content style if the source value is "AUTO"
(or mapAlign returns "AUTO")—i.e., add a guard like if
(node.counterAxisAlignContent && node.counterAxisAlignContent !== "AUTO")
styles.push(`align-content: ${mapAlign(node.counterAxisAlignContent)}`) so no
invalid `align-content: AUTO` is produced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 218f48f1-0978-4c41-b6e2-171d3ddaf748
📒 Files selected for processing (4)
src/core/adapters/tailwind-parser.test.tssrc/core/adapters/tailwind-parser.tssrc/core/engine/design-tree.tssrc/core/rules/layout/responsive-fields.test.ts
- Add overflow-x-hidden/overflow-y-hidden handlers in tailwind parser - Use explicit column-gap/row-gap instead of shorthand gap (correct axis mapping) - Skip align-content emission when value is "AUTO" (invalid CSS) - Add tests for mixed overflow hidden/scroll combinations https://claude.ai/code/session_018te5reJddNTD245p47kdKE
This was referenced Mar 25, 2026
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
2 participants
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
AnalysisNode(minWidth, maxWidth, layoutWrap, counterAxisSpacing, grid container/child properties, overflow/clip)missing-min-width,missing-max-widthrules detect FILL containers without size constraintsVerification
minWidth,maxWidth,layoutWrappresent)Changed files (8 files, +791 lines)
contracts/figma-node.tsadapters/figma-transformer.tsadapters/tailwind-parser.tsapp/figma-plugin/src/main.tsrules/layout/index.ts*.test.ts(3 files)Test plan
pnpm test:run— 338 tests passmissing-min-width+missing-max-widthdetectedhttps://claude.ai/code/session_018te5reJddNTD245p47kdKE
Summary by CodeRabbit
New Features
Tests