-
Notifications
You must be signed in to change notification settings - Fork 0
fix: add layout rule exceptions for icons, backgrounds, images, text #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
35274af
201f397
920bbbe
889fc1b
3056339
ea25465
e4cd785
1b6f3ae
43afcb1
b991e44
2a5c745
89f1013
5da9cb5
86f8a72
8704f10
8a896c1
e774f74
e131e6e
f8c15d9
c19200f
8861383
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,210 @@ | ||
| import type { AnalysisNode } from "../contracts/figma-node.js"; | ||
| import type { RuleContext } from "../contracts/rule.js"; | ||
| import { | ||
| isAutoLayoutExempt, | ||
| isAbsolutePositionExempt, | ||
| isSizeConstraintExempt, | ||
| isFixedSizeExempt, | ||
| isVisualOnlyNode, | ||
| } from "./rule-exceptions.js"; | ||
|
|
||
| function makeNode(overrides: Partial<AnalysisNode> = {}): AnalysisNode { | ||
| return { | ||
| id: "test", | ||
| name: "Test", | ||
| type: "FRAME", | ||
| visible: true, | ||
| ...overrides, | ||
| } as AnalysisNode; | ||
| } | ||
|
|
||
| function makeContext(overrides: Partial<RuleContext> = {}): RuleContext { | ||
| return { | ||
| file: {} as RuleContext["file"], | ||
| depth: 2, | ||
| componentDepth: 0, | ||
| maxDepth: 10, | ||
| path: ["Root", "Test"], | ||
| analysisState: new Map(), | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| describe("isVisualOnlyNode", () => { | ||
| it("true for vector/shape types", () => { | ||
| expect(isVisualOnlyNode(makeNode({ type: "VECTOR" as any }))).toBe(true); | ||
| expect(isVisualOnlyNode(makeNode({ type: "ELLIPSE" as any }))).toBe(true); | ||
| }); | ||
|
|
||
| it("true for nodes with image fills", () => { | ||
| const node = makeNode({ fills: [{ type: "IMAGE" }] }); | ||
| expect(isVisualOnlyNode(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("true for frame with only visual leaf children", () => { | ||
| const node = makeNode({ | ||
| children: [makeNode({ type: "VECTOR" as any }), makeNode({ type: "RECTANGLE" as any })], | ||
| }); | ||
| expect(isVisualOnlyNode(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("false for frame with mixed children", () => { | ||
| const node = makeNode({ | ||
| children: [makeNode({ type: "VECTOR" as any }), makeNode({ type: "TEXT" as any })], | ||
| }); | ||
| expect(isVisualOnlyNode(node)).toBe(false); | ||
| }); | ||
|
|
||
| it("false for plain frame without image fills", () => { | ||
| const node = makeNode({ fills: [{ type: "SOLID" }] }); | ||
| expect(isVisualOnlyNode(node)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("isAutoLayoutExempt", () => { | ||
| it("exempts frames with only visual leaf children", () => { | ||
| const node = makeNode({ | ||
| children: [ | ||
| makeNode({ type: "VECTOR" as any }), | ||
| makeNode({ type: "ELLIPSE" as any }), | ||
| ], | ||
| }); | ||
| expect(isAutoLayoutExempt(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("does not exempt image-filled frames with content children", () => { | ||
| const node = makeNode({ fills: [{ type: "IMAGE" }], children: [makeNode({ type: "TEXT" as any })] }); | ||
| expect(isAutoLayoutExempt(node)).toBe(false); | ||
| }); | ||
|
|
||
| it("does not exempt INSTANCE nodes", () => { | ||
| const node = makeNode({ type: "INSTANCE" as any, children: [makeNode()] }); | ||
| expect(isAutoLayoutExempt(node)).toBe(false); | ||
| }); | ||
|
|
||
| it("does not exempt frames with mixed children", () => { | ||
| const node = makeNode({ | ||
| children: [ | ||
| makeNode({ type: "VECTOR" as any }), | ||
| makeNode({ type: "TEXT" as any }), | ||
| ], | ||
| }); | ||
| expect(isAutoLayoutExempt(node)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("isAbsolutePositionExempt", () => { | ||
| it("exempts nodes with image fills", () => { | ||
| const node = makeNode({ | ||
| fills: [{ type: "IMAGE", scaleMode: "FILL" }], | ||
| }); | ||
| expect(isAbsolutePositionExempt(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts vector nodes", () => { | ||
| const node = makeNode({ type: "VECTOR" as any }); | ||
| expect(isAbsolutePositionExempt(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("does not exempt plain frame", () => { | ||
| const node = makeNode({ fills: [{ type: "SOLID" }] }); | ||
| expect(isAbsolutePositionExempt(node)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("isSizeConstraintExempt", () => { | ||
| it("exempts when node has maxWidth", () => { | ||
| const node = makeNode({ maxWidth: 800 }); | ||
| expect(isSizeConstraintExempt(node, makeContext())).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts small elements (width <= 200)", () => { | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 150, height: 40 }, | ||
| }); | ||
| expect(isSizeConstraintExempt(node, makeContext())).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts when parent has maxWidth", () => { | ||
| const parent = makeNode({ maxWidth: 1200 }); | ||
| const node = makeNode({}); | ||
| expect(isSizeConstraintExempt(node, makeContext({ parent }))).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts root-level frames (depth <= 1)", () => { | ||
| const node = makeNode({}); | ||
| expect(isSizeConstraintExempt(node, makeContext({ depth: 1 }))).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts when all siblings are FILL", () => { | ||
| const node = makeNode({ layoutSizingHorizontal: "FILL" as any }); | ||
| const sibling = makeNode({ layoutSizingHorizontal: "FILL" as any }); | ||
| const ctx = makeContext({ siblings: [node, sibling] }); | ||
| expect(isSizeConstraintExempt(node, ctx)).toBe(true); | ||
| }); | ||
|
|
||
| it("does not exempt when siblings have mixed sizing", () => { | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 400, height: 100 }, | ||
| layoutSizingHorizontal: "FILL" as any, | ||
| }); | ||
| const sibling = makeNode({ layoutSizingHorizontal: "FIXED" as any }); | ||
| const ctx = makeContext({ siblings: [node, sibling] }); | ||
| expect(isSizeConstraintExempt(node, ctx)).toBe(false); | ||
| }); | ||
|
|
||
| it("exempts inside GRID layout", () => { | ||
| const parent = makeNode({ layoutMode: "GRID" as any }); | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 400, height: 100 }, | ||
| layoutSizingHorizontal: "FILL" as any, | ||
| }); | ||
| const sibling = makeNode({ layoutSizingHorizontal: "FIXED" as any }); | ||
| expect(isSizeConstraintExempt(node, makeContext({ parent, siblings: [node, sibling] }))).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts inside flex wrap", () => { | ||
| const parent = makeNode({ layoutWrap: "WRAP" as any }); | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 400, height: 100 }, | ||
| layoutSizingHorizontal: "FILL" as any, | ||
| }); | ||
| const sibling = makeNode({ layoutSizingHorizontal: "FIXED" as any }); | ||
| expect(isSizeConstraintExempt(node, makeContext({ parent, siblings: [node, sibling] }))).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts TEXT nodes", () => { | ||
| const parent = makeNode({ layoutMode: "HORIZONTAL" as any }); | ||
| const node = makeNode({ type: "TEXT" as any, layoutSizingHorizontal: "FILL" as any }); | ||
| const ctx = makeContext({ | ||
| parent, | ||
| siblings: [node, makeNode({ layoutSizingHorizontal: "FIXED" as any })], | ||
| }); | ||
| expect(isSizeConstraintExempt(node, ctx)).toBe(true); | ||
| }); | ||
| }); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| describe("isFixedSizeExempt", () => { | ||
| it("exempts small elements (icons)", () => { | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 24, height: 24 }, | ||
| }); | ||
| expect(isFixedSizeExempt(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("exempts nodes with image fills", () => { | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 200, height: 200 }, | ||
| fills: [{ type: "IMAGE", scaleMode: "FILL" }], | ||
| }); | ||
| expect(isFixedSizeExempt(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("does not exempt large nodes without image fills", () => { | ||
| const node = makeNode({ | ||
| absoluteBoundingBox: { x: 0, y: 0, width: 200, height: 200 }, | ||
| fills: [{ type: "SOLID", color: { r: 1, g: 0, b: 0, a: 1 } }], | ||
| }); | ||
| expect(isFixedSizeExempt(node)).toBe(false); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| import type { AnalysisNode } from "../contracts/figma-node.js"; | ||
| import type { RuleContext } from "../contracts/rule.js"; | ||
| import { isExcludedName } from "./excluded-names.js"; | ||
|
|
||
| // ============================================ | ||
| // Shared node type helpers | ||
| // ============================================ | ||
|
|
||
| const VISUAL_LEAF_TYPES = new Set([ | ||
| "VECTOR", "BOOLEAN_OPERATION", "ELLIPSE", "LINE", "STAR", "REGULAR_POLYGON", "RECTANGLE", | ||
| ]); | ||
|
|
||
| export function isVisualLeafType(type: string): boolean { | ||
| return VISUAL_LEAF_TYPES.has(type); | ||
| } | ||
|
|
||
| /** Node has an IMAGE type fill */ | ||
| export function hasImageFill(node: AnalysisNode): boolean { | ||
| if (!Array.isArray(node.fills)) return false; | ||
| return node.fills.some( | ||
| (fill) => | ||
| typeof fill === "object" && | ||
| fill !== null && | ||
| (fill as { type?: unknown }).type === "IMAGE", | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Node is purely visual — not a layout container. | ||
| * True when: vector/shape type, has image fill, or frame with only visual leaf children. | ||
| */ | ||
| export function isVisualOnlyNode(node: AnalysisNode): boolean { | ||
| if (VISUAL_LEAF_TYPES.has(node.type)) return true; | ||
| if (hasImageFill(node)) return true; | ||
| if (node.children && node.children.length > 0 && node.children.every((c) => VISUAL_LEAF_TYPES.has(c.type))) return true; | ||
| return false; | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| // ============================================ | ||
| // Auto-layout exceptions | ||
| // ============================================ | ||
|
|
||
| /** Frames that don't need auto-layout (only visual-leaf children like icon paths) */ | ||
| export function isAutoLayoutExempt(node: AnalysisNode): boolean { | ||
| if ( | ||
| node.children && | ||
| node.children.length > 0 && | ||
| node.children.every((c) => VISUAL_LEAF_TYPES.has(c.type)) | ||
| ) return true; | ||
|
|
||
| return false; | ||
|
Comment on lines
+47
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Using 🎯 Proposed fix export function isAutoLayoutExempt(node: AnalysisNode): boolean {
- if (isVisualOnlyNode(node)) return true;
-
- return false;
+ if (node.type === "INSTANCE") return true;
+ if (
+ node.type === "FRAME" &&
+ node.children &&
+ node.children.length > 0 &&
+ node.children.every((child) => isVisualLeafType(child.type))
+ ) {
+ return true;
+ }
+ return false;
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| // ============================================ | ||
| // Absolute-position exceptions | ||
| // ============================================ | ||
|
|
||
| /** Nodes that are allowed to use absolute positioning inside auto-layout */ | ||
| export function isAbsolutePositionExempt(node: AnalysisNode): boolean { | ||
| if (isVisualOnlyNode(node)) return true; | ||
|
|
||
| // Intentional name patterns (badge, close, overlay, etc.) | ||
| if (isExcludedName(node.name)) return true; | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| // ============================================ | ||
| // Size-constraint exceptions | ||
| // ============================================ | ||
|
|
||
| /** Nodes that don't need maxWidth even with FILL sizing */ | ||
| export function isSizeConstraintExempt(node: AnalysisNode, context: RuleContext): boolean { | ||
| // Already has maxWidth | ||
| if (node.maxWidth !== undefined) return true; | ||
|
|
||
| // Small elements — won't stretch problematically | ||
| if (node.absoluteBoundingBox && node.absoluteBoundingBox.width <= 200) return true; | ||
|
|
||
| // Parent already has maxWidth — parent constrains the stretch | ||
| if (context.parent?.maxWidth !== undefined) return true; | ||
|
|
||
| // Root-level frames — they represent the screen itself | ||
| if (context.depth <= 1) return true; | ||
|
|
||
| // All siblings are FILL (e.g. single item or list view) — parent controls the width | ||
| if (context.siblings && context.siblings.length > 0) { | ||
| if (context.siblings.every((s) => s.layoutSizingHorizontal === "FILL")) return true; | ||
| } | ||
|
|
||
| // Inside grid layout — grid controls sizing | ||
| if (context.parent?.layoutMode === "GRID") return true; | ||
|
|
||
| // Inside flex wrap — wrap layout controls sizing per row | ||
| if (context.parent?.layoutWrap === "WRAP") return true; | ||
|
|
||
| // Text nodes — content length provides natural sizing | ||
| if (node.type === "TEXT") return true; | ||
|
|
||
| return false; | ||
| } | ||
|
Comment on lines
+75
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential false exemption when When Consider whether this is the intended behavior. If Suggested clarification // Only FILL child among siblings — intent is to fill the parent entirely
- if (context.siblings) {
+ if (context.siblings && context.siblings.length > 0) {
const fillSiblings = context.siblings.filter((s) => s.layoutSizingHorizontal === "FILL");
if (fillSiblings.length <= 1) return true;
}🤖 Prompt for AI Agents |
||
|
|
||
| // ============================================ | ||
| // Fixed-size exceptions | ||
| // ============================================ | ||
|
|
||
| /** Nodes that are allowed to use fixed sizing inside auto-layout */ | ||
| export function isFixedSizeExempt(node: AnalysisNode): boolean { | ||
| // Small fixed elements (icons, avatars) — intentionally fixed | ||
| if (node.absoluteBoundingBox) { | ||
| const { width, height } = node.absoluteBoundingBox; | ||
| if (width <= 48 && height <= 48) return true; | ||
| } | ||
|
|
||
| if (isVisualOnlyNode(node)) return true; | ||
|
|
||
| // Excluded names (nav, header, etc.) | ||
| if (isExcludedName(node.name)) return true; | ||
|
|
||
| return false; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.