Skip to content

refactor(css): logical property audit — replace physical with logical equivalents #321

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.

Sign up for GitHub

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

Merged
jackgranatowski merged 4 commits into from
Jun 13, 2026
Merged

refactor(css): logical property audit — replace physical with logical equivalents #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bb22fb4
refactor(css): logical property audit — replace physical with logical…
claude Jun 13, 2026
684c0cb
test(tokens): scoped semantic token value snapshot
claude Jun 13, 2026
57f9cc6
chore: sync generated docs and lock file
claude Jun 13, 2026
a14a9dd
fix(tests): update assertions for logical property changes + fix vers…
claude Jun 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/base.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
text-wrap: var(--sf-heading-text-wrap);
overflow-wrap: break-word;
text-rendering: optimizelegibility;
scroll-margin-top: calc(var(--sf-header-height, 5rem) + var(--sf-space-m));
scroll-margin-block-start: calc(var(--sf-header-height, 5rem) + var(--sf-space-m));
}

h1 { font-size: var(--sf-h1-size); line-height: var(--sf-h1-line-height); font-weight: var(--sf-h1-font-weight); letter-spacing: var(--sf-h1-letter-spacing); }
Expand Down Expand Up @@ -129,8 +129,8 @@
border-radius: var(--sf-radius-m);
background: var(--sf-color-code-block-bg, var(--sf-color-code-bg));
color: var(--sf-color-code-block-text, inherit);
overflow-x: auto;
tab-size: 2;
overflow-inline: auto;
tab-size: 2;
hyphens: none;
}
pre code {
Expand Down Expand Up @@ -170,7 +170,7 @@
}

:target {
scroll-margin-top: calc(var(--sf-header-height, 5rem) + var(--sf-space-m));
scroll-margin-block-start: calc(var(--sf-header-height, 5rem) + var(--sf-space-m));
}

::selection {
Expand Down
10 changes: 5 additions & 5 deletions core/layout.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@
flex-basis: calc((var(--sf-switcher-threshold) - 100%) * 999);
}

.sf-switcher--no-wrap { flex-wrap: nowrap; overflow-x: auto; }
.sf-switcher--no-wrap { flex-wrap: nowrap; overflow-inline: auto; }
.sf-switcher--vertical { flex-direction: column; }

