chore: sync dev changes to main - #304
Conversation
* fix(build): preserve css imports in sideEffects * fix(build): preserve css side effects in template * chore(changeset): add css sideEffects fix note * chore(changeset): fix package list and description
* refactor(flex): align story options with constants - replace hardcoded story options with Flex constants - keep Storybook controls aligned with the current contract values - reduce the risk of drift between implementation and story definitions * test(flex): strengthen contract coverage - align tests with the current Flex contract in side - use constants as the source of truth for direction, align, justify, and wrap cases - add coverage for normal values, asChild behavior, style overrides, ref forwarding, and common consumer usage patterns - verify the Flex test suite passes at the package level * test(flex): translate test descriptions to english --------- Co-authored-by: KYBee <kybee@KYBeeui-Macmini.local>
* feat: primitive token * fix: ci error * docs: update spacing primitive token * docs: update radius primitive token
* fix(tooltip): add aria-label for screen readers * refactor(tooltip): migrate styles to vanilla-extract recipe * test(tooltip): improve accessibility test coverage * refactor(tooltip): use enum instead of string literal * fix(tooltip): resolve type import error * test(tooltip): translate test descriptions to English * chore(tooltip): remove export and add changeset
* chore(chip): normalize package.json structure * chore(packages): align package.json across workspace * chore(tooling): add package.json consistency checker * chore(packages): add clean script to template and accordion * docs(claude): document package consistency policy * fix(tooling): tighten allowlist fieldPath matching * fix(tooling): harden policy allowlist validation * feat(tooling): police sideEffects field in package policy * fix(ci): mark consistency install step as non-blocking * chore: drop per-package storybook scripts * chore(tooling): promote test to hard rule, drop optionalScripts * fix(tooling): allow publishConfig hard rules to be allowlisted * docs(claude): sync package policy with enforced rules * chore(deps): sync lockfile with workspace catalogs * chore(tooling): namespace lint scripts (lint:biome, lint:package)
…as strategy (#258) * feat(tokens): update primitive token JSON with extended scale Add missing color shades (gray.950, status colors), expanded radius, spacing, and typography scales to align with semantic token requirements. Confidence: high Scope-risk: narrow * feat(tokens): add semantic token JSON draft in DTCG format Introduce dark-mode semantic tokens for color, spacing, radius, and typography. All values reference primitive aliases only — no raw hex or px values. Each token includes two usage-scenario descriptions. Light-mode tokens are deferred (tokens/semantic/light/ has .gitkeep only). Confidence: high Scope-risk: narrow * feat(tokens): redesign CSS contract to match semantic token structure Rewrite vars contract to reflect semantic layers (color.background.*, color.foreground.*, color.accent.*, spacing.component.*, spacing.layout.*, radius.component.*, radius.layout.*). Extract darkBaseColor shared object and define four brand theme variants (default, 1st–4th). Constraint: dark mode is the default; light mode is deferred Rejected: per-theme contract files | increases maintenance surface Confidence: high Scope-risk: broad * feat(button): migrate styles to new semantic vars paths Update all token references to match the redesigned CSS contract: color.primary → color.accent.default, spacing.sm/lg → spacing.component.*, radius.md → radius.component.md, focus ring wired to color.border.focus. Confidence: high Scope-risk: narrow * chore(tokens): mark unused TS primitive exports as deprecated Add @deprecated JSDoc to opacity, zIndex, borderWidth/borderStyle, shadows, spacing, breakpoints, grid, and responsiveStyle. These TS source files will be removed alongside all other TS primitives once the JSON token pipeline is complete and components have migrated to vars.*. Directive: do not add new consumers of these exports Confidence: high Scope-risk: narrow * chore: changeset * feat(tokens): add brand primitive and update accent semantic alias - Add color.brand.{default,hover,subtle} to primitive/color.json with current cohort values (#ffb24d, #d9963f, #3b2005) - Update semantic accent.* to reference {color.brand.*} instead of blue to reflect actual brand color brand primitive acts as the single update point per cohort; semantic tokens and components require no changes on rebranding. * feat(tokens): add brandColor and update defaultTheme accent to orange - Export brandColor const from colors.ts (mirrors primitive brand.*) - Import brandColor in themes.css.ts and apply to defaultTheme accent replacing hardcoded blue values Note: colors.ts and themes.css.ts are deprecated and will be removed once the Style Dictionary pipeline is in place. * fix(theme): correct assignInlineVars contract mapping in ThemeProvider assignInlineVars(vars.color, theme) was passing ThemeColor { primary, secondary, background, text, gradient } to a contract that expects { accent, foreground, border, status, ... }, causing a TypeScript error and no-op CSS variable assignment. Fix by scoping to vars.color.accent and mapping ThemeColor fields: - primary → accent.default - secondary → accent.hover - background → accent.subtle * docs(tokens): add semantic token proposal with rebrand strategy - Document semantic token structure (color, spacing, radius, typography) - Add rebranding flow section explaining brand primitive alias pattern: update color.brand.* in primitive/color.json only on cohort change - Note W3C Design Token format compatibility with Token Studio v2 and Style Dictionary v4 for upcoming Figma pipeline integration * fix(tokens): remove duplicate named exports from themes.css.ts theme1st–theme4th were exported from both colors.ts (ThemeColor objects) and themes.css.ts (createGlobalTheme return values), causing TS2308 ambiguous re-export errors in src/index.ts. createGlobalTheme calls are CSS side effects; their return values are not consumed by any external code, so export keywords are removed. * Release (#256) Co-authored-by: KYBee <kybee@KYBeeui-Macmini.local> Co-authored-by: Jinho Yeom <81306489+Yeom-JinHo@users.noreply.github.com> Co-authored-by: 이원주 <101818687+3o14@users.noreply.github.com> Co-authored-by: YOUNGBEEN, KIM <62539910+KYBee@users.noreply.github.com> Co-authored-by: 오소현 <53892427+osohyun0224@users.noreply.github.com> Co-authored-by: Minji Jeon <105787441+minji0214@users.noreply.github.com> * chore: update changeset BC * docs: update semantic-proposal docs * docs: modify semantic proposal muted token * fix: modify spacing, radius themes * chore: update deprecated annotation --------- Co-authored-by: Evan <clcl6084@gmail.com> Co-authored-by: KYBee <kybee@KYBeeui-Macmini.local> Co-authored-by: Jinho Yeom <81306489+Yeom-JinHo@users.noreply.github.com> Co-authored-by: YOUNGBEEN, KIM <62539910+KYBee@users.noreply.github.com> Co-authored-by: 오소현 <53892427+osohyun0224@users.noreply.github.com> Co-authored-by: Minji Jeon <105787441+minji0214@users.noreply.github.com>
…e tokens (#261) * feat: style dictionary pipeline * fix(tokens): adopt W3C DTCG format for primitives and fix token-names publish export * fix: restore broken pnpm-lock.yaml after merge conflict resolution
…ation (#270) * chore: formatting * feat(tokens): setup design token transformation pipeline and CI automation * ci: commit transformed tokens
…259) * refactor(tooltip): rename useTooltip.tsx to useTooltip.ts * perf(tooltip): throttle scroll/resize position update with rAF * feat(tooltip): support keyboard focus for hover trigger * refactor(tooltip): replace trigger prop with MUI-style listener controls * fix(tooltip): remove redundant isVisible condition in portal className * refactor(tooltip): remove redundant code and improve WCAG compliance - Remove always-true visible class from portal (portal only renders when isVisible=true) - Remove associated CSS opacity/transform/transition/pointerEvents overrides - Remove redundant position:fixed from inline styles (already in CSS base) - Remove redundant --tooltip-bg-color JS fallback (CSS already has var fallback) - Conditionally apply tabIndex=0 only when focus listener is active (WCAG 2.4.7) - Apply styles.button class only when asChild=false (WCAG 1.4.3) * chore: downgrade esbuild to ~0.24.0 to fix Storybook manager build esbuild 0.27 changed its behavior to refuse transforming syntax for old browser targets (es2020, chrome87). Storybook 8.5 manager build uses these hardcoded targets and breaks with 0.27. Downgrade to 0.24 as a temporary fix until Storybook is upgraded to support esbuild 0.27. * Revert "chore: downgrade esbuild to ~0.24.0 to fix Storybook manager build" This reverts commit e3e8a95. * refactor(tooltip): remove disableFocusListener prop Focus listener is always required for keyboard accessibility (WCAG 2.1.1). Removing this prop enforces accessible defaults and simplifies the API. * feat(tooltip): add fade-in animation on mount Uses @Keyframes instead of CSS transition to correctly animate on DOM insertion. Respects prefers-reduced-motion for accessibility (WCAG 2.3.3). * fix(tooltip): prevent focus event from interfering with click-controlled open state On first click, the browser fires focus before click, causing onFocus→requestOpen to set open=true before the onClick toggle sets it back to false. Fixed by tracking mousedown state so focus triggered by mouse click is ignored (keyboard focus still works). * chore: add changeset for tooltip API refactor * refactor(tooltip): restore disableFocusListener, remove hardcoded tabIndex, spread rest props - Restore disableFocusListener prop for click-only controlled mode support - Remove hardcoded tabIndex={0} to respect consumer element focusability - Spread ...rest onto wrapper so consumers can pass tabIndex, role, aria-* etc. - Merge className from rest with internal styles via clsx * test(tooltip): add missing coverage for mouseUp, scroll/resize, invalid placement * refactor(tooltip): apply PR review feedback - Change changeset from minor to major (trigger prop removal is breaking change) - Compose user event handlers with internal handlers via composeHandlers helper - Make placement and gap optional in useTooltip with default values * fix(tooltip): guard setInternalOpen with isControlled check in controlled mode Co-Authored-By: MinjiJeon <jinnyjeon@sweetspot.co.kr> --------- Co-authored-by: MinjiJeon <jinnyjeon@sweetspot.co.kr>
…n system (#263) * feat(button): 5세대 디자인 시스템 기반 버튼 컴포넌트 전면 개편 * feat(button): add to changeset docs * fix(Button): use Slottable for multi-child asChild compatibility * fix(Button): apply to typecheck CI
…de toggle (#282) * chore: replace existing data-theme * chore: update theme provider render and themes.css.ts * chore: claude allow reading tokens/dist * chore: update theme packages * chore: update themes css * chore: changeset * chore: theme utils test * chore: update changeset * chore: delete optional * chore: modify changeset * chore: add TODO comments for pending light mode * chore: widen tokens dist read permission scope
* Release (#256) Co-authored-by: KYBee <kybee@KYBeeui-Macmini.local> Co-authored-by: Jinho Yeom <81306489+Yeom-JinHo@users.noreply.github.com> Co-authored-by: 이원주 <101818687+3o14@users.noreply.github.com> Co-authored-by: YOUNGBEEN, KIM <62539910+KYBee@users.noreply.github.com> Co-authored-by: 오소현 <53892427+osohyun0224@users.noreply.github.com> Co-authored-by: Minji Jeon <105787441+minji0214@users.noreply.github.com> * Version Packages (#260) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: add title option to release workflow (#266) --------- Co-authored-by: Evan <clcl6084@gmail.com> Co-authored-by: KYBee <kybee@KYBeeui-Macmini.local> Co-authored-by: Jinho Yeom <81306489+Yeom-JinHo@users.noreply.github.com> Co-authored-by: YOUNGBEEN, KIM <62539910+KYBee@users.noreply.github.com> Co-authored-by: 오소현 <53892427+osohyun0224@users.noreply.github.com> Co-authored-by: Minji Jeon <105787441+minji0214@users.noreply.github.com> Co-authored-by: y09n <euihyun.yang.9x@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…291) packages/tokens/tsup.config.ts was the only package that redefined the root tsup config instead of inheriting it, and it omitted the vanilla-extract esbuild plugin. Two failures followed: - contract.css.ts was bundled as plain TS, so importing dist threw at runtime ("Styles were unable to be assigned to a file"). This also broke @sipe-team/side, which re-exports @sipe-team/tokens. - The --side-* bridge CSS was never emitted, and no CSS entrypoint existed to load the token values. Button — the only component consuming vars.* — rendered with padding, border-radius, font-size, font-weight, font-family and gap all collapsed to initial values. Inherit the root tsup config, and ship a single ./styles.css that concatenates the Style Dictionary value layer with the vanilla-extract bridge — splitting them would let a consumer load half the chain and break silently. sideEffects moves from false to ["**/*.css"] for a separate reason: it is what stops a downstream bundler from dropping the consumer's `import '@sipe-team/tokens/styles.css'`. It was not the cause of the missing bridge CSS (verified: the bridge emits fine under sideEffects:false once the plugin is present). 17 of 21 packages already use this value. tokens.yaml called `build:tokens` standalone, which no longer works now that the Style Dictionary step concatenates the tsup-emitted bridge — point it at `build` (tsup && build:tokens). Verified: dangling var() references in the built Storybook bundle go from 11 to 0; importing tokens/dist no longer throws; a clean checkout running the Tokens workflow's commands exits 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(accordion,grid): compile vanilla-extract styles into dist Both packages declared a local tsup config without the vanilla-extract plugin, so their `.css.ts` styles were never compiled and the `dist/index.css` behind `./styles.css` never made it into the tarball. Reuse the shared root config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reset): point reset.css export at the emitted stylesheet The export referenced `./dist/reset.css`, but the build emits `./dist/index.css`, so `@sipe-team/reset/reset.css` failed to resolve for consumers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(tokens): emit resolvable token-names barrel declarations The generated barrel re-exported `./primitive` and `./semantic` without runtime extensions, so `@sipe-team/tokens/token-names` failed to resolve under node16 ESM. Declaration files need the extension; the types-only semantic layer needs no runtime module, since TypeScript resolves `./semantic.js` to `semantic.d.ts` by extension substitution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build(release): gate publish behind publint and attw Packages point `exports` at `src/` and swap it for `dist/` via `publishConfig.exports`, a pnpm-only behaviour. The published export map is therefore never exercised by a local build, typecheck, or test run — which is how accordion, grid, reset and tokens all shipped export maps that do not resolve. Pack every package with pnpm and validate the resulting tarball, the same bytes the registry receives. `npm pack` cannot be used: it ignores the publishConfig swap, so every entrypoint reports as unresolvable. CSS subpaths are excluded from attw only, since stylesheets never carry type declarations; publint still proves they are in the tarball. `changeset publish` now runs behind this check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the publishability gate on pull requests Gating only at publish time would surface a broken export map after the version PR is already merged. As a parallel job it costs no extra wall-clock, and it gates the changesets version PR itself. The work is skipped when the diff cannot affect a tarball, while the job still runs so it stays usable as a required check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: serialize releases with a concurrency group Two merges landing while a release is in flight would run `changeset publish` concurrently against the registry. Cancellation stays off: interrupting a publish mid-flight is how packages end up half-released. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: drop the paths filter from the publishability gate Skipping the gate when the diff could not affect a tarball took an eleven-line diff step, two step-level conditions, and a full-depth checkout. It bought only runner time on docs-only pull requests — the job runs alongside CI, so it never cost wall-clock to begin with. Not worth the machinery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the publishability check as its own release step The `release` script existed only because changesets/action does not run its `publish` input through a shell, so the check could not be chained onto it. A step before the action gates the publish just as well — a failure there fails the job, so the action never runs. That drops the script, and with it the footgun of a `pnpm release` that looks like a dry run and is not. `pnpm lint:publish` stays, so the gate is still runnable locally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ordion The docs site had no path for rendering a workspace component: www declared no @sipe-team dependency, and workspace `exports` resolves to raw `src/*.ts`, which webpack cannot consume. Add the vanilla-extract webpack plugin so Docusaurus compiles the packages' `.css.ts` from source — docs pages import @sipe-team/* directly, with no prebuilt dist, alias table, CSS injection, or build ordering. Adding a component is then just a www dependency plus its MDX page. - add Preview: renders children server-side with a "Show code" toggle. No react-live, no BrowserOnly — an example that only appeared after hydration would not prove the component server-renders - accordion.mdx with hand-written prop tables read off the source. No prop extraction, so packages/ stays untouched - _TEMPLATE.md fixes the five core sections; Accessibility and Known limitations are optional trailing sections a component adds when warranted Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dark-only components landed as a dark card floating on a light neutral frame, which read as a broken render. Add an opt-in `theme="dark"` to Preview that swaps the stage to a dark canvas, so the example reads as deliberate; the Accordion examples use it. Also compact the stage padding and fold the "Show code" toggle into the card as a left-aligned footer instead of a floating right-aligned button. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(www): document Button and make the token chain survive production
Button is the only component that reads `vars`, so it's the first to need the
Style Dictionary value layer — and it exposed a production-only break: every
`var(--side-*)` resolved to nothing while dev worked and hid it.
Root cause: under source compilation the token bridge was emitted twice — once
by the injected `@sipe-team/tokens/styles.css`, and again by vanilla-extract
recompiling tokens' `themes.css.ts` through Button's `vars` import. Docusaurus's
advanced cssnano collapses the duplicated `@layer theme { … }` to a bare
`@layer theme;`, stripping the whole bridge.
Fix:
- alias `@sipe-team/tokens` to its built dist so its theme is compiled once
(via `build:tokens`); the bridge now has a single copy and full CSS
minification stays on. Components still compile from source
- inject `@sipe-team/tokens/styles.css` for the Style Dictionary value layer
(`--color-*` etc.), which is not vanilla-extract and can't come from source
button.mdx documents variant/size/leftIcon/rightIcon/asChild from source, with
hand-written prop tables. packages/ is untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(www): sharpen Button captions, fix Back arrow, clarify disabled example
- Variant/Size captions now say when to reach for each option instead of just
restating the names, matching the accordion pilot's teaching tone
- the "With icons" example pointed a right chevron at "Back"; split the inline
icon into ArrowLeft/ArrowRight so Back points left and Next points right
- the Disabled example showed two identical unlabelled buttons; label them
Fill/Outline and note that disabled styling is uniform across variants, so the
sameness reads as an intentional demonstration rather than a mistake
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
docs(www): render workspace components in Docusaurus and document Accordion
docs(www): document Avatar (+ fix onError loop and asChild sizing)
…ked warning) (#301) * fix(checkbox): inline SVG marks and consume defaultChecked - inline the check/indeterminate marks as data: URIs instead of url("public/*.svg"); esbuild resolved those paths but source-compiling consumers (webpack + vanilla-extract plugin, e.g. the docs site) could not, breaking their production build - destructure defaultChecked in Root so it seeds the uncontrolled initial state only and no longer leaks through context onto the input alongside the always-set checked, which triggered React's checked+defaultChecked warning Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(www): document Checkbox Add the Checkbox component docs page (Preview-based MDX) covering the Root/Input/Label parts, controlled/uncontrolled usage, indeterminate and disabled states, and the API reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(checkbox): keep the inlined mark SVGs readable Store the check/indeterminate marks as plain SVG markup and encode them with encodeURIComponent at build time (via a small svgToDataUri helper) instead of a hand-encoded data-URI blob. Same portable data-URI output, but the source stays readable and editable. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the Typography component docs page (Preview-based MDX) covering usage, size/weight/color examples, the asChild semantic-element note, anatomy, and the API reference. Also refresh _TEMPLATE.md to describe the source-compilation setup instead of the stale dist-alias note. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(flex): support responsive prop values * test(flex): cover responsive prop contract * docs(flex): add responsive story * fix(flex): export responsive helper types * fix(flex): isolate responsive gap values
🦋 Changeset detectedLatest commit: 3de8949 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe pull request adds publishability checks and release serialization, updates token and package build outputs, introduces responsive Flex props, fixes Avatar and Checkbox behavior, and adds a reusable documentation Preview component with new Accordion, Avatar, Button, Checkbox, and Typography guides. ChangesDesign system build and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/checkbox/src/Checkbox.tsx (1)
52-62: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCover the
defaultCheckedand controlled/uncontrolled paths.The supplied
packages/checkbox/src/Checkbox.test.tsxcoverage only verifies the no-prop default false case. Add tests fordefaultChecked={true}, controlled updates, and uncontrolled user changes.As per path instructions, component tests must cover props, edge cases, and controlled/uncontrolled behavior.
🤖 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 `@packages/checkbox/src/Checkbox.tsx` around lines 52 - 62, Add coverage in Checkbox.test.tsx for defaultChecked={true}, controlled checked updates, and uncontrolled user interaction changes, using the Checkbox component’s existing test patterns. Verify the initial and updated checked states for each path while preserving the existing no-prop default-false coverage.Source: Path instructions
🤖 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 @.github/workflows/ci.yaml:
- Around line 35-38: Update the workflow steps using actions/checkout and
jdx/mise-action to reference trusted full commit SHAs instead of mutable tags,
and add persist-credentials: false to the checkout step’s with configuration.
Preserve the existing pull-request head SHA ref behavior.
In @.storybook/preview.ts:
- Around line 4-7: Move the `@sipe-team/tokens/styles.css` import ahead of
external package imports in the Storybook preview module, keeping the prescribed
import-group ordering and a blank line separating it from the next group.
In `@packages/avatar/src/Avatar.tsx`:
- Around line 47-52: Add a regression test in Avatar.test.tsx for the Avatar
fallback flow: after the initial error swaps the source to fallback, dispatch a
second error on the same image and verify the fallback is not applied again.
Preserve the existing first-fallback assertion and exercise the onError handling
around the Avatar component.
- Around line 47-52: Replace the DOM onerror clearing in Avatar’s image error
handler with a ref or state guard that permits the fallback source to be
assigned only once. Reset this guard whenever the image src changes, and
preserve the existing fallback assignment behavior without relying on
currentTarget.onerror.
In `@packages/checkbox/src/Checkbox.css.ts`:
- Around line 47-52: Update CHECK_ICON_URL and INDETERMINATE_ICON_URL to remove
the hardcoded white SVG fill and use the approved theme-aware token-backed mask
or background color approach. Preserve both icon shapes and ensure their
rendered color resolves through the checkbox theme token contract.
In `@packages/flex/src/Flex.css.ts`:
- Around line 9-14: Update the mediaQuery definition in Flex.css.ts to import
and reuse the breakpoints/breakpointQuery contract from `@sipe-team/tokens`,
replacing the duplicated numeric media-query literals while preserving the
existing md and lg keys and generated query behavior.
In `@packages/flex/src/Flex.test.tsx`:
- Around line 253-280: Add a responsive breakpoint test near the existing “adds
responsive classes for md and lg breakpoint values” case that omits the middle
md value, such as align={{ sm: 'center', lg: 'flex-end' }}. Assert the generated
classes preserve sm and lg while leaving md unspecified so CSS cascade
inheritance remains responsible for the intermediate value, covering the
fallback behavior of getResponsiveClassNames/getResponsiveGapStyle.
In `@packages/tokens/config.js`:
- Around line 298-314: Update the generated declarations in barrelDts to add
local import type bindings for PrimitiveToken and SemanticToken before the
DesignToken alias, while preserving the existing extension-specific re-exports
and cssVar declaration.
In `@www/docs/components/avatar.mdx`:
- Around line 74-77: Update the API table entry for fallback in the avatar
documentation to describe its actual behavior: it supplies the replacement URL
when src fails, rather than being a general replacement image URL. Keep the
existing JSX fallback example and no-src text behavior unchanged.
In `@www/src/components/Preview/index.tsx`:
- Around line 17-37: Add Vitest and React Testing Library coverage for the
Preview component’s showCode behavior: assert the code is initially hidden, then
verify clicking the toggle reveals the code and updates the label, and clicking
again hides the code and restores the label. Configure the test to run in the
happy-dom environment and target the Preview component and its toggle button.
In `@www/src/components/Preview/styles.module.css`:
- Line 50: Update the Stylelint configuration for CSS Modules so the :global()
selector in .code :global(.theme-code-block) is recognized as valid, or add a
narrowly scoped suppression for this selector; preserve the existing styling
behavior and avoid disabling unrelated selector validation.
---
Outside diff comments:
In `@packages/checkbox/src/Checkbox.tsx`:
- Around line 52-62: Add coverage in Checkbox.test.tsx for
defaultChecked={true}, controlled checked updates, and uncontrolled user
interaction changes, using the Checkbox component’s existing test patterns.
Verify the initial and updated checked states for each path while preserving the
existing no-prop default-false coverage.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ff32ffce-42d0-4142-8caf-b3df32661dfe
⛔ Files ignored due to path filters (11)
.changeset/accordion-grid-compile-styles.mdis excluded by!.changeset/**.changeset/bright-flex-responsive.mdis excluded by!.changeset/**.changeset/fix-avatar-error-loop-and-aschild.mdis excluded by!.changeset/**.changeset/fix-checkbox-assets-and-controlled.mdis excluded by!.changeset/**.changeset/fix-token-css-chain.mdis excluded by!.changeset/**.changeset/reset-css-export-path.mdis excluded by!.changeset/**.changeset/tokens-token-names-resolution.mdis excluded by!.changeset/**packages/checkbox/src/public/check.svgis excluded by!**/*.svgpackages/checkbox/src/public/indeterminate.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yamlwww/static/img/avatar/sipe.pngis excluded by!**/*.png
📒 Files selected for processing (34)
.github/workflows/ci.yaml.github/workflows/release.yaml.github/workflows/tokens.yaml.storybook/preview.tspackage-policy.jsonpackage.jsonpackages/accordion/tsup.config.tspackages/avatar/src/Avatar.css.tspackages/avatar/src/Avatar.tsxpackages/button/src/Button.test.tsxpackages/checkbox/src/Checkbox.css.tspackages/checkbox/src/Checkbox.tsxpackages/flex/src/Flex.css.tspackages/flex/src/Flex.stories.tsxpackages/flex/src/Flex.test.tsxpackages/flex/src/Flex.tsxpackages/grid/tsup.config.tspackages/reset/package.jsonpackages/tokens/config.jspackages/tokens/package.jsonpackages/tokens/tsup.config.tsscripts/checkPackageConsistency.tsscripts/checkPublishability.shwww/docs/components/_TEMPLATE.mdwww/docs/components/accordion.mdxwww/docs/components/avatar.mdxwww/docs/components/button.mdxwww/docs/components/checkbox.mdxwww/docs/components/typography.mdxwww/docusaurus.config.tswww/package.jsonwww/src/components/Preview/index.tsxwww/src/components/Preview/styles.module.csswww/src/theme/MDXComponents.tsx
💤 Files with no reviewable changes (1)
- packages/button/src/Button.test.tsx
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| - uses: jdx/mise-action@v2 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin action revisions and disable checkout credential persistence.
This job executes PR-controlled package lifecycle code. Pin both actions to trusted full commit SHAs and set persist-credentials: false; the job does not need Git credentials after checkout.
Suggested hardening
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<trusted-full-commit-sha>
with:
ref: ${{ github.event.pull_request.head.sha }}
- - uses: jdx/mise-action@v2
+ persist-credentials: false
+ - uses: jdx/mise-action@<trusted-full-commit-sha>🧰 Tools
🪛 zizmor (1.26.1)
[warning] 35-37: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yaml around lines 35 - 38, Update the workflow steps
using actions/checkout and jdx/mise-action to reference trusted full commit SHAs
instead of mutable tags, and add persist-credentials: false to the checkout
step’s with configuration. Preserve the existing pull-request head SHA ref
behavior.
Source: Linters/SAST tools
| // Defines the `--side-*` custom properties that components reference. Without this the | ||
| // vanilla-extract bridge resolves to undefined vars and spacing/radius/typography collapse. | ||
| import '@sipe-team/tokens/styles.css'; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move the scoped token import before external imports.
@sipe-team/* imports must precede external packages and be separated by a blank line.
Suggested import order
+// Defines the `--side-*` custom properties that components reference. Without this the
+// vanilla-extract bridge resolves to undefined vars and spacing/radius/typography collapse.
+import '`@sipe-team/tokens/styles.css`';
+
import 'sanitize.css';
import 'sanitize.css/typography.css';
-
-// Defines the `--side-*` custom properties that components reference. Without this the
-// vanilla-extract bridge resolves to undefined vars and spacing/radius/typography collapse.
-import '`@sipe-team/tokens/styles.css`';As per coding guidelines, **/*.{ts,tsx} files must use import order: node → react → @sipe-team/* → @vanilla-extract/* → @radix-ui/* → external packages → relative paths, with blank lines between groups.
🤖 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 @.storybook/preview.ts around lines 4 - 7, Move the
`@sipe-team/tokens/styles.css` import ahead of external package imports in the
Storybook preview module, keeping the prescribed import-group ordering and a
blank line separating it from the next group.
Source: Coding guidelines
| if (fallback) { | ||
| // Clear the handler before swapping so a failing fallback can't re-trigger onError | ||
| // and reassign the same broken URL in an endless loop. | ||
| e.currentTarget.onerror = null; | ||
| e.currentTarget.src = fallback; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a regression test for a failing fallback.
The supplied packages/avatar/src/Avatar.test.tsx case only validates the first fallback swap. Add coverage that dispatches a second error and verifies the fallback is not applied repeatedly.
🤖 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 `@packages/avatar/src/Avatar.tsx` around lines 47 - 52, Add a regression test
in Avatar.test.tsx for the Avatar fallback flow: after the initial error swaps
the source to fallback, dispatch a second error on the same image and verify the
fallback is not applied again. Preserve the existing first-fallback assertion
and exercise the onError handling around the Avatar component.
Source: Path instructions
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate Avatar.tsx and related tests"
git ls-files | rg '(^|/)Avatar\.tsx$|avatar|Avatar' || true
echo
echo "Inspect packages/avatar/src/Avatar.tsx"
if [ -f packages/avatar/src/Avatar.tsx ]; then
nl -ba packages/avatar/src/Avatar.tsx | sed -n '1,140p'
fi
echo
echo "Search for Avatar tests/usages"
rg -n "Avatar|fallback|onError|src =" packages -g '*.tsx' -g '*.ts' -g '*.test.*' || true
echo
echo "Package/dependency versions or React usage hints"
for f in package.json packages/*/package.json pnpm-lock.yaml package-lock.json yarn.lock; do
[ -f "$f" ] && echo "--- $f" && rg -n '"react"|"react-dom"|react-dom-bindings|`@types/react`' "$f" -C 1 || true
doneRepository: sipe-team/side
Length of output: 732
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect packages/avatar/src/Avatar.tsx"
if [ -f packages/avatar/src/Avatar.tsx ]; then
awk '{printf "%6d\t%s\n", NR, $0}' packages/avatar/src/Avatar.tsx | sed -n '1,160p'
fi
echo
echo "Search for Avatar tests/usages"
rg -n "Avatar|fallback|onError|src =" packages -g '*.tsx' -g '*.ts' -g '*.test.*' || true
echo
echo "Package/dependency versions or React usage hints"
for f in package.json packages/*/package.json pnpm-lock.yaml package-lock.json yarn.lock; do
[ -f "$f" ] && echo "--- $f" && rg -n '"react"|"react-dom"|react-dom-bindings|`@types/react`' "$f" -C 1 || true
doneRepository: sipe-team/side
Length of output: 45850
🌐 Web query:
React DOM event handlers addEventListener vs removing assigned onerror property
💡 Result:
In React applications, the distinction between using React event handlers and native DOM event listeners (such as setting an onerror property or using addEventListener) centers on how React manages the DOM [1][2][3]. React Event Handlers vs. Native Listeners React does not attach native event listeners directly to every JSX element [4][3]. Instead, it uses a technique called event delegation: React attaches a small set of listeners to the root container of your application [1][4][3]. When a native event bubbles up to the root, React catches it, wraps it in a cross-browser "SyntheticEvent," and dispatches it to the appropriate React component based on the fiber tree [5][1][4]. 1. React Event Handlers (e.g., onClick, onChange): These should be your default choice [2]. They are automatically cleaned up when components unmount, provide a normalized API across all browsers, and integrate seamlessly with React's scheduling and state management [5][6][3]. 2. Native DOM Listeners (e.g., element.onerror, addEventListener): If you assign a function directly to an element property like onerror, or use addEventListener on a DOM node in a useEffect hook, you are bypassing React's event system [1][2]. Key Risks and Considerations: - Manual Management: Native listeners are not managed by React. You must manually add them (usually in a useEffect hook) and remove them (in the cleanup function) to avoid memory leaks [7][3]. - Event Bubbling Discrepancies: React’s SyntheticEvents only bubble through the React component tree [1][4]. Stopping propagation in a React handler (e.g., e.stopPropagation) does not prevent the event from reaching native listeners attached to the DOM [2][4]. - Global Exceptions (window.onerror): While React handlers manage UI interactions, global error reporting often relies on native window listeners [8]. For example, window.onerror is a legacy property that handles synchronous runtime errors, while window.addEventListener('error', ..., true) is required to catch resource loading errors (like failed images or scripts) that do not bubble [9][8]. Recommendation Always use React’s built-in event handlers for application logic [2]. Use native listeners only when absolutely necessary—such as for global browser events (e.g., window resize, scroll on the document) or specific DOM requirements that React does not expose—and ensure you always clean them up in the appropriate lifecycle hook [1][3].
Citations:
- 1: https://interviewlane.com/questions/how-do-react-synthetic-events-differ-from-native-dom-events
- 2: https://ryankubik.com/blog/dont-mix-react-synthetic-and-native-events
- 3: https://frontendatlas.com/react/trivia/react-why-event-delegation
- 4: https://interviewlane.com/questions/how-does-react-handle-event-delegation
- 5: https://legacy.reactjs.org/docs/events.html
- 6: https://quarterly.dev/article/synthetic-events-in-react
- 7: https://stackoverflow.com/questions/37933733/is-assigning-a-function-to-window-onerror-preferable-to-window-addeventlistener
- 8: https://js-error.com/core-javascript-error-handling-boundaries/mastering-windowonerror-and-global-event-listeners/
- 9: https://developer.mozilla.org/en-US/docs/Web/API/Window/error_event
Do not rely on currentTarget.onerror = null to stop React’s handler.
React attaches the JSX onError through its event system, so clearing the DOM onerror property leaves the React listener registered. If the fallback also fails, the handler can run again and assign the same broken source repeatedly.
Use a ref/state guard so the fallback image is attempted only once and reset the guard when src changes.
🤖 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 `@packages/avatar/src/Avatar.tsx` around lines 47 - 52, Replace the DOM onerror
clearing in Avatar’s image error handler with a ref or state guard that permits
the fallback source to be assigned only once. Reset this guard whenever the
image src changes, and preserve the existing fallback assignment behavior
without relying on currentTarget.onerror.
| const CHECK_ICON_URL = svgToDataUri( | ||
| `<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='white'><path d='M9 16.17L4.83 12l-1.42 1.41L9 19 21 7l-1.41-1.41L9 16.17z'/></svg>`, | ||
| ); | ||
| const INDETERMINATE_ICON_URL = svgToDataUri( | ||
| `<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='white'><path d='M19 13H5v-2h14v2z'/></svg>`, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a theme-aware token for the checkbox icon color.
Both inline SVGs hardcode fill='white', bypassing the token contract and potentially producing poor contrast in light or custom themes. Use a token-backed CSS mask/background approach or another approved theme-aware icon color.
As per coding guidelines, source files must not hardcode color values; as per path instructions, checkbox styles must remain theme-aware and avoid hardcoded colors.
🤖 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 `@packages/checkbox/src/Checkbox.css.ts` around lines 47 - 52, Update
CHECK_ICON_URL and INDETERMINATE_ICON_URL to remove the hardcoded white SVG fill
and use the approved theme-aware token-backed mask or background color approach.
Preserve both icon shapes and ensure their rendered color resolves through the
checkbox theme token contract.
Sources: Coding guidelines, Path instructions
| // Mirrors the current @sipe-team/tokens layout breakpoints. The token export is deprecated, | ||
| // so Flex keeps these values local until replacement breakpoint tokens are available. | ||
| const mediaQuery = { | ||
| md: 'screen and (min-width: 780px)', | ||
| lg: 'screen and (min-width: 1060px)', | ||
| } as const; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether `@sipe-team/tokens` currently exposes any breakpoint-related tokens,
# and whether other packages hardcode the same 780px/1060px values.
rg -n "breakpoint" packages/tokens -i
rg -n "780px|1060px" packages -g '!**/Flex.css.ts'Repository: sipe-team/side
Length of output: 1081
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tokens breakpoint exports =="
sed -n '1,80p' packages/tokens/src/layout/breakpoints.ts
echo
sed -n '1,80p' packages/tokens/src/layout/index.ts
echo
sed -n '1,80p' packages/tokens/src/layout/responsiveStyle.ts
echo
echo "== Flex.css.ts context =="
sed -n '1,220p' packages/flex/src/Flex.css.ts
echo
echo "== package metadata exports for tokens =="
for f in packages/tokens/package.json package.json; do
if [ -f "$f" ]; then
echo "-- $f --"
cat "$f" | sed -n '1,180p'
fi
doneRepository: sipe-team/side
Length of output: 9451
Use the token breakpoint contract instead of copying numeric breakpoints.
@sipe-team/tokens exposes breakpoints and breakpointQuery, and breakpointQuery already generates the same 780px/1060px media queries. Import and reuse those values instead of duplicating broken literals locally.
🤖 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 `@packages/flex/src/Flex.css.ts` around lines 9 - 14, Update the mediaQuery
definition in Flex.css.ts to import and reuse the breakpoints/breakpointQuery
contract from `@sipe-team/tokens`, replacing the duplicated numeric media-query
literals while preserving the existing md and lg keys and generated query
behavior.
Sources: Coding guidelines, Path instructions
| it('adds responsive classes for md and lg breakpoint values', () => { | ||
| render( | ||
| <Flex | ||
| data-testid="flex-container" | ||
| align={{ sm: 'stretch', md: 'center', lg: 'flex-start' }} | ||
| direction={{ sm: 'column', md: 'row', lg: 'row-reverse' }} | ||
| justify={{ sm: 'center', md: 'space-between', lg: 'flex-end' }} | ||
| wrap={{ sm: 'wrap', md: 'nowrap', lg: 'wrap-reverse' }} | ||
| > | ||
| <div>item 1</div> | ||
| <div>item 2</div> | ||
| </Flex>, | ||
| ); | ||
|
|
||
| const flexContainer = screen.getByTestId('flex-container'); | ||
| expect(flexContainer).toHaveClass(styles.align.sm.stretch); | ||
| expect(flexContainer).toHaveClass(styles.align.md.center); | ||
| expect(flexContainer).toHaveClass(styles.align.lg['flex-start']); | ||
| expect(flexContainer).toHaveClass(styles.direction.sm.column); | ||
| expect(flexContainer).toHaveClass(styles.direction.md.row); | ||
| expect(flexContainer).toHaveClass(styles.direction.lg['row-reverse']); | ||
| expect(flexContainer).toHaveClass(styles.justify.sm.center); | ||
| expect(flexContainer).toHaveClass(styles.justify.md['space-between']); | ||
| expect(flexContainer).toHaveClass(styles.justify.lg['flex-end']); | ||
| expect(flexContainer).toHaveClass(styles.wrap.sm.wrap); | ||
| expect(flexContainer).toHaveClass(styles.wrap.md.nowrap); | ||
| expect(flexContainer).toHaveClass(styles.wrap.lg['wrap-reverse']); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for partial breakpoint definitions (skipping a breakpoint).
All new responsive tests always supply sm, md, and lg together. The trickiest part of getResponsiveClassNames/getResponsiveGapStyle is the cascade when a middle breakpoint is omitted (e.g. align={{ sm: 'center', lg: 'flex-end' }} with no md) — no test currently locks in that md correctly falls through to the inherited sm/CSS-cascade value instead of being forced to a default. A future refactor could silently break that fallback without failing any test.
As per path instructions, "packages/*/src/*.test.tsx: ... Testing component variants, props, and edge cases."
🤖 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 `@packages/flex/src/Flex.test.tsx` around lines 253 - 280, Add a responsive
breakpoint test near the existing “adds responsive classes for md and lg
breakpoint values” case that omits the middle md value, such as align={{ sm:
'center', lg: 'flex-end' }}. Assert the generated classes preserve sm and lg
while leaving md unspecified so CSS cascade inheritance remains responsible for
the intermediate value, covering the fallback behavior of
getResponsiveClassNames/getResponsiveGapStyle.
Source: Path instructions
| /** | ||
| * Relative specifiers in declaration files must carry the runtime extension, otherwise node16 ESM | ||
| * resolution fails to find the sibling modules. The semantic layer is types-only and needs no | ||
| * runtime module: TypeScript resolves `./semantic.js` to `semantic.d.ts` by extension substitution. | ||
| * @param {'.js' | '.cjs'} ext | ||
| */ | ||
| const barrelDts = (ext) => | ||
| [ | ||
| '/** Auto-generated — do not edit directly. */', | ||
| `export * from './primitive${ext}';`, | ||
| `export * from './semantic${ext}';`, | ||
| 'export type DesignToken = PrimitiveToken | SemanticToken;', | ||
| '/** Wraps a design token name in `var()` for use in inline styles. */', | ||
| 'export declare function cssVar<T extends DesignToken>(token: T): `var(--${T})`;\n', | ||
| ].join('\n'); | ||
| writeFileSync(`${DIST_TS}/index.d.ts`, barrelDts('.js')); | ||
| writeFileSync(`${DIST_TS}/index.d.cts`, barrelDts('.cjs')); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
pnpm exec tsc --noEmit --strict --module NodeNext --moduleResolution NodeNext packages/tokens/dist/ts/index.d.tsRepository: sipe-team/side
Length of output: 623
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching packages/tokens/config.js and dist ts =="
git ls-files | rg '(^|/)packages/tokens/config\.js$|packages/tokens/|dist/' | head -200
echo
echo "== config.js relevant lines =="
if [ -f packages/tokens/config.js ]; then
nl -ba packages/tokens/config.js | sed -n '260,330p'
fi
echo
echo "== package outlines around token types =="
for f in $(git ls-files | rg 'packages/tokens/src/.*\.ts$'); do
echo "-- $f --"
rg 'export (?:interface|type|class|const) (PrimitiveToken|SemanticToken|# DesignToken|DesignToken|Primitive|Semantic)' "$f" || true
doneRepository: sipe-team/side
Length of output: 1262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/tokens/config.js relevant section =="
python3 - <<'PY'
from pathlib import Path
p = Path('packages/tokens/config.js')
lines = p.read_text().splitlines()
for i in range(250, 335):
if 0 <= i < len(lines):
print(f'{i+1:5}: {lines[i]}')
PY
echo
echo "== token type declarations =="
python3 - <<'PY'
import re
from pathlib import Path
for p in sorted(Path('packages/tokens/src').rglob('*.ts')):
text = p.read_text()
candidates = ['PrimitiveToken', 'SemanticToken', 'DesignToken', 'Primitive', 'Semantic']
hits = [(i+1, line.strip()) for i, line in enumerate(text.splitlines()) if any(c in line for c in candidates) and not line.strip().startswith('//')]
if hits:
print(f'-- {p} --')
for lineno, line in hits[:10]:
print(f'{lineno:5}: {line}')
PY
echo
echo "== generated index d.ts behavior source check =="
python3 - <<'PY'
from pathlib import Path
config = Path('packages/tokens/config.js').read_text()
if 'barrelDts' not in config:
print('barrelDts not found')
else:
m = re.search(r'const barrelDts = \([^)]*\) =>[\s\S]*?\.join\(', config)
print(m.group(0)[:1200] if m else 'not matched')
PYRepository: sipe-team/side
Length of output: 3571
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all PrimitiveToken/SemanticToken/DesignToken usages =="
python3 - <<'PY'
from pathlib import Path
for p in sorted(Path('.').rglob('*')):
if not p.is_file():
continue
s = str(p)
if '.git' in s.split('/',1)[0]:
continue
try:
text = p.read_text()
except Exception:
continue
if any(tok in text for tok in ('PrimitiveToken', 'SemanticToken', 'DesignToken')):
print(f'-- {p} --')
for i, line in enumerate(text.splitlines():
if i < 1000 and any(tok in line for tok in ('PrimitiveToken', 'SemanticToken', 'DesignToken')):
print(f'{i+1}: {line.strip()}')
PY
echo
echo "== package package.json =="
if [ -f packages/tokens/package.json ]; then
python3 - <<'PY'
import json
from pathlib import Path
print(json.dumps(json.loads(Path('packages/tokens/package.json').read_text()), indent=2))
PY
fi
echo
echo "== TypeScript / tsup availability =="
command -v tsc || true
command -v tsup || true
if [ -f package.json ]; then
python3 - <<'PY'
import json
p=json.load(open('package.json'))
print("scripts:", {k:p.get("scripts",{}).get(k) for k in ("build","dev","test") if k in p.get("scripts",{})})
print("dev deps:", {k:v for k,v in p.get("devDependencies",{}).items() if k in ("typescript","tsup","`@types/node`")})
PY
fiRepository: sipe-team/side
Length of output: 360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all PrimitiveToken/SemanticToken/DesignToken usages =="
rg -n --glob '!*.lock' --glob '!dist/**' --glob '!node_modules/**' 'PrimitiveToken|SemanticToken|DesignToken' . || true
echo
echo "== package/package files relevant to token package =="
for f in package.json packages/tokens/package.json packages/tokens/tsconfig.json packages/tokens/tsup.config.ts; do
if [ -f "$f" ]; then
echo "-- $f --"
cat -n "$f"
fi
done
echo
echo "== TypeScript/tsup availability =="
command -v tsc || true
command -v tsup || true
echo
echo "== read-only TypeScript declaration shape probe =="
tmpdir="$(mktemp -d)"
cd "$tmpdir"
cat > primitive.d.ts <<'EOF'
export interface PrimitiveToken { kind: 'primitive'; value: string }
export declare const value: PrimitiveToken
EOF
cat > semantic.d.ts <<'EOF'
export interface SemanticToken { kind: 'semantic'; value: string }
export declare const value: SemanticToken
EOF
cat > index.d.ts <<'EOF'
/** Auto-generated — do not edit directly. */
export * from './primitive';
export * from './semantic';
export type DesignToken = PrimitiveToken | SemanticToken;
export declare function cssVar<T extends DesignToken>(token: T): `var(--${T})`;
EOF
if command -v tsc >/dev/null 2>&1; then
tsc --noEmit --strict --module NodeNext --moduleResolution NodeNext index.d.ts || true
else
echo "tsc unavailable; skipped"
fiRepository: sipe-team/side
Length of output: 6672
Bind the re-exported token types before using them.
export * from ... does not introduce local PrimitiveToken and SemanticToken bindings, so the generated DesignToken declaration can fail with Cannot find name .... Add import type declarations for the used tokens before defining DesignToken.
🤖 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 `@packages/tokens/config.js` around lines 298 - 314, Update the generated
declarations in barrelDts to add local import type bindings for PrimitiveToken
and SemanticToken before the DesignToken alias, while preserving the existing
extension-specific re-exports and cssVar declaration.
| ### Text fallback | ||
|
|
||
| With no `src`, the avatar shows text instead of an image — `alt` if present, otherwise `fallback`. | ||
| Use it for initials when a user has no picture. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline packages/avatar/src/Avatar.tsx --items all
rg -n -C 8 'fallback|onError|<img' packages/avatar/src/Avatar.tsxRepository: sipe-team/side
Length of output: 2354
🏁 Script executed:
#!/bin/bash
sed -n '1,150p' www/docs/components/avatar.mdxRepository: sipe-team/side
Length of output: 4819
Align fallback’s documented behavior.
The JSX fallback example and no-src text behavior are accurate, but the API table says fallback is a replacement image URL if src fails. That is wrong: current behavior replaces src with fallback on error, and the table should describe it as either that replacement URL or remove the misleading “replacement image URL” wording.
🤖 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 `@www/docs/components/avatar.mdx` around lines 74 - 77, Update the API table
entry for fallback in the avatar documentation to describe its actual behavior:
it supplies the replacement URL when src fails, rather than being a general
replacement image URL. Keep the existing JSX fallback example and no-src text
behavior unchanged.
| export function Preview({ children, code, language = 'tsx', theme = 'light' }: PreviewProps) { | ||
| const [showCode, setShowCode] = useState(false); | ||
|
|
||
| return ( | ||
| <div className={styles.preview}> | ||
| <div className={clsx(styles.stage, theme === 'dark' && styles.stageDark)}>{children}</div> | ||
|
|
||
| <div className={styles.toolbar}> | ||
| <button type="button" className={styles.toggle} onClick={() => setShowCode((prev) => !prev)}> | ||
| {showCode ? 'Hide code' : 'Show code'} | ||
| </button> | ||
| </div> | ||
|
|
||
| {showCode && ( | ||
| <div className={styles.code}> | ||
| <CodeBlock language={language}>{code}</CodeBlock> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add coverage for the code-toggle behavior.
This new stateful component has no accompanying test. Cover the initial hidden state and both toggle transitions with Vitest and React Testing Library in happy-dom.
As per coding guidelines, “Add or update Vitest coverage for all behavior changes” and “Write component tests with Vitest and @testing-library/react in happy-dom environment.”
🤖 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 `@www/src/components/Preview/index.tsx` around lines 17 - 37, Add Vitest and
React Testing Library coverage for the Preview component’s showCode behavior:
assert the code is initially hidden, then verify clicking the toggle reveals the
code and updates the label, and clicking again hides the code and restores the
label. Configure the test to run in the happy-dom environment and target the
Preview component and its toggle button.
Source: Coding guidelines
| } | ||
|
|
||
| /* The code block already carries the page's outer spacing; strip its stray bottom margin. */ | ||
| .code :global(.theme-code-block) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Resolve the Stylelint error for the CSS Modules selector.
Line 50 triggers selector-pseudo-class-no-unknown. Configure Stylelint for CSS Modules syntax, or locally suppress this known-valid :global() selector.
Proposed local suppression
+/* stylelint-disable-next-line selector-pseudo-class-no-unknown */
.code :global(.theme-code-block) {📝 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.
| .code :global(.theme-code-block) { | |
| /* stylelint-disable-next-line selector-pseudo-class-no-unknown */ | |
| .code :global(.theme-code-block) { |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 50-50: Unknown pseudo-class selector ":global" (selector-pseudo-class-no-unknown)
(selector-pseudo-class-no-unknown)
🤖 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 `@www/src/components/Preview/styles.module.css` at line 50, Update the
Stylelint configuration for CSS Modules so the :global() selector in .code
:global(.theme-code-block) is recognized as valid, or add a narrowly scoped
suppression for this selector; preserve the existing styling behavior and avoid
disabling unrelated selector validation.
Source: Linters/SAST tools
작업 배경
dev브랜치에 누적된 작업을main브랜치와 동기화하기 위한 PR입니다.dev -> main병합 과정에서 발생한 충돌을 해결했습니다.작업 내용
main을 기준으로 최신dev변경사항 동기화확인
env CI=true mise exec -- pnpm install --frozen-lockfilemise exec -- pnpm lintmise exec -- pnpm test -- runmise exec -- pnpm typecheckmise exec -- pnpm buildmise exec -- pnpm lint:publishgit diff --check