Add comprehensive public API audit for SLASHED v0.3.0 - #94
Add comprehensive public API audit for SLASHED v0.3.0#94jackgranatowski wants to merge 1 commit into
Conversation
Full audit of SLASHED v0.3.0 public API covering token contract completeness (A), naming consistency (B), class completeness (C), cascade layer correctness (D), bundle consistency (E), doc sync (F), and API-freeze concerns (G). 2 critical and 15 orange findings. https://claude.ai/code/session_015tffPLQtpF4fh1ogtwGFpE
📝 WalkthroughWalkthroughThis PR introduces a comprehensive pre-freeze audit of the public API (v0.3.0) that systematically identifies contract incompleteness, naming inconsistencies, code-documentation misalignment, cascade semantic risks, and freeze-readiness issues across seven audit categories with a consolidated risk priority table. ChangesPublic API v0.3.0 Pre-Freeze Audit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@audits/audit-c.md`:
- Around line 77-79: The markdown fenced code blocks containing examples like
"--sf-stack-gap → --sf-space-content → --sf-content-gap → --sf-space-s",
"--sf-cluster-gap → --sf-space-gap → --sf-gap → --sf-space-m", and
"…slashed.states, slashed.themes…" (and the other occurrences around the same
areas) are missing language identifiers; update each triple-backtick fence to
include a language tag (e.g., ```text) so the blocks become ```text ... ``` to
satisfy markdownlint MD040 and improve rendering for the blocks showing the SF
token chains and slashed.* examples.
- Around line 329-330: The audit text currently claims view() timelines require
ancestors with container-type or overflow: visible; update the wording in
audits/audit-c.md to state instead that animation-timeline: view uses the
subject element’s nearest ancestor scroll container and a view timeline is only
created if that nearest ancestor has scrollable overflow (i.e., an ancestor
scroll container that can actually scroll), removing the incorrect
“container-type/overflow: visible” requirement; also add brief citations to the
spec/MDN (e.g., MDN `view-timeline`, `animation-timeline: view`,
`view-timeline-inset`, and CSS Scroll‑driven Animations Module Level 1) to
support the corrected rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e82126f-0beb-4a40-92d9-c84e776a7ddb
📒 Files selected for processing (1)
audits/audit-c.md
| ``` | ||
| --sf-stack-gap → --sf-space-content → --sf-content-gap → --sf-space-s | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
These fences are missing language tags, which triggers markdownlint MD040 and reduces readability in rendered docs.
Suggested patch
-```
+```text
--sf-stack-gap → --sf-space-content → --sf-content-gap → --sf-space-s- +text
--sf-cluster-gap → --sf-space-gap → --sf-gap → --sf-space-m
-```
+```text
…slashed.states, slashed.themes…
Also applies to: 84-86, 272-274
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@audits/audit-c.md` around lines 77 - 79, The markdown fenced code blocks
containing examples like "--sf-stack-gap → --sf-space-content → --sf-content-gap
→ --sf-space-s", "--sf-cluster-gap → --sf-space-gap → --sf-gap → --sf-space-m",
and "…slashed.states, slashed.themes…" (and the other occurrences around the
same areas) are missing language identifiers; update each triple-backtick fence
to include a language tag (e.g., ```text) so the blocks become ```text ... ```
to satisfy markdownlint MD040 and improve rendering for the blocks showing the
SF token chains and slashed.* examples.
| Klasy są w bundlu `essential`, mają tokeny (`--sf-scroll-timeline-range-*`), Safari fallback, ale zero dokumentacji poza kodem. W szczególności: brak opisu że te klasy potrzebują ancestor z `container-type` lub `overflow: visible` aby `view()` timeline działał. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For CSS scroll-driven animations using animation-timeline: view(), is an ancestor with container-typeoroverflow: visible required? Please provide MDN/spec references and browser-compat notes.
💡 Result:
No. You do not need an ancestor with container-type, and you do not need overflow: visible. For animation-timeline: view, the key requirement is that the subject element has a nearest ancestor scroller (a “nearest ancestor scroll container”) whose scrollport is created by having scrollable overflow; if the scroller “does not overflow its container in the axis dimension or if the overflow is hidden or clipped, no scroll progress timeline will be created” [1]. MDN / reference details 1) How scroller lookup works for view - view “create[s] an anonymous view progress timeline based on when an element comes into view inside its nearest scroll container” [2]. - MDN’s view documentation also reiterates that the view timeline is based on the subject’s visibility within its nearest ancestor scroller [3]. 2) Overflow requirements (not container-type) - MDN’s view-timeline shorthand documentation explicitly states that if the scroller “does not overflow its container in the axis dimension or if the overflow is hidden or clipped, no scroll progress timeline will be created” [1]. - MDN’s view-timeline-inset property includes the same requirement: if “the scroller element does not overflow its container in the axis dimension or if the overflow is hidden or clipped, no scroll progress timeline will be created” [4]. So for view, overflow: visible is just the default value, but what matters is whether there is a scroll container with scrollable overflow; and if the nearest scroll container’s overflow is hidden or clipped, the view progress timeline won’t be created [1][4]. container-type does not appear as a prerequisite in these references. Spec (CSS Scroll-driven Animations Module Level 1) - The spec defines that a ViewTimeline is “inactive” if its source or subject’s nearest ancestor scroll container has “no scrollable overflow” (or if there is no such ancestor, e.g. print media) [5]. This aligns with the MDN language about “no scroll progress timeline … created” [1][4]. Browser-compat notes (practical) - Chrome and other engines will typically treat overflow: hidden / overflow: clip as preventing the relevant scroll progress timeline from being created for the view-based timeline (because the scroll container ends up effectively not providing scrollable overflow for the ViewTimeline), as reflected by MDN’s warning that hidden/clipped overflow results in no timeline [1][4]. - The Chrome developer guide also describes View Timelines as depending on the subject being tracked within the scroller/view region (named/anonymous view timelines) but does not add any container-type requirement [6]. Answering the question directly - container-type ancestor required? No evidence from MDN/spec; view tracks the subject’s visibility within its nearest ancestor scroll container [2][3][5]. - overflow: visible ancestor required? No. What is required is a nearest ancestor scroll container with scrollable overflow; if the nearest scroll container’s overflow is hidden or clipped (or otherwise doesn’t create scrollable overflow in the axis), the view progress timeline is inactive / not created [1][4][5].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/view-timeline
- 2: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/animation-timeline/view
- 3: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_scroll-driven_animations/Timelines
- 4: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/view-timeline-inset
- 5: https://drafts.csswg.org/scroll-animations/
- 6: https://developer.chrome.google.cn/docs/css-ui/scroll-driven-animations
Fix the audit’s view() ancestor requirement claim (it’s not accurate).
For animation-timeline: view, there’s no requirement for ancestors with container-type, and overflow: visible is not needed. The ViewTimeline is based on the subject element’s nearest ancestor scroll container; if that nearest scroll container doesn’t have scrollable overflow (e.g., overflow is hidden/clipped so no axis scrollable overflow), the view progress timeline won’t be created. Update the audit wording at audits/audit-c.md (lines 329-330) to reflect the “nearest ancestor scroll container with scrollable overflow” rule and cite the spec/MDN (e.g., MDN view-timeline, animation-timeline: view, view-timeline-inset, and CSS Scroll-driven Animations Module Level 1).
🧰 Tools
🪛 LanguageTool
[typographical] ~329-~329: Przed spójnikiem „że” stawiamy przecinek: "opisu, że".
Context: ...tacji poza kodem. W szczególności: brak opisu że te klasy potrzebują ancestor z `contain...
(BRAK_PRZECINKA_ZE)
🤖 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 `@audits/audit-c.md` around lines 329 - 330, The audit text currently claims
view() timelines require ancestors with container-type or overflow: visible;
update the wording in audits/audit-c.md to state instead that
animation-timeline: view uses the subject element’s nearest ancestor scroll
container and a view timeline is only created if that nearest ancestor has
scrollable overflow (i.e., an ancestor scroll container that can actually
scroll), removing the incorrect “container-type/overflow: visible” requirement;
also add brief citations to the spec/MDN (e.g., MDN `view-timeline`,
`animation-timeline: view`, `view-timeline-inset`, and CSS Scroll‑driven
Animations Module Level 1) to support the corrected rule.
2 participants
Summary
This PR adds a detailed public API audit document (
audits/audit-c.md) for SLASHED v0.3.0, conducted on 2026-05-25. The audit comprehensively reviews the framework's CSS token system, naming conventions, class definitions, cascade layers, bundle configuration, and documentation before the planned API freeze.Key Findings
The audit identifies 17 priority issues across 7 categories:
Critical Issues (🔴)
Major Issues (🟠)
--sf-field-border-color,--sf-field-text-color,--sf-color-code-block-bg/textused across files but not formally declared in tokens.css or PUBLIC API list--sf-icon-size,--sf-bento-cols,--sf-bento-rowenable inline style overrides but lack formal PUBLIC status or documentationsf-spinandsf-shimmerkeyframes lack corresponding--sf-animation-*tokens, breaking consistency with other animations.is-dragginguses hardcodedopacity: 0.5(should use--sf-opacity-50),.is-pendinguses0.7(outside opacity scale),.skip-linkusesz-index: 9999(should use--sf-z-max).sf-entrance--*(6 animation classes) exist in bundle but have zero documentationMinor Issues (🟡)
.sf-icon--boxed,.sf-section--collapse,.sf-alternatetoken behavior.visually-hiddenand.skip-linkmissing from states.md documentationRecommendations
Before API freeze:
--sf-animation-spinand--sf-animation-shimmertokens.sf-entrance--*classes and their container-type requirementshttps://claude.ai/code/session_015tffPLQtpF4fh1ogtwGFpE
Summary by CodeRabbit