feat: add recursive shorthand expansion and background-position support #53
Conversation
**Features:** - Add recursive shorthand expansion (e.g., background → background-position → -x/-y) - Register background-position handler in shorthand registry - Handle single-layer recursive expansion while preserving multi-layer values **Changes:** - src/core/expand.ts: Implement recursive expansion with multi-layer detection - src/internal/shorthand-registry.ts: Add background-position handler - test/recursive-expansion.test.ts: Add 6 new tests for recursive expansion **Behavior:** - Single-layer: background-position: center → background-position-x/y: center - Multi-layer: background-position: 0% 0%, center → stays as background-position (needs layer-aware splitting) **Tests:** 929/929 passing (+6 new recursive expansion tests) **Integration:** Enables @b/values to use b_short with background-position-x/y support
There was a problem hiding this comment.
Pull Request Overview
This pull request implements recursive shorthand expansion functionality, enabling properties like background to expand through intermediate shorthands (e.g., background → background-position → background-position-x/y). The implementation adds multi-layer detection to prevent incorrect recursive expansion of comma-separated layer values.
Key Changes:
- Implemented recursive expansion logic in
src/core/expand.tswith multi-layer value detection - Registered
background-positionhandler in the shorthand registry - Added 6 comprehensive test cases covering single-layer, multi-layer, and edge cases
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/core/expand.ts | Adds recursive expansion loop that checks for nested shorthands and skips multi-layer values |
| src/internal/shorthand-registry.ts | Registers background-position handler for expansion |
| test/recursive-expansion.test.ts | Adds 6 test cases validating recursive expansion behavior |
| test/property-grouping.test.ts | Updates expected output to reflect background-position-x/y expansion |
| test/overrides.test.ts | Updates expected output for position property expansion |
| test/invalid-cases.test.ts | Updates expected output for background position properties |
| docs.llm/llm_src.txt | Large documentation file deletion (likely intended for rebuild) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // as they need layer-aware splitting first | ||
| for (const [prop, val] of Object.entries(longhand)) { | ||
| const nestedParse = shorthand[prop]; | ||
| const isMultiLayer = hasTopLevelCommas(val); |
There was a problem hiding this comment.
The multi-layer background detection logic may incorrectly skip recursive expansion. hasTopLevelCommas(val) returns true for values like "center center" (single-layer, no commas). This means the recursive expansion would be skipped for valid single-layer values.
Consider checking if the value actually contains commas:
const isMultiLayer = val.includes(',') || hasTopLevelCommas(val);Or more precisely, only treat as multi-layer if there are actual commas at the top level.
| const isMultiLayer = hasTopLevelCommas(val); | |
| const isMultiLayer = val.includes(',') && hasTopLevelCommas(val); |
Features:
Changes:
Behavior:
Tests: 929/929 passing (+6 new recursive expansion tests)
Integration: Enables @b/values to use b_short with background-position-x/y support