refactor(ui): improve stories and related components#477
Conversation
Greptile SummaryThis PR performs a comprehensive Storybook and UI component refactor across 91 files. The key changes are:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| services/platform/app/components/ui/primitives/button.tsx | Consolidated button variants: removed default, outline, and duplicate primary; renamed default to primary and outline to secondary. Clean, backward-compatible simplification. |
| services/platform/.storybook/preview.tsx | Major Storybook preview rewrite: adds TanStack Router provider, ThemeContext, and themed docs container. Enables router-dependent components to work in stories. |
| services/platform/.storybook/theme.ts | Adds dark theme for Storybook UI alongside existing light theme. Migrates import from @storybook/theming/create to storybook/theming/create. |
| services/platform/.storybook/main.js | Updates Storybook config: migrates addons to v10, sets SITE_URL default, disables telemetry, uses import.meta.dirname instead of __dirname, adds storybook:build support. |
| services/platform/app/components/ui/dialog/dialog.tsx | Simplifies header padding logic (pr-8 always when close visible) and adds gap/margin tweaks for header actions container. |
| services/platform/app/components/ui/navigation/pagination.tsx | Improves empty state handling: disables all controls when total=0, shows "No items" text instead of "0-0 of 0". |
| services/platform/app/components/ui/navigation/tab-navigation.tsx | Adds scrollbar-hide overflow-x-auto to enable horizontal scrolling for many tabs without visible scrollbar. |
| services/platform/app/components/ui/overlays/sheet.tsx | Converts SheetCloseButton to forwardRef component to properly forward refs and props when used with Radix asChild. |
| services/platform/app/components/ui/data-table/data-table-action-menu.tsx | Updates variant type to match new button variant names: removes default and outline, adds primary. |
| services/platform/app/features/products/components/product-view-dialog.tsx | Refactors from raw label/div markup to Field component for consistent read-only data display. |
| services/platform/app/features/websites/components/website-view-dialog.tsx | Replaces local Label/Value components with Field/FieldGroup components, removes unused local component definitions. |
| .github/workflows/build.yml | Adds Storybook build CI job that runs when platform changes are detected. |
| turbo.json | Adds storybook:build task definition and passthrough environment variables for telemetry opt-out. |
| services/platform/package.json | Migrates Storybook deps from v8 to v10: removes @storybook/addon-essentials, addon-interactions, addon-links, manager-api, test, theming; adds addon-docs, addon-vitest. |
Flowchart
flowchart TD
subgraph ButtonVariants["Button Variant Consolidation"]
OLD_DEFAULT["default (gradient primary)"] -->|renamed| NEW_PRIMARY["primary"]
OLD_OUTLINE["outline (ring border)"] -->|renamed| NEW_SECONDARY["secondary"]
OLD_PRIMARY["primary (branded blue)"] -->|removed| DROPPED1["❌ Removed"]
OLD_SECONDARY["secondary (muted bg)"] -->|replaced by| NEW_SECONDARY
end
subgraph StoryMigration["Storybook v8 → v10"]
SB_TEST["@storybook/test"] -->|migrated| SB_TEST_NEW["storybook/test"]
SB_THEMING["@storybook/theming"] -->|migrated| SB_THEMING_NEW["storybook/theming/create"]
SB_MANAGER["@storybook/manager-api"] -->|migrated| SB_MANAGER_NEW["storybook/manager-api"]
SB_ESSENTIALS["@storybook/addon-essentials"] -->|replaced| SB_DOCS["@storybook/addon-docs + addon-vitest"]
end
subgraph PreviewDecorators["Storybook Preview Stack"]
STORY["Story Component"]
ROUTER["TanStack Router Provider"]
THEME["ThemeContext Provider"]
I18N["I18nProvider"]
THEMETOGGLE["withThemeByClassName"]
THEMETOGGLE --> ROUTER --> THEME --> I18N --> STORY
end
Last reviewed commit: bbd4b07
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Story: Parameters<DecoratorFunction<ReactRenderer>>[0]; | ||
| context: Parameters<DecoratorFunction<ReactRenderer>>[1]; | ||
| }) { | ||
| const router = createStoryRouter(); |
There was a problem hiding this comment.
New router on every render
createStoryRouter() is called inside the component body without memoization, which means a brand-new router instance is created on every render of WithProviders. This can cause unnecessary re-mounts of the entire RouterProvider subtree and could lead to unexpected state resets in stories that trigger re-renders.
Consider memoizing the router:
| const router = createStoryRouter(); | |
| const router = useMemo(() => createStoryRouter(), []); |
📝 WalkthroughWalkthroughThis pull request introduces comprehensive Storybook upgrades and component refactoring for the platform service. It adds a new GitHub Actions workflow job to build Storybook, updates Storybook configuration (main.js, manager.ts, preview.tsx, theme.ts) with routing and theming enhancements, and refactors button variants throughout the codebase from "default"/"outline" to "primary"/"secondary". Additionally, many Storybook stories are converted from args-based to render-based implementations with internal state management. The PR updates Storybook dependencies (removing essentials, interactions, links; adding docs, vitest), adds telemetry-disabling environment variables to multiple Dockerfiles, exports ThemeContext publicly, and refactors various UI components to use consistent Field wrappers and updated styling patterns. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
services/platform/app/components/ui/data-table/data-table-action-menu.stories.tsx (1)
149-167: 🧹 Nitpick | 🔵 Trivial
AllVariantsstory should explicitly passvariant="primary"for the "Primary" item.Line 152 labels the button "Primary" but relies on the implicit default variant. In a story that showcases all variants side by side, each variant should be explicitly specified to be self-documenting and resilient to default-variant changes.
♻️ Proposed fix
- <DataTableActionMenu label="Primary" icon={Plus} onClick={() => {}} /> + <DataTableActionMenu label="Primary" icon={Plus} variant="primary" onClick={() => {}} />services/platform/app/components/ui/navigation/tab-navigation.stories.tsx (1)
136-155:⚠️ Potential issue | 🟡 MinorMissing
variantprop on the Button in the WithActions story.The AI summary indicates this was refactored from
variant="outline"tovariant="secondary", but thevariantprop is absent entirely. The button will use the default variant (likely "primary"), which may not be the intended look for a secondary action alongside tabs.Proposed fix
- <Button size="sm" icon={Plus}> + <Button variant="secondary" size="sm" icon={Plus}>services/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsx (2)
60-68:⚠️ Potential issue | 🟡 MinorPre-existing: toast
descriptionis identical for both connected and not-connected branches.Lines 65–66 produce the same string regardless of
isConnected. Thetitledifferentiates ("update successful" vs "connection successful"), but the description likely should too — e.g., an "updated" variant for the already-connected case.
69-79:⚠️ Potential issue | 🟠 MajorReplace the disconnect error key with a connection-specific error message.
Line 75 uses
failedToDisconnectin thehandleConnectcatch block, which displays a "failed to disconnect" message when a connect/update operation fails. This is incorrect and appears to be copied fromhandleDisconnect(line 97).The i18n file currently has no
failedToConnectkey. Either:
- Add a new
integrations.failedToConnectkey toservices/platform/messages/en.json(and other language files), or- Reuse an existing key like
integrations.failedToTestConnection("Failed to test connection")Then update line 75 to use the appropriate key.
Example fix (requires adding the i18n key)
toast({ title: isConnected ? t('integrations.updateFailed') : t('integrations.connectionTestFailed'), - description: t('integrations.failedToDisconnect', { + description: t('integrations.failedToConnect', { provider: 'Shopify', }), variant: 'destructive', });services/platform/app/components/ui/dialog/confirm-dialog.tsx (1)
90-98: 🧹 Nitpick | 🔵 TrivialRedundant destructive styling override via
confirmButtonVariants.When
variant='destructive', the Button component already applies thedestructivevariant styles (bg-destructive text-destructive-foreground hover:bg-destructive/90). ThenconfirmButtonVariants({ variant })additionally appliesbg-red-600 hover:bg-red-700, overriding the theme token with hardcoded colors. This defeats the purpose of theming and creates a maintenance inconsistency.Consider removing
confirmButtonVariantsentirely and relying solely on the Button's built-indestructivevariant.♻️ Proposed refactor
-import { cva } from 'class-variance-authority'; import * as React from 'react'; import { useT } from '@/lib/i18n/client'; -import { cn } from '@/lib/utils/cn'; import { Button } from '../primitives/button'; import { Dialog } from './dialog'; -const confirmButtonVariants = cva('', { - variants: { - variant: { - default: '', - destructive: 'bg-red-600 hover:bg-red-700', - }, - }, - defaultVariants: { - variant: 'default', - }, -});Then simplify the confirm button:
<Button type="button" variant={variant === 'destructive' ? 'destructive' : 'primary'} onClick={(e) => { e.stopPropagation(); onConfirm(); }} disabled={isLoading} - className={cn(confirmButtonVariants({ variant }))} >services/platform/app/components/ui/primitives/icon-button.test.tsx (1)
26-35: 🧹 Nitpick | 🔵 TrivialConsider adding the
linkvariant to the test matrix.The stories for both
ButtonandIconButtonlistlinkas a valid variant option, but this test doesn't cover it. Iflinkis a supported variant forIconButton, add it here for completeness.it.each([ 'primary', 'destructive', 'success', 'secondary', 'ghost', + 'link', ] as const)('renders %s variant', (variant) => {services/platform/app/components/ui/primitives/icon-button.stories.tsx (1)
92-107: 🧹 Nitpick | 🔵 Trivial
linkvariant is listed in argTypes but missing fromAllVariants.The
argTypes.variant.optionsarray includeslink(line 53), but theAllVariantsstory only renders 5 of the 6 variants. Iflinkis valid forIconButton, add it here for documentation completeness; otherwise, remove it from the options list.services/platform/.storybook/main.js (1)
31-44:⚠️ Potential issue | 🟡 MinorLocal development: Add Node version constraint for
import.meta.dirname
import.meta.dirname(line 37) requires Node.js ≥ 21.2, which is available in CI (Node 22+). However, there's no.node-version,.nvmrc, orenginesconstraint inpackage.json, so local developers could run older Node versions and encounter an undefinedimport.meta.dirnameerror. Add a version constraint file (.node-versionwith22or higher) to enforce this locally.
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 110-113: Move the hardcoded Turbo environment variables out of the
single job and into a reusable location: either add TURBO_API, TURBO_TOKEN, and
TURBO_TEAM to the workflow-level env block so all jobs inherit them, or put them
into the existing/composite action used for Turbo setup (e.g., the "setup-turbo"
composite) and reference that action in jobs that need Turbo; update the job
that currently declares the env block to remove the duplicates and ensure it
still references TURBO_TOKEN from secrets.GITHUB_TOKEN if required.
In `@services/platform/.storybook/preview.tsx`:
- Around line 43-69: WithProviders calls createStoryRouter() directly in the
component body which recreates the router on every render and forces
RouterProvider to remount; stabilize the router by creating it once per story
(e.g., wrap createStoryRouter() in useMemo or useState initializer) and pass
that memoized router to RouterProvider so the router instance does not change
across re-renders; update the WithProviders function to use the memoized/mounted
router instance instead of calling createStoryRouter() directly.
In `@services/platform/app/components/theme/theme-provider.tsx`:
- Around line 22-24: Add a short JSDoc above the exported ThemeContext to
indicate it's intentionally exported for Storybook/test use only and not the
recommended API for production; mention that ThemeProvider (or useTheme())
should be used in app code and that direct context access is for
decorators/tests to avoid DOM/localStorage side effects. Reference the exported
symbol ThemeContext and the existing ThemeProvider/useTheme helpers in the
comment to make the intent clear.
In `@services/platform/app/components/ui/forms/radio-group.tsx`:
- Around line 127-132: The label always applies "cursor-pointer" which overrides
the disabled cursor on the inner RadioGroupPrimitive.Item; update the label
rendering in radio-group.tsx to conditionally apply the cursor class based on
the disabled prop (i.e., if props.disabled then use "cursor-not-allowed" else
"cursor-pointer") or use a Tailwind state variant such as
has-[:disabled]:cursor-not-allowed targeting the nested control; locate the
wrapping <label> that renders {radio} and {label} and change its className to
derive the cursor class from the component's disabled prop
(RadioGroupPrimitive.Item / radio variable) so disabled radios show the correct
cursor; leave the Biome noLabelWithoutControl lint as-is (false positive) since
the control is rendered inside the label.
In `@services/platform/app/components/ui/navigation/pagination.stories.tsx`:
- Around line 7-34: The currentPage calculation in PaginationWrapper uses
parseInt(search.page) which can produce NaN for malformed values; update the
assignment to parse the page with a radix (e.g., parseInt(search.page, 10)) and
fall back to initialPage when the result is NaN or when search.page is undefined
(e.g., const parsed = search.page ? parseInt(search.page, 10) : NaN; const
currentPage = Number.isNaN(parsed) ? initialPage : parsed), so Pagination always
receives a valid numeric currentPage.
In `@services/platform/Dockerfile`:
- Around line 126-128: Remove the TURBO_TELEMETRY_DISABLED environment variable
from the runtime image stage in the Dockerfile: it's a Turborepo build-time
setting and doesn't belong in Stage 5 alongside CADDY_CA_CERT_PATH and
DO_NOT_TRACK; either delete the TURBO_TELEMETRY_DISABLED line from the runtime
stage or, if turborepo is used in your build, move that env var to the builder
stage where turborepo runs.
In `@services/platform/package.json`:
- Around line 124-126: The package.json has inconsistent Storybook version
pinning: `@storybook/addon-docs` and `@storybook/addon-vitest` are using caret
ranges (^10.2.5) while other `@storybook/`* deps are pinned to exact 10.2.5;
update the versions for `@storybook/addon-docs` and `@storybook/addon-vitest` to the
exact "10.2.5" (matching the other `@storybook` packages) so all `@storybook/`*
entries use the same exact versioning.
| env: | ||
| TURBO_API: 'http://127.0.0.1:9080' | ||
| TURBO_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| TURBO_TEAM: 'tale' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting Turbo env vars to a reusable pattern if more Turbo-based jobs are added.
The TURBO_API, TURBO_TOKEN, and TURBO_TEAM env vars are hardcoded here. If future jobs also need Turbo caching, these could be centralized (e.g., workflow-level env block or within the setup-turbo composite action). This is fine for now with a single Turbo consumer job.
🤖 Prompt for AI Agents
In @.github/workflows/build.yml around lines 110 - 113, Move the hardcoded Turbo
environment variables out of the single job and into a reusable location: either
add TURBO_API, TURBO_TOKEN, and TURBO_TEAM to the workflow-level env block so
all jobs inherit them, or put them into the existing/composite action used for
Turbo setup (e.g., the "setup-turbo" composite) and reference that action in
jobs that need Turbo; update the job that currently declares the env block to
remove the duplicates and ensure it still references TURBO_TOKEN from
secrets.GITHUB_TOKEN if required.
| function WithProviders({ | ||
| Story, | ||
| context, | ||
| }: { | ||
| Story: Parameters<DecoratorFunction<ReactRenderer>>[0]; | ||
| context: Parameters<DecoratorFunction<ReactRenderer>>[1]; | ||
| }) { | ||
| const router = createStoryRouter(); | ||
| const currentTheme = context.globals.theme === 'dark' ? 'dark' : 'light'; | ||
|
|
||
| const themeValue = useMemo( | ||
| () => ({ | ||
| theme: currentTheme, | ||
| resolvedTheme: currentTheme, | ||
| setTheme: () => {}, | ||
| }), | ||
| [currentTheme], | ||
| ); | ||
|
|
||
| return ( | ||
| <ThemeContext.Provider value={themeValue}> | ||
| <I18nProvider> | ||
| <RouterProvider router={router} defaultComponent={() => <Story />} /> | ||
| </I18nProvider> | ||
| </ThemeContext.Provider> | ||
| ); | ||
| } |
There was a problem hiding this comment.
createStoryRouter() is called on every render — stabilize it.
createStoryRouter() at line 50 runs in the component body, creating a fresh router instance on each render. This causes the RouterProvider to fully remount its tree on every re-render, which can reset internal routing state and cause flicker.
Wrap it in useMemo (or useState initializer) so it's created once per story:
♻️ Proposed fix
function WithProviders({
Story,
context,
}: {
Story: Parameters<DecoratorFunction<ReactRenderer>>[0];
context: Parameters<DecoratorFunction<ReactRenderer>>[1];
}) {
- const router = createStoryRouter();
+ const [router] = useState(() => createStoryRouter());
const currentTheme = context.globals.theme === 'dark' ? 'dark' : 'light';🤖 Prompt for AI Agents
In `@services/platform/.storybook/preview.tsx` around lines 43 - 69, WithProviders
calls createStoryRouter() directly in the component body which recreates the
router on every render and forces RouterProvider to remount; stabilize the
router by creating it once per story (e.g., wrap createStoryRouter() in useMemo
or useState initializer) and pass that memoized router to RouterProvider so the
router instance does not change across re-renders; update the WithProviders
function to use the memoized/mounted router instance instead of calling
createStoryRouter() directly.
| export const ThemeContext = createContext<ThemeContextValue | undefined>( | ||
| undefined, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Exporting ThemeContext for Storybook integration — reasonable trade-off.
This widens the public API surface, but it's necessary for Storybook decorators that need to provide theme context without the full ThemeProvider (which has DOM/localStorage side effects). Production consumers should continue using useTheme().
Consider adding a brief JSDoc comment to signal that direct context usage is intended for test/Storybook environments:
📝 Suggested documentation
+/**
+ * Exported for use in Storybook decorators and test harnesses.
+ * Prefer `useTheme()` hook in application code.
+ */
export const ThemeContext = createContext<ThemeContextValue | undefined>(
undefined,
);📝 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.
| export const ThemeContext = createContext<ThemeContextValue | undefined>( | |
| undefined, | |
| ); | |
| /** | |
| * Exported for use in Storybook decorators and test harnesses. | |
| * Prefer `useTheme()` hook in application code. | |
| */ | |
| export const ThemeContext = createContext<ThemeContextValue | undefined>( | |
| undefined, | |
| ); |
🤖 Prompt for AI Agents
In `@services/platform/app/components/theme/theme-provider.tsx` around lines 22 -
24, Add a short JSDoc above the exported ThemeContext to indicate it's
intentionally exported for Storybook/test use only and not the recommended API
for production; mention that ThemeProvider (or useTheme()) should be used in app
code and that direct context access is for decorators/tests to avoid
DOM/localStorage side effects. Reference the exported symbol ThemeContext and
the existing ThemeProvider/useTheme helpers in the comment to make the intent
clear.
| <label className="flex cursor-pointer items-center gap-2"> | ||
| {radio} | ||
| <Label htmlFor={id} className="cursor-pointer font-normal"> | ||
| <span className="text-xs leading-none font-normal md:text-sm"> | ||
| {label} | ||
| </Label> | ||
| </div> | ||
| </span> | ||
| </label> |
There was a problem hiding this comment.
cursor-pointer on the label isn't suppressed when the radio item is disabled.
When disabled is passed through ...props, the inner RadioGroupPrimitive.Item gets disabled:cursor-not-allowed, but the wrapping <label> still unconditionally applies cursor-pointer, overriding the visual cue. Consider conditionally removing the pointer or using a Tailwind has-[:disabled]: variant.
Regarding the Biome noLabelWithoutControl lint error — this is a false positive. The radio control is rendered inside the <label> via the {radio} variable, which provides valid implicit label association.
Proposed fix
- <label className="flex cursor-pointer items-center gap-2">
+ <label className="flex cursor-pointer items-center gap-2 has-[:disabled]:cursor-not-allowed has-[:disabled]:opacity-50">📝 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.
| <label className="flex cursor-pointer items-center gap-2"> | |
| {radio} | |
| <Label htmlFor={id} className="cursor-pointer font-normal"> | |
| <span className="text-xs leading-none font-normal md:text-sm"> | |
| {label} | |
| </Label> | |
| </div> | |
| </span> | |
| </label> | |
| <label className="flex cursor-pointer items-center gap-2 has-[:disabled]:cursor-not-allowed has-[:disabled]:opacity-50"> | |
| {radio} | |
| <span className="text-xs leading-none font-normal md:text-sm"> | |
| {label} | |
| </span> | |
| </label> |
🧰 Tools
🪛 Biome (2.3.14)
[error] 127-132: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
🤖 Prompt for AI Agents
In `@services/platform/app/components/ui/forms/radio-group.tsx` around lines 127 -
132, The label always applies "cursor-pointer" which overrides the disabled
cursor on the inner RadioGroupPrimitive.Item; update the label rendering in
radio-group.tsx to conditionally apply the cursor class based on the disabled
prop (i.e., if props.disabled then use "cursor-not-allowed" else
"cursor-pointer") or use a Tailwind state variant such as
has-[:disabled]:cursor-not-allowed targeting the nested control; locate the
wrapping <label> that renders {radio} and {label} and change its className to
derive the cursor class from the component's disabled prop
(RadioGroupPrimitive.Item / radio variable) so disabled radios show the correct
cursor; leave the Biome noLabelWithoutControl lint as-is (false positive) since
the control is rendered inside the label.
| function PaginationWrapper({ | ||
| initialPage = 1, | ||
| total = 100, | ||
| pageSize = 10, | ||
| totalPages, | ||
| hasNextPage, | ||
| }: { | ||
| initialPage?: number; | ||
| total?: number; | ||
| pageSize?: number; | ||
| totalPages?: number; | ||
| hasNextPage?: boolean; | ||
| }) { | ||
| const search: Record<string, string | undefined> = useSearch({ | ||
| strict: false, | ||
| }); | ||
| const currentPage = search.page ? parseInt(search.page) : initialPage; | ||
|
|
||
| return ( | ||
| <Pagination | ||
| currentPage={currentPage} | ||
| total={total} | ||
| pageSize={pageSize} | ||
| totalPages={totalPages} | ||
| hasNextPage={hasNextPage} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clean wrapper pattern for bridging URL-based pagination with Storybook.
The PaginationWrapper nicely decouples the initialPage default from the URL-driven currentPage, making stories interactive within Storybook's router context.
One minor note: parseInt(search.page) on line 23 could yield NaN if the URL param is malformed. This is low-risk in a story context, but a simple fallback would make it defensive:
Optional hardening
- const currentPage = search.page ? parseInt(search.page) : initialPage;
+ const currentPage = search.page ? parseInt(search.page, 10) || initialPage : initialPage;🤖 Prompt for AI Agents
In `@services/platform/app/components/ui/navigation/pagination.stories.tsx` around
lines 7 - 34, The currentPage calculation in PaginationWrapper uses
parseInt(search.page) which can produce NaN for malformed values; update the
assignment to parse the page with a radix (e.g., parseInt(search.page, 10)) and
fall back to initialPage when the result is NaN or when search.page is undefined
(e.g., const parsed = search.page ? parseInt(search.page, 10) : NaN; const
currentPage = Number.isNaN(parsed) ? initialPage : parsed), so Pagination always
receives a valid numeric currentPage.
| CADDY_CA_CERT_PATH=/caddy-data/caddy/pki/authorities/local/root.crt \ | ||
| DO_NOT_TRACK=1 \ | ||
| TURBO_TELEMETRY_DISABLED=1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
TURBO_TELEMETRY_DISABLED is unnecessary in the runtime image.
DO_NOT_TRACK=1 is a well-known convention and is fine here. However, TURBO_TELEMETRY_DISABLED=1 targets Turborepo, which is a build-time tool not present in the runtime stage (Stage 5). This env var is harmless but adds noise; consider moving it to the builder stage if turbo is used there, or removing it from the Dockerfile entirely.
🤖 Prompt for AI Agents
In `@services/platform/Dockerfile` around lines 126 - 128, Remove the
TURBO_TELEMETRY_DISABLED environment variable from the runtime image stage in
the Dockerfile: it's a Turborepo build-time setting and doesn't belong in Stage
5 alongside CADDY_CA_CERT_PATH and DO_NOT_TRACK; either delete the
TURBO_TELEMETRY_DISABLED line from the runtime stage or, if turborepo is used in
your build, move that env var to the builder stage where turborepo runs.
| "@storybook/addon-docs": "^10.2.5", | ||
| "@storybook/addon-themes": "10.2.5", | ||
| "@storybook/manager-api": "8.6.15", | ||
| "@storybook/addon-vitest": "^10.2.5", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent version pinning for Storybook packages.
The newly added @storybook/addon-docs and @storybook/addon-vitest use caret ranges (^10.2.5), while all other @storybook/* packages (lines 123, 125, 127, 128, 151) are pinned to exact versions (10.2.5). This can cause version drift within the Storybook ecosystem, leading to subtle compatibility issues.
🔧 Proposed fix: pin to exact versions
- "@storybook/addon-docs": "^10.2.5",
+ "@storybook/addon-docs": "10.2.5",
"@storybook/addon-themes": "10.2.5",
- "@storybook/addon-vitest": "^10.2.5",
+ "@storybook/addon-vitest": "10.2.5",📝 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.
| "@storybook/addon-docs": "^10.2.5", | |
| "@storybook/addon-themes": "10.2.5", | |
| "@storybook/manager-api": "8.6.15", | |
| "@storybook/addon-vitest": "^10.2.5", | |
| "@storybook/addon-docs": "10.2.5", | |
| "@storybook/addon-themes": "10.2.5", | |
| "@storybook/addon-vitest": "10.2.5", |
🤖 Prompt for AI Agents
In `@services/platform/package.json` around lines 124 - 126, The package.json has
inconsistent Storybook version pinning: `@storybook/addon-docs` and
`@storybook/addon-vitest` are using caret ranges (^10.2.5) while other
`@storybook/`* deps are pinned to exact 10.2.5; update the versions for
`@storybook/addon-docs` and `@storybook/addon-vitest` to the exact "10.2.5"
(matching the other `@storybook` packages) so all `@storybook/`* entries use the
same exact versioning.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores