Storybook on Vite + Storybook 10; ESM-only package; flat-config lint - #431
Storybook on Vite + Storybook 10; ESM-only package; flat-config lint#431rushi wants to merge 31 commits into
Conversation
Storybook now uses the Vite builder (@storybook/react-vite), matching the build tool the library already ships with. Stays on Storybook 8.6. Changes: - .storybook/main.js: framework -> @storybook/react-vite; drop webpack-only addons (addon-webpack5-compiler-babel, addon-styling-webpack) and their hand-rolled CSS/PostCSS loader block. Add viteFinal: camelCase CSS-module convention plus shims for tailwind.config.js (CommonJS default import, __dirname, path) consumed by the Configuration stories. - package.json: remove react-webpack5, the two webpack addons, webpack, css-loader, style-loader, postcss-loader. Add @storybook/react-vite and @vitejs/plugin-react. Regenerate lockfile. - Rename 54 story files + preview.js .js -> .jsx (esbuild rejects JSX in .js). - Installation.mdx: import README via ?raw. - ImageUpload story: remove duplicate csvAcceptFormats argType key. - Add .storybook/node-path-shim.js (browser path shim, Storybook build only). Verified: build:storybook succeeds; agent-browser console sweep identical before/after (156 pass, 2 mdx docs allowlisted); story IDs identical (158); CSS modules still hash+emit; test and lint green. Fixtures in docs/fixtures/. Dev-tooling change only. No public API or component behavior change.
Upgrade to Storybook 9 (via storybook@9 upgrade codemods), keeping the Vite
builder from the previous commit. SB9 consolidates most addons into the core
storybook package, so this is largely dependency removal plus import-path moves.
Changes:
- .storybook/main.js: addons reduced to addon-links + addon-docs (essentials
and actions merged into core). backgrounds disabled via top-level
features: { backgrounds: false } (v9 mechanism). viteFinal block preserved.
- .storybook/manager.js: @storybook/manager-api -> storybook/manager-api.
- .storybook/xola.js: @storybook/theming -> storybook/theming.
- Installation.mdx, Introduction.mdx: import Meta/Markdown from
@storybook/addon-docs/blocks (Meta no longer exported from the root entry
in v9; this broke the build until fixed).
- package.json: all @storybook/* on ^9.1.20; remove packages consolidated into
core (addon-actions, addon-essentials, manager-api, react, test, theming);
add @storybook/addon-docs. Remove orphaned babel deps (babel-loader,
@babel/core, preset-env, preset-react, preset-typescript) and babel.config.js.
Regenerate lockfile.
Verified: build:storybook succeeds; agent-browser console sweep 158/158 pass
(mdx docs pages now render under v9); story IDs identical (158); CSS modules
still hash+emit identically; test, lint, dev all green.
Known follow-up: main.js stays CommonJS (viteFinal shims depend on __dirname);
Vite warns this is deprecated, becomes a hard requirement only in SB10.
Dev-tooling upgrade only. No public API or component behavior change.
The Configuration stories import tailwind.config.js as a default ESM import to display theme tokens. The previous shims (build.commonjsOptions, __dirname define, path alias + node-path-shim.js) only fixed the production build: Vite serves root CommonJS files raw in dev, so `import cfg from ".../tailwind.config"` resolved to a module with no default export and every Configuration story threw "does not provide an export named 'default'" under `npm run dev`. Replace all four shims with a single Vite plugin that evaluates the config in Node (where require/__dirname/path work natively) and returns a plain ESM data object. Works identically in dev and build; functions (the plugins array) are dropped since the stories only read `.theme`. tailwind.config.js stays CommonJS (it is published and required by apps and repo scripts). Verified: all 8 Configuration stories render in dev and in the static build with no console errors; build:storybook and lint green.
UI Kit now ships as ESM only, to be consumed from ESM projects. Add
"type": "module" and convert every remaining CommonJS file to ESM.
BREAKING CHANGE: the published tailwind.config.js and postcss.config.js are now
ESM (export default), and the package is "type": "module". Consumers that
`require("@xola/ui-kit/tailwind.config")` must switch to an ESM re-export (see
README). Warrants a major version bump on release.
- tailwind.config.js, postcss.config.js: require/module.exports -> import/export
default. __dirname derived via import.meta.url; deep import uses the explicit
tailwindcss/defaultTheme.js path.
- scripts/prepare.js, createThemeFile.js, createThemeCss.js: converted to ESM.
- .storybook/main.js: converted to ESM. It no longer imports tailwind.config.js
(which pulls Node-only deps into the Storybook graph); instead it serves the
generated browser-safe src/theme.js in place of the config for the
Configuration stories. Also clears the Vite "CJS Node API deprecated" warning.
- CLAUDE.md, README.md: document ESM-only, the module-format rule, and the ESM
config re-export for consumers.
Verified: build, build:storybook, and the dev server all succeed; Configuration
stories and high-risk components render with no console errors; lint green;
no CommonJS/Vite deprecation warnings remain.
Bump all @storybook/* and storybook to 10.5.2 (react-vite builder), and migrate linting from the removed xola-lint CLI to @xola/jslint 3.2 flat config (eslint 9, typescript 5). Move react/react-dom to peerDependencies.
Storybook 9.1.20 -> 10.5.2 (via storybook upgrade), keeping the Vite builder. SB10 requires an ESM main config, already in place. Removed the addon-mcp that the automigration injected (unwanted). React 18+ only: move react/react-dom to peerDependencies (>=18), kept in devDependencies for local dev. jslint 3.2.x applies its React rules unconditionally, so this no longer breaks lint. Lint migrated from @xola/jslint 3.0.1 (xola-lint CLI / xo) to 3.2.1 (flat ESLint 9 config). Adds eslint ^9.18, bumps typescript to ^5. New eslint.config.mjs re-exports @xola/jslint and carries the repo's ignores (src/stories, generated theme, build output, tests). Removed .xo-config.json and .eslintignore. lint scripts now call eslint directly. Source fixes surfaced by the new (unconditional) React rules: - Add displayName to forwardRef / sub-components (react/display-name). - HeaderToolbar: fix a copy-paste bug that set Breadcrumb.displayName twice instead of setting Search.displayName. - Remove stale eslint-disable directives that no longer match any rule. Verified: build, build:storybook, and dev all succeed; agent-browser sweep 158/158 identical to the v9 baseline; story IDs identical; lint 0 errors (3 pre-existing complexity warnings); test green.
- CLAUDE.md: React 17 -> 18, Storybook 9 -> 10, Jest -> Vitest (commands + testing section), fix single-test example path (src/utils/avatar.test.js), note eslint flat config. - README.md: add react/react-dom to the peer-install list (now peerDependencies). - .claude/rules/testing.md: Jest -> Vitest, drop Chromatic, include src/utils/. - Remove all Chromatic references and em-dashes across the docs and rules per house style.
rushi
changed the title
Update checkout/setup-node to v7, get-user-teams-membership to v4, and eslint-annotate-action to the node24 v4 build to clear the GitHub Actions Node 20 deprecation warnings. Rename the annotate token input to GITHUB_TOKEN.
Removes xola-lint CLI reference and adds ESLint 9 flat config to align with the root package migration. Updates scripts to run ESLint directly and adds required devDependencies.
Adds @storybook/addon-vitest with Vitest 3, @vitest/browser, coverage-v8, and Playwright to enable story-based interaction tests alongside existing unit tests. Configures separate node and browser test projects with npm scripts for unit, storybook, and combined runs.
Without keys, React remounts on jsxDecorator double-render and throws "Rendered more hooks than during previous render" in smoke tests. Affects Currency, Flash, and ToggleButton stories.
Badge story asserts its text renders correctly. Button gets new Clickable story that clicks the button and verifies the onClick spy was invoked.
Runs both unit and Storybook interaction tests on push and pull requests, with Playwright chromium pre-installed for browser-based story execution.
Increases visual test coverage for simple components with assertions for render output, interaction behavior, and disabled states across multiple story variants per component.
Verify interactive behaviors and render assertions across form inputs, buttons, tabs, and toggles with multiple story variants. Catches user interaction issues that static analysis misses.
Add Storybook play-function tests for Select, ComboBox, Modal, Popover, and Tooltip to validate interactions, portal content, and async behavior. Fix ComboBox story args set to the string "boolean".
Cover the remaining non-date-picker components: overlays that portal to the body (Drawer, BottomSheet, PopoverList, InlineValuePopover), RangeSlider handles, plus Sidebar, HeaderToolbar, Breakdown, Login.
Restrict workflow to master branch instead of running on every push. Reindent YAML to 2 spaces for consistency.
Validate core interactions and edge cases through Storybook play functions, including day selection, disabled states, navigation, custom content, and range views. Improves confidence in the library's most critical component.
Add a copy-paste prompt for AI agents to guide automated installation and configuration. Update install to @next dist-tag. Clarify post-migration lint and testing workflows.
Browser-dependent initialization that ran at module load prevented imports in Node.js environments. Moved to lazy resolution with fallbacks, enabling SSR-safe usage without affecting client behavior.
Upgrade get-user-locale from v1 (CommonJS) to v3 (ESM). Bundle react-day-picker and google-libphonenumber in the Vite library build: these CommonJS-only deps expose named exports Node's ESM loader cannot resolve when left external. Other deps stay external to keep it lean.
ESLint generates .eslintcache locally when running lint commands. Add to .gitignore to prevent this cache from being committed.
Remove dependencies no longer referenced in the codebase. Add knip as a devDependency for continuous dead code detection.
Delete a barrel re-export file with no importers. Remove unnecessary export keywords from utilities only used internally.
Components were importing from lodash, which is not a declared dependency. Corrected to import from lodash-es for consistency.
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 38 seconds. |
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (118)
📝 WalkthroughWalkthroughThe project migrates to ESM-based tooling, modernizes Storybook and GitHub Actions, introduces Vitest unit and Storybook testing, adds SSR-safe utility behavior, expands configuration stories, and attaches interaction tests across existing Storybook stories. ChangesPlatform and testing migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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/storybook-test.yml:
- Around line 3-6: Change the workflow trigger from pull_request_target to
pull_request in the on configuration, since the job checks out and executes
pull-request code. Add minimal explicit permissions, such as contents: read, and
keep privileged reporting outside this test workflow.
- Around line 18-25: Pin every listed GitHub Action to its full immutable commit
SHA instead of mutable version tags. Update actions/checkout and
actions/setup-node in .github/workflows/storybook-test.yml (18-25),
.github/workflows/deploy-icons.yml (16-18), and .github/workflows/deploy.yml
(16-18); update tspascoal/get-user-teams-membership, actions/checkout,
actions/setup-node, and ataylorme/eslint-annotate-action in
.github/workflows/eslint.yml (29-29, 42-49, 64-68); and update actions/checkout,
actions/setup-node, and tsunematsu21/actions-publish-gh-pages in
.github/workflows/publish.yml (12-15, 28-28).
- Around line 18-22: Add persist-credentials: false to each actions/checkout
step in .github/workflows/storybook-test.yml (lines 18-22),
.github/workflows/deploy-icons.yml (line 16), .github/workflows/deploy.yml (line
16), .github/workflows/eslint.yml (lines 42-46), and
.github/workflows/publish.yml (lines 12-13).
In `@CLAUDE.md`:
- Around line 32-33: Update the architecture description in CLAUDE.md to reflect
that the component library uses both JavaScript/JSX and TypeScript/TSX,
including the existing DatePicker/LocalizedDayPicker.tsx boundary. Remove the
inaccurate “no TypeScript” claim while preserving the Vite, React, Tailwind, and
hand-maintained index.d.ts details.
In `@README.md`:
- Around line 77-82: Update the Tailwind v3 instructions in README.md to
reference tailwind.config.mjs instead of tailwind.config.js, keeping the
documented configuration filename consistent with the preceding creation
command.
- Line 228: Update the README command description for npm run lint to state that
it only lints src, removing the claim that it auto-fixes issues. Keep the
documented npm run lint:fix command as the fixing workflow.
- Around line 55-61: Update both manual npm install commands in the README,
including the `@xola/ui-kit` installation and peer-dependency installation, to use
--legacy-peer-deps consistently with the AI-agent instructions.
- Around line 41-42: Update both Tailwind v4 `@source` examples in README.md to
use the stylesheet-relative ../node_modules/@xola/ui-kit path instead of
./node_modules, including the later duplicate example; leave the `@import` path
unchanged.
In `@src/stories/DataDisplay/DateRangePicker.stories.jsx`:
- Line 52: Remove the narration-only comments at
src/stories/DataDisplay/DateRangePicker.stories.jsx lines 52-52,
src/stories/DataDisplay/DotProgress.stories.jsx lines 43-43, and
src/stories/Overlay/PopoverList.stories.jsx lines 103-103; leave the surrounding
story implementations unchanged.
In `@src/stories/Forms/ButtonGroup.stories.jsx`:
- Around line 48-54: Replace the Tailwind class assertions in Default.play of
src/stories/Forms/ButtonGroup.stories.jsx (lines 48-54) with assertions against
the button’s semantic selected state, exposing aria-pressed in the ButtonGroup
implementation if needed. Remove the .bg-danger assertion in
src/stories/Forms/Input.stories.jsx (lines 101-105); leave required-dot
appearance as visual Storybook coverage.
In `@src/stories/Forms/Select.stories.jsx`:
- Around line 24-29: Update the Default story’s userEvent.selectOptions call to
pass the option’s value rather than its display label, while preserving the
existing assertions that verify the initial and selected values.
In `@src/stories/Navigation/Tabs.stories.jsx`:
- Line 67: Remove the comments immediately preceding the assertions at the
referenced locations in the story, including the “Only the active panel is
rendered” comment, while leaving the assertions and their behavior unchanged.
In `@src/utils/ssr-safe.test.js`:
- Around line 1-32: Split the SSR regression cases from the cross-utility suite
into the source-matched numbers.test.js, currency.test.js, phone.test.js, and
date.test.js suites beside their respective utilities. Move each dynamic import
and contract assertion into its corresponding file, retain the no-navigator SSR
coverage there, and remove the cross-utility ssr-safe.test.js coverage.
In `@vite.config.js`:
- Around line 5-13: Update the externalization comment near bundleForEsmInterop
to state that react-day-picker and google-libphonenumber are bundled for ESM
interop, while only the remaining dependencies from pkg.dependencies are
externalized. Keep the existing dependency filter unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dbcd8bac-e420-4b74-a4da-65d9c1cd2c25
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (118)
.claude/rules/commenting.md.claude/rules/testing.md.eslintignore.github/workflows/chromatic.yml.github/workflows/deploy-icons.yml.github/workflows/deploy.yml.github/workflows/eslint.yml.github/workflows/publish.yml.github/workflows/storybook-test.yml.gitignore.storybook/main.js.storybook/manager.js.storybook/preview.jsx.storybook/xola.js.xo-config.jsonCLAUDE.mdREADME.mdbabel.config.jseslint.config.mjsknip.jsonpackage.jsonpostcss.config.jsscripts/createThemeCss.jsscripts/createThemeFile.jsscripts/prepare.jssrc/components/BottomSheet.jsxsrc/components/Buttons/SecondaryButton.jsxsrc/components/Buttons/SubmitButton.jsxsrc/components/DatePicker/DatePickerPopover.jsxsrc/components/DatePicker/LocalizedDayPicker.tsxsrc/components/DatePicker/NavbarElement.jsxsrc/components/DatePicker/RelativeDateRange.jsxsrc/components/Drawer.jsxsrc/components/Forms/BaseInput.jsxsrc/components/Forms/ComboBox.jsxsrc/components/Forms/Input.jsxsrc/components/Forms/Textarea.jsxsrc/components/GooglePlacesAutocomplete.jsxsrc/components/HeaderToolbar.jsxsrc/components/Popover/Popover.jsxsrc/components/Search.jsxsrc/components/Sidebar/Sidebar.Account.jsxsrc/components/Sidebar/Sidebar.Link.jsxsrc/components/Sidebar/Sidebar.Menu.jsxsrc/components/Sidebar/Sidebar.jsxsrc/components/Table.jsxsrc/components/Utilities/Currency.jsxsrc/components/Utilities/Number.jsxsrc/helpers/browser.jssrc/hooks/useViewportHeight.jssrc/icons/eslint.config.mjssrc/icons/package.jsonsrc/stories/Configuration/BorderRadius.stories.jsxsrc/stories/Configuration/Colors.stories.jsxsrc/stories/Configuration/Fonts.stories.jsxsrc/stories/Configuration/Links.stories.jsxsrc/stories/Configuration/Paragraphs.stories.jsxsrc/stories/Configuration/Spacing.stories.jsxsrc/stories/Configuration/Text.stories.jsxsrc/stories/DataDisplay/Alert.stories.jsxsrc/stories/DataDisplay/Badge.stories.jsxsrc/stories/DataDisplay/Counter.stories.jsxsrc/stories/DataDisplay/Currency.stories.jsxsrc/stories/DataDisplay/DatePicker.stories.jsxsrc/stories/DataDisplay/DateRangePicker.stories.jsxsrc/stories/DataDisplay/Dot.stories.jsxsrc/stories/DataDisplay/DotProgress.stories.jsxsrc/stories/DataDisplay/Flash.stories.jsxsrc/stories/DataDisplay/Key.stories.jsxsrc/stories/DataDisplay/Number.stories.jsxsrc/stories/DataDisplay/Phone.stories.jsxsrc/stories/DataDisplay/Skeleton.stories.jsxsrc/stories/DataDisplay/Table.stories.jsxsrc/stories/DataDisplay/Tag.stories.jsxsrc/stories/Forms/Button.stories.jsxsrc/stories/Forms/ButtonGroup.stories.jsxsrc/stories/Forms/Checkbox.stories.jsxsrc/stories/Forms/ComboBox.stories.jsxsrc/stories/Forms/InlineValuePopover.stories.jsxsrc/stories/Forms/Input.stories.jsxsrc/stories/Forms/RangeSlider.stories.jsxsrc/stories/Forms/SecondaryButton.stories.jsxsrc/stories/Forms/Select.stories.jsxsrc/stories/Forms/SubmitButton.stories.jsxsrc/stories/Forms/Switch.stories.jsxsrc/stories/Forms/Textarea.stories.jsxsrc/stories/Forms/ToggleButton.stories.jsxsrc/stories/Installation.mdxsrc/stories/Introduction.mdxsrc/stories/Media/Avatar.stories.jsxsrc/stories/Media/Icons.stories.jsxsrc/stories/Media/ImageUpload.stories.jsxsrc/stories/Media/Images.stories.jsxsrc/stories/Media/Logo.stories.jsxsrc/stories/Navigation/Breadcrumb.stories.jsxsrc/stories/Navigation/Sidebar.stories.jsxsrc/stories/Navigation/Tabs.stories.jsxsrc/stories/Other/Breakdown.stories.jsxsrc/stories/Other/HeaderToolbar.stories.jsxsrc/stories/Other/Search.stories.jsxsrc/stories/Other/Spinner.stories.jsxsrc/stories/Overlay/BottomSheet.stories.jsxsrc/stories/Overlay/Drawer.stories.jsxsrc/stories/Overlay/Modal.stories.jsxsrc/stories/Overlay/Popover.stories.jsxsrc/stories/Overlay/PopoverList.stories.jsxsrc/stories/Overlay/Tooltip.stories.jsxsrc/stories/Screens/Login.stories.jsxsrc/utils/currency.jssrc/utils/date.jssrc/utils/index.jssrc/utils/numbers.jssrc/utils/phone.jssrc/utils/ssr-safe.test.jssrc/utils/userLocale.jstailwind.config.jsvite.config.jsvitest.config.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
xola/x2-seller(manual)xola/x2-checkout(manual)xola/jslint(auto-detected)
💤 Files with no reviewable changes (11)
- src/components/Sidebar/Sidebar.Menu.jsx
- src/components/Sidebar/Sidebar.jsx
- .xo-config.json
- babel.config.js
- .github/workflows/chromatic.yml
- src/components/Search.jsx
- src/components/Sidebar/Sidebar.Link.jsx
- .eslintignore
- src/components/Table.jsx
- src/components/Sidebar/Sidebar.Account.jsx
- src/utils/index.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (.claude/rules/commenting.md)
**/*: Write self-explanatory code and add comments only when necessary to explain why, not what; prefer clearer names or refactoring when they eliminate the need for a comment.
Use comments for non-obvious constraints, invariants, performance trade-offs, complex Tailwind class rationale, custom prop-types validation, and workarounds for third-party library quirks.
Do not add obvious, redundant, dead-code, changelog/ticket, caller-history, or divider comments.
Use the prefixesTODO:,FIXME:,HACK:,NOTE:, andPERF:for work-in-progress and maintenance annotations.
Files:
scripts/prepare.jssrc/stories/Installation.mdxknip.jsonsrc/icons/package.jsonsrc/stories/Introduction.mdxsrc/stories/Forms/SecondaryButton.stories.jsxsrc/components/Utilities/Number.jsxsrc/components/Drawer.jsxvitest.config.jssrc/components/Buttons/SecondaryButton.jsxsrc/components/BottomSheet.jsxsrc/utils/ssr-safe.test.jseslint.config.mjssrc/components/DatePicker/DatePickerPopover.jsxsrc/components/GooglePlacesAutocomplete.jsxsrc/components/Forms/Input.jsxsrc/stories/Configuration/Links.stories.jsxsrc/utils/userLocale.jssrc/stories/Forms/Checkbox.stories.jsxsrc/components/DatePicker/NavbarElement.jsxsrc/components/Popover/Popover.jsxsrc/stories/Other/Breakdown.stories.jsxsrc/components/Forms/Textarea.jsxsrc/icons/eslint.config.mjspostcss.config.jssrc/stories/Media/Icons.stories.jsxsrc/stories/DataDisplay/Number.stories.jsxsrc/stories/DataDisplay/Badge.stories.jsxsrc/stories/Configuration/Text.stories.jsxsrc/stories/Other/HeaderToolbar.stories.jsxsrc/stories/Configuration/BorderRadius.stories.jsxsrc/stories/DataDisplay/DotProgress.stories.jsxsrc/stories/Configuration/Spacing.stories.jsxsrc/stories/Forms/SubmitButton.stories.jsxsrc/stories/Overlay/Popover.stories.jsxsrc/stories/Forms/Button.stories.jsxsrc/stories/Forms/RangeSlider.stories.jsxsrc/stories/DataDisplay/Counter.stories.jsxsrc/stories/Configuration/Paragraphs.stories.jsxsrc/stories/Forms/ToggleButton.stories.jsxsrc/stories/Forms/InlineValuePopover.stories.jsxscripts/createThemeCss.jssrc/stories/DataDisplay/Alert.stories.jsxsrc/stories/Navigation/Breadcrumb.stories.jsxscripts/createThemeFile.jssrc/stories/DataDisplay/Skeleton.stories.jsxsrc/utils/currency.jssrc/stories/DataDisplay/Table.stories.jsxsrc/utils/phone.jssrc/stories/DataDisplay/Dot.stories.jsxsrc/stories/Forms/Textarea.stories.jsxvite.config.jssrc/stories/Media/Logo.stories.jsxsrc/components/Forms/BaseInput.jsxsrc/stories/Forms/Select.stories.jsxsrc/stories/Navigation/Sidebar.stories.jsxsrc/stories/Navigation/Tabs.stories.jsxsrc/stories/Other/Spinner.stories.jsxsrc/stories/Other/Search.stories.jsxsrc/components/Buttons/SubmitButton.jsxsrc/stories/Overlay/Modal.stories.jsxsrc/components/DatePicker/LocalizedDayPicker.tsxsrc/stories/DataDisplay/Currency.stories.jsxsrc/stories/DataDisplay/Flash.stories.jsxsrc/stories/Forms/ButtonGroup.stories.jsxsrc/stories/Overlay/PopoverList.stories.jsxsrc/stories/Media/Images.stories.jsxsrc/stories/Forms/Input.stories.jsxsrc/stories/Configuration/Fonts.stories.jsxsrc/stories/Overlay/Tooltip.stories.jsxsrc/stories/Overlay/BottomSheet.stories.jsxsrc/stories/Media/Avatar.stories.jsxsrc/components/Forms/ComboBox.jsxsrc/stories/DataDisplay/Tag.stories.jsxsrc/stories/Overlay/Drawer.stories.jsxsrc/components/HeaderToolbar.jsxsrc/stories/DataDisplay/DateRangePicker.stories.jsxsrc/stories/Configuration/Colors.stories.jsxsrc/stories/DataDisplay/Phone.stories.jsxsrc/stories/DataDisplay/Key.stories.jsxsrc/stories/Forms/Switch.stories.jsxsrc/components/DatePicker/RelativeDateRange.jsxsrc/utils/date.jssrc/components/Utilities/Currency.jsxsrc/hooks/useViewportHeight.jssrc/stories/Forms/ComboBox.stories.jsxsrc/stories/Screens/Login.stories.jsxsrc/utils/numbers.jssrc/stories/Media/ImageUpload.stories.jsxtailwind.config.jssrc/helpers/browser.jsCLAUDE.mdREADME.mdpackage.jsonsrc/stories/DataDisplay/DatePicker.stories.jsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.claude/rules/commenting.md)
Use JSDoc for shared helpers and utilities used across many components, documenting their purpose, parameters, and return values.
Files:
scripts/prepare.jssrc/stories/Forms/SecondaryButton.stories.jsxsrc/components/Utilities/Number.jsxsrc/components/Drawer.jsxvitest.config.jssrc/components/Buttons/SecondaryButton.jsxsrc/components/BottomSheet.jsxsrc/utils/ssr-safe.test.jssrc/components/DatePicker/DatePickerPopover.jsxsrc/components/GooglePlacesAutocomplete.jsxsrc/components/Forms/Input.jsxsrc/stories/Configuration/Links.stories.jsxsrc/utils/userLocale.jssrc/stories/Forms/Checkbox.stories.jsxsrc/components/DatePicker/NavbarElement.jsxsrc/components/Popover/Popover.jsxsrc/stories/Other/Breakdown.stories.jsxsrc/components/Forms/Textarea.jsxpostcss.config.jssrc/stories/Media/Icons.stories.jsxsrc/stories/DataDisplay/Number.stories.jsxsrc/stories/DataDisplay/Badge.stories.jsxsrc/stories/Configuration/Text.stories.jsxsrc/stories/Other/HeaderToolbar.stories.jsxsrc/stories/Configuration/BorderRadius.stories.jsxsrc/stories/DataDisplay/DotProgress.stories.jsxsrc/stories/Configuration/Spacing.stories.jsxsrc/stories/Forms/SubmitButton.stories.jsxsrc/stories/Overlay/Popover.stories.jsxsrc/stories/Forms/Button.stories.jsxsrc/stories/Forms/RangeSlider.stories.jsxsrc/stories/DataDisplay/Counter.stories.jsxsrc/stories/Configuration/Paragraphs.stories.jsxsrc/stories/Forms/ToggleButton.stories.jsxsrc/stories/Forms/InlineValuePopover.stories.jsxscripts/createThemeCss.jssrc/stories/DataDisplay/Alert.stories.jsxsrc/stories/Navigation/Breadcrumb.stories.jsxscripts/createThemeFile.jssrc/stories/DataDisplay/Skeleton.stories.jsxsrc/utils/currency.jssrc/stories/DataDisplay/Table.stories.jsxsrc/utils/phone.jssrc/stories/DataDisplay/Dot.stories.jsxsrc/stories/Forms/Textarea.stories.jsxvite.config.jssrc/stories/Media/Logo.stories.jsxsrc/components/Forms/BaseInput.jsxsrc/stories/Forms/Select.stories.jsxsrc/stories/Navigation/Sidebar.stories.jsxsrc/stories/Navigation/Tabs.stories.jsxsrc/stories/Other/Spinner.stories.jsxsrc/stories/Other/Search.stories.jsxsrc/components/Buttons/SubmitButton.jsxsrc/stories/Overlay/Modal.stories.jsxsrc/components/DatePicker/LocalizedDayPicker.tsxsrc/stories/DataDisplay/Currency.stories.jsxsrc/stories/DataDisplay/Flash.stories.jsxsrc/stories/Forms/ButtonGroup.stories.jsxsrc/stories/Overlay/PopoverList.stories.jsxsrc/stories/Media/Images.stories.jsxsrc/stories/Forms/Input.stories.jsxsrc/stories/Configuration/Fonts.stories.jsxsrc/stories/Overlay/Tooltip.stories.jsxsrc/stories/Overlay/BottomSheet.stories.jsxsrc/stories/Media/Avatar.stories.jsxsrc/components/Forms/ComboBox.jsxsrc/stories/DataDisplay/Tag.stories.jsxsrc/stories/Overlay/Drawer.stories.jsxsrc/components/HeaderToolbar.jsxsrc/stories/DataDisplay/DateRangePicker.stories.jsxsrc/stories/Configuration/Colors.stories.jsxsrc/stories/DataDisplay/Phone.stories.jsxsrc/stories/DataDisplay/Key.stories.jsxsrc/stories/Forms/Switch.stories.jsxsrc/components/DatePicker/RelativeDateRange.jsxsrc/utils/date.jssrc/components/Utilities/Currency.jsxsrc/hooks/useViewportHeight.jssrc/stories/Forms/ComboBox.stories.jsxsrc/stories/Screens/Login.stories.jsxsrc/utils/numbers.jssrc/stories/Media/ImageUpload.stories.jsxtailwind.config.jssrc/helpers/browser.jssrc/stories/DataDisplay/DatePicker.stories.jsx
**/*.{js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx}: Use native ESM exclusively: author JavaScript and JSX files withimport/export; never userequireormodule.exports. Deep Node imports must include file extensions, and__dirnamemust be derived fromimport.meta.url.
Use named colors fromtailwind.config.jssuch asbg-primary,text-gray-dark, andborder-gray-light; never use Tailwind's default numbered palette such astext-gray-600.
Always clean upuseEffectside effects, including event listeners, timers, tippy/nouislider instances, and subscriptions, when components unmount.
Do not suppressreact-hooks/exhaustive-deps.
Write self-explanatory code and add comments only to explain why, not what; follow.claude/rules/commenting.mdfor detailed guidance.
Files:
scripts/prepare.jssrc/stories/Forms/SecondaryButton.stories.jsxsrc/components/Utilities/Number.jsxsrc/components/Drawer.jsxvitest.config.jssrc/components/Buttons/SecondaryButton.jsxsrc/components/BottomSheet.jsxsrc/utils/ssr-safe.test.jssrc/components/DatePicker/DatePickerPopover.jsxsrc/components/GooglePlacesAutocomplete.jsxsrc/components/Forms/Input.jsxsrc/stories/Configuration/Links.stories.jsxsrc/utils/userLocale.jssrc/stories/Forms/Checkbox.stories.jsxsrc/components/DatePicker/NavbarElement.jsxsrc/components/Popover/Popover.jsxsrc/stories/Other/Breakdown.stories.jsxsrc/components/Forms/Textarea.jsxpostcss.config.jssrc/stories/Media/Icons.stories.jsxsrc/stories/DataDisplay/Number.stories.jsxsrc/stories/DataDisplay/Badge.stories.jsxsrc/stories/Configuration/Text.stories.jsxsrc/stories/Other/HeaderToolbar.stories.jsxsrc/stories/Configuration/BorderRadius.stories.jsxsrc/stories/DataDisplay/DotProgress.stories.jsxsrc/stories/Configuration/Spacing.stories.jsxsrc/stories/Forms/SubmitButton.stories.jsxsrc/stories/Overlay/Popover.stories.jsxsrc/stories/Forms/Button.stories.jsxsrc/stories/Forms/RangeSlider.stories.jsxsrc/stories/DataDisplay/Counter.stories.jsxsrc/stories/Configuration/Paragraphs.stories.jsxsrc/stories/Forms/ToggleButton.stories.jsxsrc/stories/Forms/InlineValuePopover.stories.jsxscripts/createThemeCss.jssrc/stories/DataDisplay/Alert.stories.jsxsrc/stories/Navigation/Breadcrumb.stories.jsxscripts/createThemeFile.jssrc/stories/DataDisplay/Skeleton.stories.jsxsrc/utils/currency.jssrc/stories/DataDisplay/Table.stories.jsxsrc/utils/phone.jssrc/stories/DataDisplay/Dot.stories.jsxsrc/stories/Forms/Textarea.stories.jsxvite.config.jssrc/stories/Media/Logo.stories.jsxsrc/components/Forms/BaseInput.jsxsrc/stories/Forms/Select.stories.jsxsrc/stories/Navigation/Sidebar.stories.jsxsrc/stories/Navigation/Tabs.stories.jsxsrc/stories/Other/Spinner.stories.jsxsrc/stories/Other/Search.stories.jsxsrc/components/Buttons/SubmitButton.jsxsrc/stories/Overlay/Modal.stories.jsxsrc/stories/DataDisplay/Currency.stories.jsxsrc/stories/DataDisplay/Flash.stories.jsxsrc/stories/Forms/ButtonGroup.stories.jsxsrc/stories/Overlay/PopoverList.stories.jsxsrc/stories/Media/Images.stories.jsxsrc/stories/Forms/Input.stories.jsxsrc/stories/Configuration/Fonts.stories.jsxsrc/stories/Overlay/Tooltip.stories.jsxsrc/stories/Overlay/BottomSheet.stories.jsxsrc/stories/Media/Avatar.stories.jsxsrc/components/Forms/ComboBox.jsxsrc/stories/DataDisplay/Tag.stories.jsxsrc/stories/Overlay/Drawer.stories.jsxsrc/components/HeaderToolbar.jsxsrc/stories/DataDisplay/DateRangePicker.stories.jsxsrc/stories/Configuration/Colors.stories.jsxsrc/stories/DataDisplay/Phone.stories.jsxsrc/stories/DataDisplay/Key.stories.jsxsrc/stories/Forms/Switch.stories.jsxsrc/components/DatePicker/RelativeDateRange.jsxsrc/utils/date.jssrc/components/Utilities/Currency.jsxsrc/hooks/useViewportHeight.jssrc/stories/Forms/ComboBox.stories.jsxsrc/stories/Screens/Login.stories.jsxsrc/utils/numbers.jssrc/stories/Media/ImageUpload.stories.jsxtailwind.config.jssrc/helpers/browser.jssrc/stories/DataDisplay/DatePicker.stories.jsx
src/stories/**/*.jsx
📄 CodeRabbit inference engine (CLAUDE.md)
Add a Storybook story for new components, organizing stories to mirror the Storybook sidebar categories.
Files:
src/stories/Forms/SecondaryButton.stories.jsxsrc/stories/Configuration/Links.stories.jsxsrc/stories/Forms/Checkbox.stories.jsxsrc/stories/Other/Breakdown.stories.jsxsrc/stories/Media/Icons.stories.jsxsrc/stories/DataDisplay/Number.stories.jsxsrc/stories/DataDisplay/Badge.stories.jsxsrc/stories/Configuration/Text.stories.jsxsrc/stories/Other/HeaderToolbar.stories.jsxsrc/stories/Configuration/BorderRadius.stories.jsxsrc/stories/DataDisplay/DotProgress.stories.jsxsrc/stories/Configuration/Spacing.stories.jsxsrc/stories/Forms/SubmitButton.stories.jsxsrc/stories/Overlay/Popover.stories.jsxsrc/stories/Forms/Button.stories.jsxsrc/stories/Forms/RangeSlider.stories.jsxsrc/stories/DataDisplay/Counter.stories.jsxsrc/stories/Configuration/Paragraphs.stories.jsxsrc/stories/Forms/ToggleButton.stories.jsxsrc/stories/Forms/InlineValuePopover.stories.jsxsrc/stories/DataDisplay/Alert.stories.jsxsrc/stories/Navigation/Breadcrumb.stories.jsxsrc/stories/DataDisplay/Skeleton.stories.jsxsrc/stories/DataDisplay/Table.stories.jsxsrc/stories/DataDisplay/Dot.stories.jsxsrc/stories/Forms/Textarea.stories.jsxsrc/stories/Media/Logo.stories.jsxsrc/stories/Forms/Select.stories.jsxsrc/stories/Navigation/Sidebar.stories.jsxsrc/stories/Navigation/Tabs.stories.jsxsrc/stories/Other/Spinner.stories.jsxsrc/stories/Other/Search.stories.jsxsrc/stories/Overlay/Modal.stories.jsxsrc/stories/DataDisplay/Currency.stories.jsxsrc/stories/DataDisplay/Flash.stories.jsxsrc/stories/Forms/ButtonGroup.stories.jsxsrc/stories/Overlay/PopoverList.stories.jsxsrc/stories/Media/Images.stories.jsxsrc/stories/Forms/Input.stories.jsxsrc/stories/Configuration/Fonts.stories.jsxsrc/stories/Overlay/Tooltip.stories.jsxsrc/stories/Overlay/BottomSheet.stories.jsxsrc/stories/Media/Avatar.stories.jsxsrc/stories/DataDisplay/Tag.stories.jsxsrc/stories/Overlay/Drawer.stories.jsxsrc/stories/DataDisplay/DateRangePicker.stories.jsxsrc/stories/Configuration/Colors.stories.jsxsrc/stories/DataDisplay/Phone.stories.jsxsrc/stories/DataDisplay/Key.stories.jsxsrc/stories/Forms/Switch.stories.jsxsrc/stories/Forms/ComboBox.stories.jsxsrc/stories/Screens/Login.stories.jsxsrc/stories/Media/ImageUpload.stories.jsxsrc/stories/DataDisplay/DatePicker.stories.jsx
src/components/**/*.jsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.jsx: Implement components as named functional components using hooks, typicallyconst ComponentName = (props) => { ... }.
Define every public prop withprop-types, includingchildrenandclassName; do not omit prop types because a prop seems obvious.
Destructure component props in the function signature instead of accessing properties throughprops.x.
Useclsxfor conditionalclassNameconstruction instead of template literals or string concatenation.
Keep JSX indentation to a maximum of six levels.
Avoid custom CSS and inlinestyleprops; if Tailwind cannot express the design, flag the limitation rather than bypassing the convention.
Do not add dark mode support.
Validate new component behavior through an accompanying Storybook story rather than a rendered unit test; do not introduce@testing-library/reactwithout discussion.
Files:
src/components/Utilities/Number.jsxsrc/components/Drawer.jsxsrc/components/Buttons/SecondaryButton.jsxsrc/components/BottomSheet.jsxsrc/components/DatePicker/DatePickerPopover.jsxsrc/components/GooglePlacesAutocomplete.jsxsrc/components/Forms/Input.jsxsrc/components/DatePicker/NavbarElement.jsxsrc/components/Popover/Popover.jsxsrc/components/Forms/Textarea.jsxsrc/components/Forms/BaseInput.jsxsrc/components/Buttons/SubmitButton.jsxsrc/components/Forms/ComboBox.jsxsrc/components/HeaderToolbar.jsxsrc/components/DatePicker/RelativeDateRange.jsxsrc/components/Utilities/Currency.jsx
**/*.test.{js,jsx}
📄 CodeRabbit inference engine (.claude/rules/testing.md)
**/*.test.{js,jsx}: Use Vitest for all JavaScript and JSX test files; do not add@testing-library/reactwithout discussion, and validate component behavior visually through Storybook rather than rendered unit tests.
Cover pure logic insrc/utils/,src/helpers/, andsrc/hooks/, including data transformations and branching behavior independent of rendering.
Do not hard-code dates in tests; for date-dependent helpers, generate dates dynamically relative toDate.now().
Test both happy paths and edge or failure cases, including empty input,null/undefined, and boundary values.
Never test for the presence of specific Tailwind class-name strings.
Do not mock third-party libraries such asdayjs,clsx, ordownshiftwhen testing trivial pass-through logic; test the helper's actual behavior instead.
Keep helper tests focused on one function's contract and do not reach into unrelated modules to set up a test.
Files:
src/utils/ssr-safe.test.js
src/utils/**/*.test.js
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for tests. New pure logic in
src/utils/andsrc/helpers/must have Vitest coverage.
Files:
src/utils/ssr-safe.test.js
src/hooks/**/*.{js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useMount,useUnmount, anduseMemoizedFnfromahooksinstead of hand-rolleduseEffectoruseCallbackequivalents.
Files:
src/hooks/useViewportHeight.js
tailwind.config.js
📄 CodeRabbit inference engine (CLAUDE.md)
Define the Tailwind theme and custom color scale in
tailwind.config.js, which is the single source of truth for generated theme files.
Files:
tailwind.config.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: xola/ui-kit
Timestamp: 2026-07-23T10:13:55.734Z
Learning: Test files must match their source filename and live next to the source file, such as `avatar.js` and `avatar.test.js` in the same directory.
Learnt from: CR
Repo: xola/ui-kit
Timestamp: 2026-07-23T10:13:55.734Z
Learning: For new or changed components, add or update a Storybook story under `src/stories/` instead of adding a Vitest test, so behavior is visible in Storybook.
🪛 ast-grep (0.44.1)
src/stories/Configuration/BorderRadius.stories.jsx
[warning] 18-21: A list component should have a key to prevent re-rendering
Context:
rounded
{name}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
src/stories/Configuration/Spacing.stories.jsx
[warning] 30-30: A list component should have a key to prevent re-rendering
Context: w-{key}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 36-36: A list component should have a key to prevent re-rendering
Context: {spacing[key]}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
src/stories/DataDisplay/Currency.stories.jsx
[warning] 84-84: A list component should have a key to prevent re-rendering
Context: {currency}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 105-105: A list component should have a key to prevent re-rendering
Context: Locale: {locale}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
src/stories/Configuration/Colors.stories.jsx
[warning] 40-40: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 41-43: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 42-42: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🪛 zizmor (1.26.1)
.github/workflows/deploy-icons.yml
[warning] 16-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
.github/workflows/deploy.yml
[warning] 16-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
.github/workflows/publish.yml
[warning] 12-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 9-9: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
.github/workflows/storybook-test.yml
[warning] 18-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 1-44: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 3-6: use of fundamentally insecure workflow trigger (dangerous-triggers): pull_request_target is almost always used insecurely
(dangerous-triggers)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
.github/workflows/eslint.yml
[error] 29-29: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 42-46: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 42-42: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 49-49: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 64-64: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (101)
src/components/BottomSheet.jsx (1)
3-3: LGTM!src/components/Buttons/SecondaryButton.jsx (1)
29-29: LGTM!src/components/Buttons/SubmitButton.jsx (1)
92-97: LGTM!src/components/DatePicker/LocalizedDayPicker.tsx (1)
8-11: LGTM!Also applies to: 21-22
src/components/DatePicker/NavbarElement.jsx (1)
27-29: LGTM!src/components/DatePicker/RelativeDateRange.jsx (2)
367-367: LGTM!
76-76: 🎯 Functional CorrectnessNo action needed: keep
dateRangesinternal. It’s only used insidesrc/components/DatePicker/RelativeDateRange.jsx, and the package surface exportsRelativeDateRange,rangeLabels, andrangeOptionsonly.> Likely an incorrect or invalid review comment.src/components/Drawer.jsx (1)
128-129: LGTM!src/helpers/browser.js (1)
1-1: LGTM!Also applies to: 3-11
src/hooks/useViewportHeight.js (1)
9-22: LGTM!src/components/DatePicker/DatePickerPopover.jsx (1)
135-137: LGTM!src/components/Forms/BaseInput.jsx (1)
74-85: LGTM!src/components/Forms/ComboBox.jsx (1)
31-32: LGTM!Also applies to: 56-57
src/components/Forms/Input.jsx (1)
10-11: LGTM!src/components/Forms/Textarea.jsx (1)
30-31: LGTM!src/components/GooglePlacesAutocomplete.jsx (1)
3-3: LGTM!src/components/HeaderToolbar.jsx (1)
46-50: LGTM!src/components/Popover/Popover.jsx (1)
82-83: LGTM!eslint.config.mjs (1)
1-15: LGTM!knip.json (1)
1-6: LGTM!package.json (1)
6-6: LGTM!Also applies to: 13-13, 30-37, 43-48, 63-85, 92-93
postcss.config.js (1)
1-4: LGTM!scripts/createThemeCss.js (1)
6-11: LGTM!scripts/createThemeFile.js (1)
2-3: LGTM!scripts/prepare.js (1)
2-3: LGTM!src/components/Utilities/Currency.jsx (1)
5-9: LGTM!Also applies to: 62-69, 98-98
src/utils/currency.js (1)
1-9: LGTM!tailwind.config.js (1)
1-6: LGTM!Also applies to: 132-132, 247-247
vitest.config.js (1)
1-40: LGTM!src/icons/eslint.config.mjs (1)
1-5: LGTM!src/icons/package.json (1)
19-20: LGTM!Also applies to: 48-49
src/stories/Introduction.mdx (1)
1-1: LGTM!src/components/Utilities/Number.jsx (1)
4-6: LGTM!src/utils/userLocale.js (1)
1-21: LGTM!.storybook/main.js (1)
1-49: LGTM!.storybook/manager.js (1)
1-1: LGTM!.storybook/preview.jsx (1)
1-26: LGTM!.storybook/xola.js (1)
1-1: LGTM!src/stories/Installation.mdx (1)
1-2: LGTM!src/utils/numbers.js (1)
3-3: LGTM!Also applies to: 13-13, 45-45
src/utils/phone.js (1)
4-18: LGTM!Also applies to: 37-38
src/utils/date.js (2)
1-1: LGTM!Also applies to: 55-55, 81-90
14-14: 🎯 Functional CorrectnessNo API break here —
DateFormatanddateToStringare local-only insrc/utils/date.js;src/index.jsonly re-exportsformatDate,formatTime,dateFromObjectId, and related helpers, so consumers can’t import those symbols from the package.> Likely an incorrect or invalid review comment.src/stories/Configuration/BorderRadius.stories.jsx (1)
1-37: LGTM!src/stories/Configuration/Colors.stories.jsx (1)
1-67: LGTM!src/stories/Configuration/Fonts.stories.jsx (1)
1-56: LGTM!src/stories/DataDisplay/DatePicker.stories.jsx (1)
3-11: LGTM!Also applies to: 58-73, 100-108, 138-147, 167-175, 210-213, 229-234, 250-255, 270-274, 299-304, 337-341, 352-357, 371-375, 394-397, 430-438, 476-479
src/stories/DataDisplay/DateRangePicker.stories.jsx (1)
3-51: LGTM!Also applies to: 53-57, 75-82, 101-106, 135-139
src/stories/DataDisplay/Dot.stories.jsx (1)
2-2: LGTM!Also applies to: 34-39, 53-57
src/stories/DataDisplay/DotProgress.stories.jsx (1)
2-2: LGTM!Also applies to: 41-42, 44-47
src/stories/Overlay/Popover.stories.jsx (1)
2-2: LGTM!Also applies to: 92-99
src/stories/Overlay/PopoverList.stories.jsx (1)
2-2: LGTM!Also applies to: 101-102, 104-108, 126-132, 187-192
src/stories/Overlay/Tooltip.stories.jsx (1)
2-2: LGTM!Also applies to: 88-93
src/stories/Screens/Login.stories.jsx (1)
2-2: LGTM!Also applies to: 50-57, 67-70, 80-92
src/stories/Configuration/Links.stories.jsx (1)
1-32: LGTM!src/stories/Configuration/Paragraphs.stories.jsx (1)
1-40: LGTM!src/stories/Configuration/Text.stories.jsx (1)
1-49: LGTM!src/stories/DataDisplay/Flash.stories.jsx (1)
93-100: LGTM!src/stories/DataDisplay/Key.stories.jsx (1)
2-2: LGTM!Also applies to: 31-45, 58-64
src/stories/DataDisplay/Number.stories.jsx (1)
2-2: LGTM!Also applies to: 57-60
src/stories/DataDisplay/Phone.stories.jsx (1)
2-2: LGTM!Also applies to: 45-50
src/stories/DataDisplay/Skeleton.stories.jsx (1)
2-2: LGTM!Also applies to: 14-29
src/stories/DataDisplay/Table.stories.jsx (1)
2-2: LGTM!Also applies to: 96-103, 154-159
src/stories/DataDisplay/Tag.stories.jsx (1)
2-2: LGTM!Also applies to: 48-80
src/stories/Configuration/Spacing.stories.jsx (1)
18-20: 🎯 Functional CorrectnessNo issue here.
tailwind.config.jsonly defines numeric spacing keys, soNumber.parseFloat()won’t hit apxtoken.> Likely an incorrect or invalid review comment.src/stories/DataDisplay/Alert.stories.jsx (1)
2-2: LGTM!Also applies to: 54-57, 69-75
src/stories/DataDisplay/Badge.stories.jsx (1)
2-2: LGTM!Also applies to: 46-49
src/stories/DataDisplay/Counter.stories.jsx (1)
2-2: LGTM!Also applies to: 33-38
src/stories/DataDisplay/Currency.stories.jsx (1)
84-84: LGTM!Also applies to: 105-105, 146-146
src/stories/Other/Breakdown.stories.jsx (1)
2-2: LGTM!Also applies to: 86-92
src/stories/Other/HeaderToolbar.stories.jsx (1)
2-2: LGTM!Also applies to: 40-46
src/stories/Other/Search.stories.jsx (1)
3-3: LGTM!Also applies to: 84-88, 97-102
src/stories/Other/Spinner.stories.jsx (1)
2-2: LGTM!Also applies to: 37-40, 51-54
src/stories/Overlay/BottomSheet.stories.jsx (1)
3-3: LGTM!Also applies to: 59-67, 103-109
src/stories/Overlay/Drawer.stories.jsx (1)
2-2: LGTM!Also applies to: 61-69
src/stories/Overlay/Modal.stories.jsx (1)
2-2: LGTM!Also applies to: 89-100
src/stories/Forms/Button.stories.jsx (1)
2-2: LGTM!Also applies to: 57-68
src/stories/Forms/Checkbox.stories.jsx (1)
2-2: LGTM!Also applies to: 27-32, 43-49
src/stories/Forms/ComboBox.stories.jsx (1)
3-12: LGTM!Also applies to: 60-70, 95-98
src/stories/Forms/InlineValuePopover.stories.jsx (1)
3-3: LGTM!Also applies to: 60-66, 106-110
src/stories/Forms/Input.stories.jsx (1)
2-2: LGTM!Also applies to: 26-32, 64-72
src/stories/Forms/RangeSlider.stories.jsx (1)
2-2: LGTM!Also applies to: 21-25, 39-41
src/stories/Forms/SecondaryButton.stories.jsx (1)
2-2: LGTM!Also applies to: 36-41
src/stories/Forms/SubmitButton.stories.jsx (1)
2-2: LGTM!Also applies to: 106-109
src/stories/Forms/Switch.stories.jsx (1)
2-2: LGTM!Also applies to: 41-46, 58-63, 65-76
src/stories/Forms/Textarea.stories.jsx (1)
2-2: LGTM!Also applies to: 26-32, 63-68
src/stories/Forms/ToggleButton.stories.jsx (1)
2-2: LGTM!Also applies to: 53-53, 69-74
src/stories/Media/Avatar.stories.jsx (1)
2-2: LGTM!Also applies to: 68-95
src/stories/Media/Icons.stories.jsx (1)
4-4: LGTM!Also applies to: 133-136
src/stories/Media/ImageUpload.stories.jsx (1)
2-2: LGTM!Also applies to: 45-48, 129-132
src/stories/Media/Images.stories.jsx (1)
3-3: LGTM!Also applies to: 84-87, 108-111
src/stories/Media/Logo.stories.jsx (1)
2-2: LGTM!Also applies to: 47-52, 63-66
src/stories/Navigation/Breadcrumb.stories.jsx (1)
2-2: LGTM!Also applies to: 50-54, 67-72
src/stories/Navigation/Sidebar.stories.jsx (1)
2-2: LGTM!Also applies to: 135-139, 157-161, 241-245
src/stories/Navigation/Tabs.stories.jsx (1)
2-2: LGTM!Also applies to: 66-66, 71-76, 103-108
.claude/rules/commenting.md (1)
16-16: LGTM!Also applies to: 53-60, 79-79, 92-92, 121-121
.claude/rules/testing.md (1)
11-17: LGTM!Also applies to: 32-41
.github/workflows/publish.yml (1)
4-6: LGTM!Also applies to: 21-27
.gitignore (1)
13-13: LGTM!CLAUDE.md (1)
18-28: LGTM!Also applies to: 35-58, 64-66, 90-99, 109-111
README.md (1)
19-40: LGTM!Also applies to: 45-48, 64-68, 124-130, 151-167
| on: | ||
| push: | ||
| pull_request_target: | ||
| types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not execute pull-request code under pull_request_target.
This workflow checks out the PR head and runs its npm scripts, so a malicious PR can execute with the base repository’s GITHUB_TOKEN and default permissions. Use pull_request for these tests and declare minimal permissions such as contents: read; reserve any privileged reporting for a separate trusted workflow.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 3-6: use of fundamentally insecure workflow trigger (dangerous-triggers): pull_request_target is almost always used insecurely
(dangerous-triggers)
🤖 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/storybook-test.yml around lines 3 - 6, Change the workflow
trigger from pull_request_target to pull_request in the on configuration, since
the job checks out and executes pull-request code. Add minimal explicit
permissions, such as contents: read, and keep privileged reporting outside this
test workflow.
Source: Linters/SAST tools
| - uses: actions/checkout@v7 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## checkout usages\n'
rg -n "actions/checkout|persist-credentials" .github/workflows -S
printf '\n## relevant workflow snippets\n'
for f in .github/workflows/storybook-test.yml .github/workflows/deploy-icons.yml .github/workflows/deploy.yml .github/workflows/eslint.yml .github/workflows/publish.yml; do
echo
echo "### $f"
nl -ba "$f" | sed -n '1,140p'
doneRepository: xola/ui-kit
Length of output: 639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "actions/checkout|persist-credentials" .github/workflows -S
for f in .github/workflows/storybook-test.yml .github/workflows/deploy-icons.yml .github/workflows/deploy.yml .github/workflows/eslint.yml .github/workflows/publish.yml; do
echo "### $f"
nl -ba "$f" | sed -n '1,140p'
doneRepository: xola/ui-kit
Length of output: 587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/storybook-test.yml .github/workflows/deploy-icons.yml .github/workflows/deploy.yml .github/workflows/eslint.yml .github/workflows/publish.yml; do
echo "===== $f ====="
nl -ba "$f" | sed -n '1,220p'
doneRepository: xola/ui-kit
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/storybook-test.yml .github/workflows/deploy-icons.yml .github/workflows/deploy.yml .github/workflows/eslint.yml .github/workflows/publish.yml; do
echo "===== $f ====="
sed -n '1,220p' "$f"
echo
doneRepository: xola/ui-kit
Length of output: 6437
🏁 Script executed:
#!/bin/bash
set -euo pipefail
show() {
f="$1"
start="$2"
end="$3"
echo "===== $f ($start-$end) ====="
sed -n "${start},${end}p" "$f"
echo
}
show .github/workflows/storybook-test.yml 1 80
show .github/workflows/deploy-icons.yml 1 120
show .github/workflows/deploy.yml 1 120
show .github/workflows/eslint.yml 1 120
show .github/workflows/publish.yml 1 120Repository: xola/ui-kit
Length of output: 6476
Disable checkout credential persistence
actions/checkout writes GITHUB_TOKEN into .git/config by default. Add persist-credentials: false to these jobs so later install/build/lint/publish steps don't inherit a writable token:
.github/workflows/storybook-test.yml.github/workflows/deploy-icons.yml.github/workflows/deploy.yml.github/workflows/eslint.yml.github/workflows/publish.yml
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 18-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
📍 Affects 5 files
.github/workflows/storybook-test.yml#L18-L22(this comment).github/workflows/deploy-icons.yml#L16-L16.github/workflows/deploy.yml#L16-L16.github/workflows/eslint.yml#L42-L46.github/workflows/publish.yml#L12-L13
🤖 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/storybook-test.yml around lines 18 - 22, Add
persist-credentials: false to each actions/checkout step in
.github/workflows/storybook-test.yml (lines 18-22),
.github/workflows/deploy-icons.yml (line 16), .github/workflows/deploy.yml (line
16), .github/workflows/eslint.yml (lines 42-46), and
.github/workflows/publish.yml (lines 12-13).
Source: Linters/SAST tools
| - uses: actions/checkout@v7 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
|
|
||
| - name: Node.JS 20 | ||
| uses: actions/setup-node@v7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
while read -r repo ref; do
git ls-remote "https://github.com/${repo}.git" "refs/tags/${ref}" "refs/heads/${ref}"
done <<'EOF'
actions/checkout v7
actions/setup-node v7
tspascoal/get-user-teams-membership v4
ataylorme/eslint-annotate-action v4.0.0-beta.1
tsunematsu21/actions-publish-gh-pages v1.0.2
EOFRepository: xola/ui-kit
Length of output: 434
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for f in .github/workflows/storybook-test.yml .github/workflows/deploy-icons.yml .github/workflows/deploy.yml .github/workflows/eslint.yml .github/workflows/publish.yml; do
echo "== $f =="
nl -ba "$f" | sed -n '1,120p' | sed -n '1,120p'
echo
doneRepository: xola/ui-kit
Length of output: 232
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for f in \
.github/workflows/storybook-test.yml \
.github/workflows/deploy-icons.yml \
.github/workflows/deploy.yml \
.github/workflows/eslint.yml \
.github/workflows/publish.yml
do
echo "== $f =="
sed -n '1,140p' "$f"
echo
doneRepository: xola/ui-kit
Length of output: 6407
Pin these GitHub Actions to immutable commit SHAs.
These refs are still mutable tags:
.github/workflows/storybook-test.yml:actions/checkout@v7,actions/setup-node@v7.github/workflows/deploy-icons.yml:actions/checkout@v7,actions/setup-node@v7.github/workflows/deploy.yml:actions/checkout@v7,actions/setup-node@v7.github/workflows/eslint.yml:tspascoal/get-user-teams-membership@v4,actions/checkout@v7,actions/setup-node@v7,ataylorme/eslint-annotate-action@v4.0.0-beta.1.github/workflows/publish.yml:actions/checkout@v7,actions/setup-node@v7,tsunematsu21/actions-publish-gh-pages@v1.0.2
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 18-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
📍 Affects 5 files
.github/workflows/storybook-test.yml#L18-L25(this comment).github/workflows/deploy-icons.yml#L16-L18.github/workflows/deploy.yml#L16-L18.github/workflows/eslint.yml#L29-L29.github/workflows/eslint.yml#L42-L49.github/workflows/eslint.yml#L64-L68.github/workflows/publish.yml#L12-L15.github/workflows/publish.yml#L28-L28
🤖 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/storybook-test.yml around lines 18 - 25, Pin every listed
GitHub Action to its full immutable commit SHA instead of mutable version tags.
Update actions/checkout and actions/setup-node in
.github/workflows/storybook-test.yml (18-25), .github/workflows/deploy-icons.yml
(16-18), and .github/workflows/deploy.yml (16-18); update
tspascoal/get-user-teams-membership, actions/checkout, actions/setup-node, and
ataylorme/eslint-annotate-action in .github/workflows/eslint.yml (29-29, 42-49,
64-68); and update actions/checkout, actions/setup-node, and
tsunematsu21/actions-publish-gh-pages in .github/workflows/publish.yml (12-15,
28-28).
Source: Linters/SAST tools
| React 18 + Tailwind CSS v3 component library, built with Vite. Plain JavaScript/JSX, no | ||
| TypeScript. `index.d.ts` is hand-maintained for consumers that want types. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the “no TypeScript” architecture claim.
The repository includes src/components/DatePicker/LocalizedDayPicker.tsx, so describing the project as plain JavaScript/JSX is inaccurate. Document the mixed-language boundary instead.
🤖 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 `@CLAUDE.md` around lines 32 - 33, Update the architecture description in
CLAUDE.md to reflect that the component library uses both JavaScript/JSX and
TypeScript/TSX, including the existing DatePicker/LocalizedDayPicker.tsx
boundary. Remove the inaccurate “no TypeScript” claim while preserving the Vite,
React, Tailwind, and hand-maintained index.d.ts details.
| @source "./node_modules/@xola/ui-kit/build/ui-kit.es.js"; | ||
| @import "@xola/ui-kit/build/theme.css"; /* after any of your own theme tokens */ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced README section with line numbers.
sed -n '1,120p' README.md | nl -ba | sed -n '30,60p'Repository: xola/ui-kit
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any other references to the same Tailwind `@source` pattern.
rg -n '`@source` "./node_modules|`@xola/ui-kit/build/ui-kit.es.js`|theme.css' README.md . || trueRepository: xola/ui-kit
Length of output: 1462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Determine whether README examples refer to a CSS file location that would affect relative paths.
rg -n 'src/index.css|index.css|tailwind' README.md . || trueRepository: xola/ui-kit
Length of output: 9031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the repo structure for build output and package metadata relevant to the path claim.
git ls-files | rg '^(README\.md|package\.json|.*tailwind.*|.*css)$' || trueRepository: xola/ui-kit
Length of output: 574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '34,48p' README.md | nl -baRepository: xola/ui-kit
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=108 && NR<=128 { printf "%4d %s\n", NR, $0 }' README.mdRepository: xola/ui-kit
Length of output: 1160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=36 && NR<=43 { printf "%4d %s\n", NR, $0 }' README.mdRepository: xola/ui-kit
Length of output: 802
Update the Tailwind v4 @source example to use a stylesheet-relative path. ./node_modules/... resolves from the CSS entry file, so in a common src/index.css setup it points at src/node_modules; use ../node_modules/... (and the same fix in the later duplicate example).
🤖 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 `@README.md` around lines 41 - 42, Update both Tailwind v4 `@source` examples in
README.md to use the stylesheet-relative ../node_modules/@xola/ui-kit path
instead of ./node_modules, including the later duplicate example; leave the
`@import` path unchanged.
| Default.play = async ({ canvas, userEvent }) => { | ||
| const second = canvas.getByRole("button", { name: "Second" }); | ||
| // No button is active until one is clicked | ||
| await expect(second).not.toHaveClass("bg-primary"); | ||
| await userEvent.click(second); | ||
| await expect(second).toHaveClass("bg-primary"); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stop asserting Tailwind implementation classes.
src/stories/Forms/ButtonGroup.stories.jsx#L48-L54: expose and assert semantic selected state (for example,aria-pressed) instead ofbg-primary.src/stories/Forms/Input.stories.jsx#L101-L105: remove the.bg-dangerassertion; the required-dot appearance is visual Storybook coverage, not a class-name contract.
As per coding guidelines, “Never test for the presence of specific Tailwind class-name strings.”
📍 Affects 2 files
src/stories/Forms/ButtonGroup.stories.jsx#L48-L54(this comment)src/stories/Forms/Input.stories.jsx#L101-L105
🤖 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 `@src/stories/Forms/ButtonGroup.stories.jsx` around lines 48 - 54, Replace the
Tailwind class assertions in Default.play of
src/stories/Forms/ButtonGroup.stories.jsx (lines 48-54) with assertions against
the button’s semantic selected state, exposing aria-pressed in the ButtonGroup
implementation if needed. Remove the .bg-danger assertion in
src/stories/Forms/Input.stories.jsx (lines 101-105); leave required-dot
appearance as visual Storybook coverage.
Source: Coding guidelines
| Default.play = async ({ canvas, userEvent }) => { | ||
| const select = canvas.getByRole("combobox"); | ||
| await expect(select).toHaveValue("1"); | ||
| await userEvent.selectOptions(select, "Two"); | ||
| await expect(select).toHaveValue("2"); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/stories/Forms/Select.stories.jsx --items all
rg -n -C2 'option value=|selectOptions\(' src/stories/Forms/Select.stories.jsxRepository: xola/ui-kit
Length of output: 2520
Select the option by value.
userEvent.selectOptions matches against option values, so "Two" won’t select this option.
Proposed fix
- await userEvent.selectOptions(select, "Two");
+ await userEvent.selectOptions(select, "2");📝 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.
| Default.play = async ({ canvas, userEvent }) => { | |
| const select = canvas.getByRole("combobox"); | |
| await expect(select).toHaveValue("1"); | |
| await userEvent.selectOptions(select, "Two"); | |
| await expect(select).toHaveValue("2"); | |
| }; | |
| Default.play = async ({ canvas, userEvent }) => { | |
| const select = canvas.getByRole("combobox"); | |
| await expect(select).toHaveValue("1"); | |
| await userEvent.selectOptions(select, "2"); | |
| await expect(select).toHaveValue("2"); | |
| }; |
🤖 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 `@src/stories/Forms/Select.stories.jsx` around lines 24 - 29, Update the
Default story’s userEvent.selectOptions call to pass the option’s value rather
than its display label, while preserving the existing assertions that verify the
initial and selected values.
| }; | ||
|
|
||
| Default.play = async ({ canvas, userEvent }) => { | ||
| // Only the active panel is rendered |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove comments that only restate the assertions.
These comments explain what the following assertions do rather than why the behavior matters; the assertions are already self-explanatory.
Also applies to: 69-69
🤖 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 `@src/stories/Navigation/Tabs.stories.jsx` at line 67, Remove the comments
immediately preceding the assertions at the referenced locations in the story,
including the “Only the active panel is rendered” comment, while leaving the
assertions and their behavior unchanged.
Source: Coding guidelines
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| // These utils previously called getUserLocale() and PhoneNumberUtil.getInstance() | ||
| // at module load, which crashed when imported without a browser (SSR/Node). This | ||
| // suite runs in the node environment (no `navigator`) and verifies they import | ||
| // and execute with the "en-US" fallback instead of throwing. | ||
| describe("SSR-safe utils (no navigator)", () => { | ||
| it("runs without a browser navigator", () => { | ||
| expect(typeof navigator).toBe("undefined"); | ||
| }); | ||
|
|
||
| it("numbers util imports and formats", async () => { | ||
| const { numberFormat, compactNumber } = await import("./numbers"); | ||
| expect(numberFormat(1234.5)).toBe("1,234.50"); | ||
| expect(typeof compactNumber(1_234_567)).toBe("string"); | ||
| }); | ||
|
|
||
| it("currency util imports and returns a symbol", async () => { | ||
| const { getSymbol } = await import("./currency"); | ||
| expect(typeof getSymbol("USD")).toBe("string"); | ||
| }); | ||
|
|
||
| it("phone util imports and formats", async () => { | ||
| const { formatPhoneNumber } = await import("./phone"); | ||
| expect(formatPhoneNumber("5402322157", "US")).toBe("(540) 232-2157"); | ||
| }); | ||
|
|
||
| it("date util imports", async () => { | ||
| const { formatDate } = await import("./date"); | ||
| expect(typeof formatDate).toBe("function"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Split this cross-utility test into source-matched suites.
Move each SSR regression check into its corresponding numbers.test.js, currency.test.js, phone.test.js, or date.test.js file. Keep each suite scoped to that utility’s contract.
As per coding guidelines, “Keep helper tests focused on one function's contract and do not reach into unrelated modules to set up a test.” Based on learnings, test files must match their source filename and live next to it.
🤖 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 `@src/utils/ssr-safe.test.js` around lines 1 - 32, Split the SSR regression
cases from the cross-utility suite into the source-matched numbers.test.js,
currency.test.js, phone.test.js, and date.test.js suites beside their respective
utilities. Move each dynamic import and contract assertion into its
corresponding file, retain the no-navigator SSR coverage there, and remove the
cross-utility ssr-safe.test.js coverage.
Sources: Coding guidelines, Learnings
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 10 minutes. |
|
@greptileai please review |
Greptile SummaryThis PR migrates the Storybook dev toolchain from webpack5 to Vite + Storybook 10, converts the package to ESM-only (
Confidence Score: 3/5The Storybook/Vite migration, ESM conversion, and SSR utility refactors are clean and well-tested; the new storybook-test.yml CI workflow runs untrusted fork code with a base-repo write-scoped token and no contributor gate. The component and build changes are solid — Vite/Storybook 10 migration is consistent, the SSR-safe locale/phone refactor is covered by a new Node-environment test suite, and the ESM conversion is thorough. The single issue requiring attention before merging is the new storybook-test.yml: it uses pull_request_target to obtain a write-scoped GITHUB_TOKEN, then checks out and executes arbitrary scripts from a PR fork with no membership check — a pattern the repo's own eslint.yml already guards against with a team gate. Files Needing Attention: .github/workflows/storybook-test.yml needs a team-membership gate (matching eslint.yml) before any fork code is executed.
|
| Filename | Overview |
|---|---|
| .github/workflows/storybook-test.yml | New CI workflow uses pull_request_target + fork checkout without a team-membership gate, allowing untrusted fork code to run with base-repo write privileges. |
| package.json | Major overhaul: ESM-only ("type":"module"), React moved to peerDependencies, Storybook 10 + Vite, jslint 4 flat config, lint script now auto-fixes (--fix) with lint:ci added for check-only CI use. |
| .storybook/main.js | Migrated from webpack5 to Vite builder; drops babel/webpack addons; adds viteFinal plugin that substitutes the root tailwind config with the browser-safe pre-generated theme object for Configuration stories. |
| vite.config.js | Bundles react-day-picker and google-libphonenumber (CJS-only) into the ESM output to fix named-export resolution in Node ESM context; all other deps remain external. |
| vitest.config.js | Introduces Vitest project config: unit project for Node-environment pure-logic tests, storybook project for browser-based Storybook interaction tests via Playwright. |
| src/utils/userLocale.js | New SSR-safe locale helper: lazily calls getUserLocale() on first use with navigator guard and en-US fallback; replaces eager module-level calls that crashed Node imports. |
| src/utils/phone.js | Defers PhoneNumberUtil.getInstance() to first call via a lazy singleton; fixes eager side-effect on module import that affected SSR startup. |
| src/utils/ssr-safe.test.js | New Vitest suite running in Node environment (no navigator) that verifies the SSR-safe lazy-init refactors for numbers, currency, phone, and date utils. |
| src/utils/date.js | Switches isDayjs calls to dayjs.isDayjs; removes export from DateFormat and dateToString (internal helpers not part of the public API). Clean non-breaking change. |
| eslint.config.mjs | New ESLint 9 flat config using @xola/jslint; correctly excludes stories, generated theme files, and test files from linting. |
| CLAUDE.md | Updated to reflect Vite/Storybook 10 and ESM-only architecture; npm run lint comment implies check-only but the script actually runs with --fix. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR push / pull_request_target] --> B{storybook-test.yml}
B -->|npm ci| C[Runs fork prepare scripts]
B -->|npm test| D[Runs fork test scripts]
C --> E[GITHUB_TOKEN accessible]
D --> E
A2[PR push / pull_request_target] --> F{eslint.yml}
F --> G[Team membership check]
G -->|not in xola/Engineering| H[Exit 1 – no code runs]
G -->|member| I[npm ci + lint:report]
subgraph Storybook Build
J[npm run build] --> K[vite build to build/]
K --> L[viteFinal plugin intercepts tailwind.config.js]
L --> M[Returns JSON from src/theme.js instead]
end
subgraph Vitest Projects
N[vitest.config.js] --> O[unit: Node env src/**/*.test.js]
N --> P[storybook: Playwright Chromium]
end
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
.github/workflows/storybook-test.yml:5-22
**Untrusted fork code runs under `pull_request_target` privileges**
`pull_request_target` gives the workflow write-level `GITHUB_TOKEN` (base-repo context), but the checkout checks out the PR head from an untrusted fork and then runs `npm ci` (which executes the fork's `prepare` script) followed by `npm test` and `npm run test:storybook`. A malicious PR author can add a `postinstall` hook to the fork's `package.json` that exfiltrates the `GITHUB_TOKEN` or pushes commits to the base repo. Compare `eslint.yml`, which defends the same `pull_request_target` + fork-checkout pattern with a team-membership gate before touching the PR code. This workflow has no such gate.
### Issue 2 of 2
CLAUDE.md:18-19
The `lint` script comment says "eslint on src" (implying read-only check), but the actual command in `package.json` includes `--fix` — auto-rewriting files silently. `lint` and `lint:fix` are now identical. The no-fix variant is `lint:ci`. Updating the comment avoids surprising developers who run `npm run lint` expecting a dry-run check.
```suggestion
npm run lint # eslint --fix on src (flat config from @xola/jslint); use lint:ci for a read-only check
npm run lint:fix # eslint --fix on src (alias for lint)
npm run lint:ci # eslint check only – exits non-zero on violations without writing files
```
Reviews (1): Last reviewed commit: "chore: 🧹 Lint" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| pull_request_target: | ||
| types: [opened, synchronize, reopened] | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| test: | ||
| name: 🧪 Storybook Tests | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v7 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} |
There was a problem hiding this comment.
pull_request_target privileges
pull_request_target gives the workflow write-level GITHUB_TOKEN (base-repo context), but the checkout checks out the PR head from an untrusted fork and then runs npm ci (which executes the fork's prepare script) followed by npm test and npm run test:storybook. A malicious PR author can add a postinstall hook to the fork's package.json that exfiltrates the GITHUB_TOKEN or pushes commits to the base repo. Compare eslint.yml, which defends the same pull_request_target + fork-checkout pattern with a team-membership gate before touching the PR code. This workflow has no such gate.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/storybook-test.yml
Line: 5-22
Comment:
**Untrusted fork code runs under `pull_request_target` privileges**
`pull_request_target` gives the workflow write-level `GITHUB_TOKEN` (base-repo context), but the checkout checks out the PR head from an untrusted fork and then runs `npm ci` (which executes the fork's `prepare` script) followed by `npm test` and `npm run test:storybook`. A malicious PR author can add a `postinstall` hook to the fork's `package.json` that exfiltrates the `GITHUB_TOKEN` or pushes commits to the base repo. Compare `eslint.yml`, which defends the same `pull_request_target` + fork-checkout pattern with a team-membership gate before touching the PR code. This workflow has no such gate.
How can I resolve this? If you propose a fix, please make it concise.| npm run lint # eslint on src (flat config from @xola/jslint) | ||
| npm run lint:fix # eslint --fix on src |
There was a problem hiding this comment.
lint script comment says "eslint on src" (implying read-only check), but the actual command in package.json includes --fix — auto-rewriting files silently. lint and lint:fix are now identical. The no-fix variant is lint:ci. Updating the comment avoids surprising developers who run npm run lint expecting a dry-run check.
| npm run lint # eslint on src (flat config from @xola/jslint) | |
| npm run lint:fix # eslint --fix on src | |
| npm run lint # eslint --fix on src (flat config from @xola/jslint); use lint:ci for a read-only check | |
| npm run lint:fix # eslint --fix on src (alias for lint) | |
| npm run lint:ci # eslint check only – exits non-zero on violations without writing files |
Prompt To Fix With AI
This is a comment left during a code review.
Path: CLAUDE.md
Line: 18-19
Comment:
The `lint` script comment says "eslint on src" (implying read-only check), but the actual command in `package.json` includes `--fix` — auto-rewriting files silently. `lint` and `lint:fix` are now identical. The no-fix variant is `lint:ci`. Updating the comment avoids surprising developers who run `npm run lint` expecting a dry-run check.
```suggestion
npm run lint # eslint --fix on src (flat config from @xola/jslint); use lint:ci for a read-only check
npm run lint:fix # eslint --fix on src (alias for lint)
npm run lint:ci # eslint check only – exits non-zero on violations without writing files
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Migrates Storybook to the Vite builder and Storybook 10, makes the package ESM-only, and moves linting to
@xola/jslint3.2 flat config (ESLint 9). Dev-tooling and packaging changes; no component behavior or public component API change. The package is"type": "module"and the publishedtailwind.config.js/postcss.config.jsare ESM (export default).Important
Consumers that do
require("@xola/ui-kit/tailwind.config")must switch to an ESM re-export, see the updated README Tailwind v3 section.Important
reactandreact-dommoved fromdependenciestopeerDependencies(>=18). Consuming apps already provide React, so this avoids duplicate copies; install commands in the README are updated accordingly.Commits
chromaticscript, and.github/workflows/chromatic.ymlremoved (to be set up fresh).@storybook/react-vite, drop webpack-only addons + hand-rolled CSS loader block. Rename 54 stories + preview.js→.jsx, README?rawimport, dedup argType key.storybook/theming,storybook/manager-api),Metafrom@storybook/addon-docs/blocks,features.backgrounds:false.type: module+ all remaining CJS files converted.@storybook/*andstorybookon 10.x (react-vite builder).automigrateadded@storybook/addon-mcp.@storybook/addon-docs/blocksimport path unchanged in v10..storybook/main.jsstays ESM with the theme-data viteFinal plugin intact.xola-lintCLI in favor of a flat ESLint config (peers: ESLint 9, TypeScript 5). Addedeslint.config.mjs, removed.xo-config.json/.eslintignore, bumpedtypescriptto 5 and addedeslint9, switched scripts toeslint src.displayNametoforwardRef/compound components and removed staleeslint-disabledirectives for rules no longer in the shared config..claude/rulesupdated for Storybook 10, Vitest, flat-config lint, and the React peer-dep move.Testing
Verified locally with an agent-browser console-error sweep driving the Storybook 10 build:
build,build:storybook, andnpm run devall succeed; no CommonJS/Vite deprecation warnings.#storybook-root, zero console errors), including the Configuration stories that exercise the theme plugin. Sweep result is byte-identical to the Storybook 9 baseline.npm test(Vitest) green.npm run lint(ESLint 9 / jslint flat config) exits 0, only pre-existing complexity warnings remain (DatePicker, Sidebar, Search).Summary by CodeRabbit
en-USfallback.