/* -- Grid ----------------------------------------------------
Expand Down Expand Up @@ -368,10 +368,10 @@
display: flex;
block-size: var(--sf-reel-height);
gap: var(--sf-reel-gap);
overflow-x: auto;
scroll-snap-type: x mandatory;
scroll-padding-inline: var(--sf-reel-gap);
overscroll-behavior-x: contain;
overflow-inline: auto;
scroll-snap-type: inline mandatory;
scroll-padding-inline: var(--sf-reel-gap);
overscroll-behavior-inline: contain;
scrollbar-width: thin;
}

Expand Down
14 changes: 7 additions & 7 deletions core/macros.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
.sf-prose hr { margin-block: var(--sf-prose-hr-margin); }
.sf-prose figure { margin-block: var(--sf-prose-figure-margin); }
.sf-prose figcaption { font-size: var(--sf-prose-figcaption-size); color: var(--sf-color-text--muted); margin-block-start: var(--sf-prose-list-gap); }
.sf-prose table { margin-block: var(--sf-prose-block-margin); display: block; overflow-x: auto; width: 100%; max-width: 100%; }
.sf-prose table { margin-block: var(--sf-prose-block-margin); display: block; overflow-inline: auto; inline-size: 100%; max-inline-size: 100%; }
.sf-prose :is(th, td) { padding: var(--sf-prose-table-pad); }
.sf-prose video { margin-block: var(--sf-prose-media-margin); border-radius: var(--sf-prose-media-radius); }

Expand All @@ -72,7 +72,7 @@
.sf-prose .sf-not-prose img { margin-block: 0; border-radius: 0; }
.sf-prose .sf-not-prose figure { margin-block: 0; }
.sf-prose .sf-not-prose figcaption { font-size: inherit; color: inherit; margin-block-start: 0; }
.sf-prose .sf-not-prose table { margin-block: 0; display: revert; overflow-x: revert; max-width: revert; }
.sf-prose .sf-not-prose table { margin-block: 0; display: revert; overflow-inline: revert; inline-size: revert; max-inline-size: revert; }
.sf-prose .sf-not-prose :is(th,td) { padding: revert; }
.sf-prose .sf-not-prose video { margin-block: 0; border-radius: 0; }

Expand Down Expand Up @@ -149,11 +149,11 @@
/* -- Scroll shadow -------------------------------------------
Top + bottom mask gradient that reveals when content is
scrolled inside a vertical-scroll container. Pure CSS,
no JS. Combine with overflow-y: auto on the same element.
no JS. Combine with overflow-block: auto on the same element.
Override: style="--sf-scroll-shadow-size: 3rem"
---------------------------------------------------------- */
.sf-scroll-shadow {
overflow-y: auto;
overflow-block: auto;
/* stylelint-disable-next-line property-no-vendor-prefix -- required for Safari < 15.4 */
-webkit-mask-image:
linear-gradient(to bottom,
Expand All @@ -175,9 +175,9 @@
For horizontal snap, use the .sf-reel layout primitive.
---------------------------------------------------------- */
.sf-scroll-snap {
overflow-y: auto;
scroll-snap-type: y mandatory;
overscroll-behavior-y: contain;
overflow-block: auto;
scroll-snap-type: block mandatory;
overscroll-behavior-block: contain;
Comment on lines +178 to +180

@coderabbitai coderabbitai Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Test expects scrollSnapType to contain 'y', but scroll-snap-type: block computes differently.

The test at tests/macros.spec.js:154-169 asserts expect(cs).toContain('y') for .sf-scroll-snap. However, scroll-snap-type: block mandatory computes to 'block mandatory' in getComputedStyle(), not 'y mandatory'. The test will fail unless updated to check for 'block' instead of 'y'.

#!/bin/bash
# Verify the test assertion that will break
rg -n -B2 -A8 "scrollSnapType" tests/macros.spec.js
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/macros.css` around lines 178 - 180, The test currently asserts
expect(cs).toContain('y') for the .sf-scroll-snap rule but the CSS uses
scroll-snap-type: block mandatory which computes to 'block mandatory'; update
the test assertion to look for 'block' (e.g. expect(cs).toContain('block')) or
otherwise check for the computed value 'block mandatory' instead of 'y' so it
matches the actual computed scroll-snap-type produced by the .sf-scroll-snap
rule; alternatively, if you prefer to keep the test, change the CSS value for
scroll-snap-type to an axis-based token that computes to 'y', but the simpler
fix is to update the test expectation.

}
.sf-scroll-snap > * {
scroll-snap-align: start;
Expand Down
2 changes: 1 addition & 1 deletion core/reset.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
/* stylelint-disable-next-line property-no-vendor-prefix -- Safari <17: unprefixed text-size-adjust only landed in Safari 17 */
-webkit-text-size-adjust: 100%;
text-size-adjust: 100%;
scroll-padding-top: var(--sf-header-height, 5rem);
scroll-padding-block-start: var(--sf-header-height, 5rem);
scrollbar-gutter: stable;
color-scheme: var(--sf-color-scheme, light dark);
hanging-punctuation: first last;
Expand Down
2 changes: 1 addition & 1 deletion docs/api-index.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30190,7 +30190,7 @@
"category": "Macro classes",
"area": "macros",
"group": "Scroll shadow",
"description": "Top + bottom mask gradient that reveals when content is scrolled inside a vertical-scroll container. Pure CSS, no JS. Combine with overflow-y: auto on the same element. Override: style=\"--sf-scroll-shadow-size: 3rem\"",
"description": "Top + bottom mask gradient that reveals when content is scrolled inside a vertical-scroll container. Pure CSS, no JS. Combine with overflow-block: auto on the same element. Override: style=\"--sf-scroll-shadow-size: 3rem\"",
"isVariant": false,
"baseClass": null,
"optional": false,
Expand Down
2 changes: 1 addition & 1 deletion docs/api-index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ and a short description. The machine-readable companion (with all columns) is
| `.sf-scrim--bottom` | PUBLIC | macro | Scrim | text anchored at the top |
| `.sf-scrim--full` | PUBLIC | macro | Scrim | text anchored at the bottom (default) |
| `.sf-scrim--top` | PUBLIC | macro | Scrim | Lift content above the scrim, but leave media below it so the gradient actually darkens the picture. |
| `.sf-scroll-shadow` | PUBLIC | macro | Scroll shadow | Top + bottom mask gradient that reveals when content is scrolled inside a vertical-scroll container. Pure CSS, no JS. Combine with overflow-y: auto on the same element. Override: style="--sf-scroll-shadow-size: 3rem" |
| `.sf-scroll-shadow` | PUBLIC | macro | Scroll shadow | Top + bottom mask gradient that reveals when content is scrolled inside a vertical-scroll container. Pure CSS, no JS. Combine with overflow-block: auto on the same element. Override: style="--sf-scroll-shadow-size: 3rem" |

@coderabbitai coderabbitai Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use block-axis wording here.

The description still uses physical-axis phrasing (“vertical-scroll”, “top + bottom”), which is misleading in a logical-property audit. Please reword it in block-axis terms so the docs stay correct for vertical writing modes.

♻️ Suggested wording 
- Top + bottom mask gradient that reveals when content is scrolled inside a vertical-scroll container.
+ Block-axis mask gradient that reveals when content is scrolled inside a block-scroll container.
📝 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.

Suggested change
| `.sf-scroll-shadow` | PUBLIC | macro | Scroll shadow | Top + bottom mask gradient that reveals when content is scrolled inside a vertical-scroll container. Pure CSS, no JS. Combine with overflow-block: auto on the same element. Override: style="--sf-scroll-shadow-size: 3rem" |
| `.sf-scroll-shadow` | PUBLIC | macro | Scroll shadow | Block-axis mask gradient that reveals when content is scrolled inside a block-scroll container. Pure CSS, no JS. Combine with overflow-block: auto on the same element. Override: style="--sf-scroll-shadow-size: 3rem" |
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/api-index.md` at line 1055, The docs entry for the `.sf-scroll-shadow`
macro uses physical-axis phrasing; update the description to use
block-axis/logical-property wording: replace "Scroll shadow | Top + bottom mask
gradient that reveals when content is scrolled inside a vertical-scroll
container. Pure CSS, no JS. Combine with overflow-block: auto on the same
element." with a block-axis phrasing such as "Scroll shadow | Mask gradient at
the block-start and block-end that reveals when content is scrolled along the
block axis. Pure CSS, no JS. Combine with overflow-block: auto on the same
element. Override: style=\"--sf-scroll-shadow-size: 3rem\"" so the entry
references block-start/block-end and scrolling along the block axis and keeps
the existing override note; ensure the symbol `.sf-scroll-shadow` remains
unchanged.

| `.sf-scroll-snap` | PUBLIC | macro | Scroll snap | Vertical scroll-snap container for vertically stacked sections. Pairs with consumer-supplied scroll-snap-align on children. For horizontal snap, use the .sf-reel layout primitive. |
| `.sf-surface` | PUBLIC | macro | Surface | Generic surface macro. Set --sf-surface-color on an element with .sf-surface to derive background, auto-contrast foreground, and contextual surface tokens. 11 precomputed variants (.sf-surface--primary through .sf-surface--danger) cover all brand + status + inverse colors.… |
| `.sf-surface--action` | PUBLIC | macro | Surface | Generic surface macro. Set --sf-surface-color on an element with .sf-surface to derive background, auto-contrast foreground, and contextual surface tokens. 11 precomputed variants (.sf-surface--primary through .sf-surface--danger) cover all brand + status + inverse colors.… |
Expand Down
8 changes: 2 additions & 6 deletions docs/roadmap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,8 @@ Current version: **0.5.41**

## Before v1.0

- **Logical property audit** — replace remaining physical properties
(`margin-left`, `padding-right`) with logical equivalents
(`margin-inline-start`, `padding-inline-end`) for RTL support.
- **Scoped token snapshot tests** — Playwright snapshot assertions for ~20 key
semantic tokens (`--sf-color-text`, `--sf-color-primary`, …) in both modes, to
catch value regressions during releases.
*All pre-v1.0 items are complete as of v0.5.42. The API is frozen for
non-additive changes; v0.6.x will focus on the components layer.*

## Post-1.0

Expand Down
3 changes: 3 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tests/demo-visual.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ test.describe('Layout — Reel', () => {
const snapType = await getStyle(reel, 'scrollSnapType');
expect(display).toBe('flex');
expect(overflow).toBe('auto');
expect(snapType).toContain('x');
expect(snapType).toContain('inline');
});

test('.sf-reel children have scroll-snap-align', async ({ page }) => {
Expand Down
2 changes: 1 addition & 1 deletion tests/layout.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ test.describe('layout: .sf-reel', () => {
}));
expect(cs.display).toBe('flex');
expect(cs.overflow).toBe('auto');
expect(cs.snap).toContain('x');
expect(cs.snap).toContain('inline');
expect(cs.snap).toContain('mandatory');
});

Expand Down
119 changes: 119 additions & 0 deletions tests/token-semantic.snapshot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
{
"colors.light": {
"--sf-color-text": {
"r": 2,
"g": 6,
"b": 15
},
"--sf-color-heading": {
"r": 2,
"g": 6,
"b": 15
},
"--sf-color-link": {
"r": 0,
"g": 85,
"b": 180
},
"--sf-color-bg": {
"r": 245,
"g": 249,
"b": 252
},
"--sf-color-primary": {
"r": 1,
"g": 55,
"b": 238
},
"--sf-color-secondary": {
"r": 17,
"g": 26,
"b": 45
},
"--sf-color-action": {
"r": 0,
"g": 108,
"b": 204
},
"--sf-color-border": {
"r": 210,
"g": 212,
"b": 216
},
"--sf-color-text--muted": {
"r": 97,
"g": 105,
"b": 120
},
"--sf-color-text--on-primary": {
"r": 238,
"g": 238,
"b": 238
}
},
"colors.dark": {
"--sf-color-text": {
"r": 227,
"g": 236,
"b": 251
},
"--sf-color-heading": {
"r": 227,
"g": 236,
"b": 251
},
"--sf-color-link": {
"r": 0,
"g": 173,
"b": 255
},
"--sf-color-bg": {
"r": 30,
"g": 31,
"b": 33
},
"--sf-color-primary": {
"r": 81,
"g": 150,
"b": 255
},
"--sf-color-secondary": {
"r": 191,
"g": 203,
"b": 227
},
"--sf-color-action": {
"r": 0,
"g": 173,
"b": 255
},
"--sf-color-border": {
"r": 67,
"g": 69,
"b": 72
},
"--sf-color-text--muted": {
"r": 147,
"g": 156,
"b": 170
},
"--sf-color-text--on-primary": {
"r": 3,
"g": 3,
"b": 3
}
},
"raw": {
"--sf-text-m": "calc(clamp(calc(1 * 1rem), calc((1.25 - 1) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * 1rem), calc(1.25 * 1rem)) * 1)",
"--sf-text-s": "calc(clamp(calc(1 * pow(1.25, -1) * 1rem), calc((1.25 * pow(1.333, -1) - 1 * pow(1.25, -1)) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * pow(1.25, -1) * 1rem), calc(1.25 * pow(1.333, -1) * 1rem)) * 1)",
"--sf-text-l": "calc(clamp(calc(1 * pow(1.25, 1) * 1rem), calc((1.25 * pow(1.333, 1) - 1 * pow(1.25, 1)) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * pow(1.25, 1) * 1rem), calc(1.25 * pow(1.333, 1) * 1rem)) * 1)",
"--sf-h1-size": "calc(clamp(calc(1 * pow(1.25, 5) * 1rem), calc((1.25 * pow(1.333, 5) - 1 * pow(1.25, 5)) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * pow(1.25, 5) * 1rem), calc(1.25 * pow(1.333, 5) * 1rem)) * 1)",
"--sf-space-m": "calc(clamp(calc(1 * 1rem), calc((2 - 1) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * 1rem), calc(2 * 1rem)) * 1)",
"--sf-space-l": "calc(clamp(calc(1 * pow(1.25, 1) * 1rem), calc((2 * pow(1.333, 1) - 1 * pow(1.25, 1)) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * pow(1.25, 1) * 1rem), calc(2 * pow(1.333, 1) * 1rem)) * 1)",
"--sf-gap": "calc(clamp(calc(1 * 1rem), calc((2 - 1) / (90 - 22.5) * (100vw - 22.5 * 1rem) + 1 * 1rem), calc(2 * 1rem)) * 1)",
"--sf-radius-m": "calc(8px * 1)",
"--sf-border-width-1": "1px",
"--sf-font-weight-normal": "400",
"--sf-font-weight-bold": "700"
}
}
Loading