Skip to content

feat(mobile): Storybook + ember theme tooling (Sprint 1 / PR 1 of 7)#4874

Open
justincrich wants to merge 17 commits into
superset-sh:mainfrom
justincrich:chat-mobile-sprint-1-tooling
Open

feat(mobile): Storybook + ember theme tooling (Sprint 1 / PR 1 of 7)#4874
justincrich wants to merge 17 commits into
superset-sh:mainfrom
justincrich:chat-mobile-sprint-1-tooling

Conversation

@justincrich
Copy link
Copy Markdown
Contributor

@justincrich justincrich commented May 23, 2026

Summary

Foundation PR of the chat-mobile sprint-1 stack. Pure tooling: scaffolds Storybook RN, rewrites the mobile app's color tokens to the desktop ember palette (Path A), and lands the Storybook config tweaks needed to make stories prep without crashing.

Storybook scaffold (`apps/mobile/.rnstorybook/`)

  • `main.js` story globs, `preview.tsx` decorators, `index.tsx` UI shell
  • on-device controls + actions addons
  • Geist + Geist Mono fonts via `@expo-google-fonts/geist`

Ember theme tokens

  • `apps/mobile/global.css` and `apps/mobile/lib/theme.ts` rewritten to the ember warm palette (`#e07850` accent, warm-neutral surface ramp #151110 / #1a1716 / #201e1c / #2a2827)
  • `NAV_THEME` (expo-router) updated to mirror ember tokens
  • State palette (live / warning / danger / success / neutral) for chat-domain status indicators

Storybook config fixes

  • AsyncStorage adapter passed to `getStorybookUI`
  • Node `tty` module stubbed for Metro (Storybook 9 RN compatibility)
  • `screens/**/*.stories` excluded from the glob — they transitively import `useTheme` → `lib/theme.ts` → `expo-router/react-navigation` which crashes at prep time
  • Decorator wrapped in `` from `expo-router/react-navigation` (mitigation attempt) then re-doubled-down on the screens exclusion when wrap proved insufficient

Stack position — 1 of 5

This PR must merge first; PRs 2–5 build on it.

# Branch Status
PR 1 (this) `chat-mobile-sprint-1-tooling` foundation — Storybook + ember tokens
PR 2 `chat-mobile-sprint-1-ported` port vendor primitives + first-party app components to ember
PR 3 `chat-mobile-sprint-1-atoms` 10 chat-view atoms
PR 4 `chat-mobile-sprint-1-molecules` 19 chat-view molecules
PR 5 `chat-mobile-sprint-1-organisms` 10 chat-view organisms

Each downstream branch contains this PR's commits plus its own (cross-fork stacks can't set non-`main` bases). After this PR merges, downstream branches rebase onto fresh `main` and shrink to their phase-specific diff.

Test plan

  • `cd apps/mobile && bun storybook` — Storybook opens in Expo
  • No expo-router crash at prep time
  • Storybook UI shell renders with on-device controls + actions tabs
  • `bun run typecheck` passes (28/28)

Open in Stage

Summary by cubic

Added on-device Storybook for React Native, gated by EXPO_PUBLIC_STORYBOOK=true, to speed up mobile UI work. The app theme matches main (stock shadcn/ui zinc); token galleries and the HelloWorld scaffold were removed.

  • New Features

    • Storybook scaffold under apps/mobile/.rnstorybook; app boots Storybook when EXPO_PUBLIC_STORYBOOK=true.
    • Integrated @storybook/react-native via withStorybook in metro.config.js for env-gated builds.
    • Loaded Geist and Geist Mono via @expo-google-fonts/geist and @expo-google-fonts/geist-mono, with expo-splash-screen gating and error-safe hide.
  • Bug Fixes

    • Fixed persisted-selection crash by passing AsyncStorage to getStorybookUI.
    • Avoided expo-router context errors with a StorybookRouterProvider (sets PreviewRouteContext, LinkingContext, UnhandledLinkingContext) and removed a duplicate NavigationContainer so it passes SDK 56 checks.
    • Stubbed Node tty using a Storybook module mock; added a default Welcome story to prevent EmptyIndex crashes.
    • Ensured splash screen never stalls on font load errors.

Written for commit 4a82191. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Storybook integration for interactive component development and testing.
    • New npm scripts: storybook to launch the Storybook environment and storybook-generate for story generation.
  • Chores

    • Updated application configuration and build system to enable conditional Storybook environment loading.
    • Updated font handling dependencies.

Review Change Stack

Cherry-picked from f4f2a687b (originally on the deleted chat-mobile-ui-elements
branch). Adds Storybook 9 native sandbox env-gated on EXPO_PUBLIC_STORYBOOK,
Design System token stories (Colors/Typography/Spacing/Icons reading existing
global.css tokens via className), and a HelloWorld reference component.

Manifest updated with structured `constraints` block:
  - preserve_theme: lib/theme.ts, global.css, uniwind-env.d.ts, uniwind-types.d.ts
    are canonical — scaffold and later phases must not overwrite.
  - wireframes_are_reference_only: PRD ASCII wireframes describe structural
    intent only; use frontend-design skill for high-fidelity during build.

Gates: discover/target/equip/scaffold = passed on mobile-ios and mobile-android.
Next: /pixel-perfect:build --platform mobile-ios (or mobile-android).
Adds storybook 9 + addon-ondevice-actions/controls + @storybook/react-native
devDeps in apps/mobile to match the cherry-picked package.json. Also picks
up apps/desktop 1.10.3 -> 1.11.0 from main.
…Phase 0)

Execute the token migration described in plans/chat-mobile-plan/14-token-migration-audit.md.
Path A: keep flat shadcn key names (--color-*) for rn-reusables CLI compatibility; swap
values to desktop ember warm palette + add chat-domain extensions (state palette, domain
tokens, fonts, touch-target spacing).

Vendor react-native-reusables components in apps/mobile/components/ui/* are not touched —
they read tokens at runtime and cascade automatically against the new values, per the
"vendor libraries + style overrides only" rule.

- apps/mobile/global.css: rewrite under Tailwind 4 @theme + uniwind @variant. Warm-neutral
  ramp (#151110 background / #201e1c card / #2a2827 secondary in dark; #ffffff / warm-tinted
  light grays in light). Ember accent #e07850 (hsl(17 69% 60%)) as --color-primary in both
  themes. Add state palette (live/warning/danger/success/neutral × fg/bg) and chat domain
  tokens (streaming-cursor, tool-rule). Pre-compute oklch literals as hsl for RN safety;
  omit color-mix() hover/pressed (rn-reusables handles interaction via opacity/scale).
- apps/mobile/lib/theme.ts: mirror global.css key-for-key. NAV_THEME.primary now resolves
  to ember (was inverted-neutral); NAV_THEME.notification stays destructive per audit §4.
  Add stateXxx + streamingCursor / toolRule + fontBody / fontMono.
- apps/mobile/app/_layout.tsx: wire Geist + Geist Mono via @expo-google-fonts/geist with
  SplashScreen.preventAutoHideAsync gate. Storybook + production both wait for fonts.
- apps/mobile/.rnstorybook/stories/DesignSystem/Colors.stories.tsx: extend with State
  palette + Domain tokens sections.
- apps/mobile/.rnstorybook/stories/DesignSystem/Typography.stories.tsx: add Font families
  section demonstrating all 4 Geist weights + Geist Mono weights via fontFamily prop.
- apps/mobile/design/manifest.json: bump to v5.1.0. Vibe rewritten to ember. Narrow
  preserve_theme.paths to uniwind machinery only (global.css + lib/theme.ts no longer
  locked). Add vendor_components_immutable constraint (per the new rule). Add
  tokens_source pointer to designs/tokens/tokens.css. Add fonts tool spec to both
  platform entries.
- apps/mobile/package.json + bun.lock: add @expo-google-fonts/geist 0.4.2,
  @expo-google-fonts/geist-mono 0.4.2, expo-font ~56, expo-splash-screen ~56.

Verified: bun typecheck passes (exit 0), biome check passes on touched files.
Storybook v9 react-native does not auto-detect AsyncStorage when
`shouldPersistSelection: true` — it expects the consumer to pass a
`storage` adapter explicitly. Without one, the persistence layer attempts
to call `.getItem()` on `undefined` and throws:

  TypeError: Cannot read property 'getItem' of undefined

This appears on first launch when Storybook tries to read the last-selected
story.

Fix: pass `{ getItem: AsyncStorage.getItem, setItem: AsyncStorage.setItem }`
from `@react-native-async-storage/async-storage` (already a dep). Storybook
now persists story selection across launches without erroring.
Storybook 9.x's `instrumenter` (transitively via @storybook/addon-ondevice-
controls + @storybook/addon-ondevice-actions) pulls in `tinyrainbow`, which
requires Node's built-in `tty` module. Metro cannot bundle it, surfacing as:

    ERROR  Unable to resolve module tty from
    .../storybook@9.1.20/.../instrumenter/index.cjs

Fix: shim `tty` to an empty module via `resolver.resolveRequest`. Returning
`{ type: "empty" }` is Metro's built-in pattern for Node built-ins it does
not bundle (same trick used for `fs`, `path`, etc. in RN bundles).

Pre-existing infra issue surfaced during pixel-perfect Wave-1 atom
verification; unrelated to Wave-1 atom additions.
Several screen modules (OrganizationHeaderButton, AuthenticatedTabBar,
TabBarAccessory, OrgDropdown, MoreMenuScreen, SettingsScreen, etc.) call
`useRouter`/`useNavigation` hooks that require an active expo-router
NavigationContainer. Storybook 9's `addon-ondevice-controls` eagerly
evaluates each story's render function during `createPreparedStoryMapping`,
which throws "Couldn't find an UnhandledLinkingContext context" outside a
running navigator.

The affected stories already comment-acknowledge they're "not renderable
in Storybook isolation" — they exist mainly as documentation surfaces.
Commenting out the `../screens/**/*.stories.?(ts|tsx|js|jsx)` glob in
.rnstorybook/main.js stops Storybook from pulling them into the bundle.

To restore screen stories later, uncomment the glob AND add a nav-mock
decorator to preview.tsx that provides UnhandledLinkingContext + a stub
useRouter (or wrap the story tree in a real NavigationContainer with mock
state).

Pre-existing infra issue surfaced during pixel-perfect Wave-1 atom
verification; unrelated to Wave-1 atom additions.
Pre-existing screen modules (OrganizationHeaderButton, AuthenticatedTabBar,
TabBarAccessory, OrgDropdown, MoreMenuScreen, SettingsScreen, etc.) call
`useRouter`/`useNavigation` hooks. Storybook 9's `addon-ondevice-controls`
eagerly evaluates each story's render path during `createPreparedStoryMapping`,
throwing "Couldn't find an UnhandledLinkingContext context" outside a
running NavigationContainer.

Fix: wrap the preview decorator chain in `<NavigationContainer>` from
`expo-router/react-navigation`. That sets up `UnhandledLinkingContext`,
`LinkingContext`, `LocaleDirContext`, and the base navigation state — the
same contexts expo-router's `<ExpoRoot>` provides in the real app.

Using the expo-router sub-path (`expo-router/react-navigation`) instead of
`@react-navigation/native` directly so we ride on apps/mobile's already-
declared expo-router dep — avoids a phantom-dep on `@react-navigation/native`
(which is only present transitively in the bun store).

Reverts the prior commit `dedf3dd56` that hid screens stories from main.js.
Screens stories now render correctly in Storybook isolation.
… prep

Tried wrapping the storybook preview chain in `<NavigationContainer>` from
`expo-router/react-navigation` (commit 0e805a1). Did not resolve the
`UnhandledLinkingContext` error.

Root cause: Storybook 9 RN's `loadStory` (called inside
`createPreparedStoryMapping`) does eager module + render-fn evaluation
BEFORE preview decorators apply. The screen placeholder stories
transitively import `useTheme` → `lib/theme.ts` → `expo-router/react-navigation`,
which during prep calls `useLinking` → reads the default
`UnhandledLinkingContext` value's getter → throws "Couldn't find an
UnhandledLinkingContext context."

Decorators in preview.tsx wrap render-time only, not prep-time. So the
NavigationContainer wrapper is structurally unable to fix this chain.

Re-exclude `../screens/**/*.stories.?(ts|tsx)` (same as the earlier
dedf3dd commit). Keep the NavigationContainer in preview.tsx as defense
for any future story that DOES route through decorators and needs nav
context (e.g. a future composer molecule that uses `<Link>`).

Long-term restoration of screen stories requires decoupling them from
`lib/theme.ts` (mirror the pattern used in `components/ScrollFade/ScrollFade.tsx`)
or moving them under `expo-router/testing-library`'s `renderRouter`.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 47b54cc7-ed0e-4cb5-beae-1d4360452d78

📥 Commits

Reviewing files that changed from the base of the PR and between 52add93 and 4a82191.

📒 Files selected for processing (2)
  • apps/mobile/.rnstorybook/preview.tsx
  • apps/mobile/.rnstorybook/stories/Welcome.stories.tsx
✅ Files skipped from review due to trivial changes (1)
  • apps/mobile/.rnstorybook/stories/Welcome.stories.tsx

📝 Walkthrough

Walkthrough

This PR integrates React Native Storybook into the mobile app. It establishes navigation contexts for Storybook preview mode, configures story discovery and UI rendering, modifies the app root to conditionally load Storybook, integrates Storybook into the Metro bundler, and adds necessary npm scripts and dependencies.

Changes

Mobile Storybook Integration

Layer / File(s) Summary
Storybook navigation and routing contexts
apps/mobile/.rnstorybook/router/LinkingContext.ts, apps/mobile/.rnstorybook/router/UnhandledLinkingContext.ts, apps/mobile/.rnstorybook/StorybookRouterProvider.tsx
React Navigation linking and unhandled linking contexts configure Storybook-specific preview routing. StorybookRouterProvider wraps content in theme, linking contexts, and conditionally mounts NavigationContainer with storybook linking options and dark theme.
Storybook configuration and preview setup
apps/mobile/.rnstorybook/main.js, apps/mobile/.rnstorybook/preview.tsx, apps/mobile/.rnstorybook/index.tsx, apps/mobile/.rnstorybook/mocks/tty.js, apps/mobile/.rnstorybook/.gitignore
Main.js discovers stories from ./stories and ../components directories and enables on-device controls/actions addons. Preview.tsx applies global decorator wrapping stories with conditional fullscreen padding and PortalHost, plus control matchers for color and Date props. Index.tsx wires AsyncStorage-backed persisted selection and renders StorybookUIRoot wrapped in StorybookRouterProvider.
App root integration with Storybook conditional loading
apps/mobile/app/_layout.tsx
App component loads Geist fonts via useFonts, manages splash screen lifecycle, and conditionally renders StorybookRoot or RootLayout based on EXPO_PUBLIC_STORYBOOK environment variable.
Metro bundler and package configuration
apps/mobile/metro.config.js, apps/mobile/package.json
Integrates Storybook into Metro via withStorybook wrapper conditionally enabled by EXPO_PUBLIC_STORYBOOK. Adds storybook npm scripts, replaces @expo-google-fonts packages with expo-font, and adds Storybook dev dependencies at version ^9.
Welcome example story
apps/mobile/.rnstorybook/stories/Welcome.stories.tsx
Provides a simple Welcome story component with centered "Storybook is working" message and default story export.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • saddlepaddle

Poem

🐰 A mobile app gets Storybook's light,
Components dancing in preview sight!
From routing to build, all pieces aligned,
Design in isolation? Now it's defined!
✨ Stories flourish, the UI takes flight

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding Storybook scaffolding and theme tooling to the mobile app, with context about sprint position.
Description check ✅ Passed The description provides comprehensive detail: summary of changes, related issues, type of change (new feature), testing instructions, and additional context including stack position and commit notes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@stage-review
Copy link
Copy Markdown

stage-review Bot commented May 23, 2026

Ready to review this PR? Stage has broken it down into 6 individual chapters for you:

Title
1 Install Storybook and font dependencies
2 Configure Metro for Storybook integration
3 Scaffold Storybook on-device configuration
4 Implement Storybook navigation and routing shims
5 Wire Storybook and fonts into App entry
6 Other changes
Open in Stage

Chapters generated by Stage for commit 4a82191 on May 26, 2026 7:06pm UTC.

Unblock storybook RN prep for views that transitively import expo-router
by providing a self-contained StorybookRouterProvider wrapping the
preview-time linking contexts.

- Add StorybookRouterProvider that wraps PreviewRouteContext +
  LinkingContext + UnhandledLinkingContext so views (including AskUserSheet
  from REMED-009) can render in Storybook without crashing on
  "Couldn't find an UnhandledLinkingContext context."
- Update preview.tsx + index.tsx to wrap stories in
  StorybookRouterProvider; remove the prior "do not import nav modules"
  ban (now properly worked-around at runtime, not at module-load time).
- metro.config.js teaches metro to resolve the new router/ subpath +
  storybook config files in app bundling.

Follow-up to ae4494c (REMED-009 task commit).
@justincrich justincrich changed the title feat(mobile): Storybook + ember theme tooling (Sprint 1 / PR 1 of 5) feat(mobile): Storybook + ember theme tooling (Sprint 1 / PR 1 of 7) May 24, 2026
@justincrich justincrich marked this pull request as ready for review May 24, 2026 21:49
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 24, 2026

Greptile Summary

This PR lays the Storybook React Native scaffold for the mobile chat sprint and rewrites the mobile color tokens to the ember warm palette. The StorybookRouterProvider avoids NavigationContainer-related crashes at Storybook prep time by supplying stub linking contexts through both a Metro resolver alias (active only when EXPO_PUBLIC_STORYBOOK=true) and a React context provider layer that conditionally wraps stories in a real NavigationContainer.

  • Added .rnstorybook/ with main.js, preview.tsx, index.tsx, on-device Controls + Actions addons, and design-system stories (Colors, Typography, Spacing, Icons) plus a HelloWorld validation component.
  • Rewrote global.css and lib/theme.ts to the ember palette (hsl(17 69% 60%) accent, warm-neutral dark surfaces), extended THEME with a state palette and chat domain tokens, and updated NAV_THEME to match.
  • Gated the Storybook root behind EXPO_PUBLIC_STORYBOOK=true in _layout.tsx using a module-level require() that Metro dead-code-eliminates in production builds.

Confidence Score: 4/5

Safe to merge as a foundation PR — all changes are additive tooling and token rewrites with no modifications to production logic paths.

The Storybook scaffold and ember token migration are well-scoped. The Metro resolver aliases are gated behind EXPO_PUBLIC_STORYBOOK=true for the context overrides, and the dead-code-elimination guard in _layout.tsx keeps Storybook out of production bundles. The main concerns are contained to dev tooling: the StorybookRouterProvider imports from a private expo-router/build path that could silently break on an expo-router version bump, and the dark-theme hard-code means light-mode story variants are untestable in Storybook. Neither affects the production app.

apps/mobile/.rnstorybook/StorybookRouterProvider.tsx — private API import and hard-coded dark theme; apps/mobile/metro.config.js — unconditional tty stub

Important Files Changed

Filename Overview
apps/mobile/.rnstorybook/StorybookRouterProvider.tsx New provider wrapping stories in NavigationContainer + expo-router contexts; imports from a private build path and hard-codes NAV_THEME.dark for all stories
apps/mobile/metro.config.js Adds Storybook Metro integration, context aliases for expo-router linking contexts, and a tty stub; tty stub fires unconditionally across all builds
apps/mobile/app/_layout.tsx Font loading with splash-screen gate and dead-code-eliminatable Storybook root toggle; pattern is correct
apps/mobile/lib/theme.ts Full ember palette rewrite with extended tokens (state palette, domain chat tokens); NAV_THEME correctly typed as Record<light
apps/mobile/global.css CSS token rewrite to ember warm palette; mirrors theme.ts key-for-key as documented
apps/mobile/package.json New Storybook v9 dev deps and Geist font packages; expo-font and expo-splash-screen use semver ranges inconsistent with other exact-version expo packages
apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx Spacing and radius gallery story; contains a stale --radius: 0.5rem label that should now read 0.625rem

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["app/_layout.tsx\n(module eval)"] -->|EXPO_PUBLIC_STORYBOOK=true| B["require('.rnstorybook').default\n→ StorybookRoot"]
    A -->|default| C["RootLayout\n(production app)"]

    B --> D["StorybookRouterProvider\n(outer shell)"]
    D -->|no NavigationContainerRefContext| E["NavigationContainer\n(theme: NAV_THEME.dark)"]
    E --> F["ThemeProvider + LinkingContext\n+ UnhandledLinkingContext\n+ PreviewRouteContext"]
    F --> G["StorybookUIRoot\n(getStorybookUI + AsyncStorage)"]

    G -->|renders each story| H["preview.tsx decorator"]
    H --> I["StorybookRouterProvider\n(inner, per-story)"]
    I -->|navigationRef already set| J["ThemeProvider + Stub Contexts\n(no new NavigationContainer)"]
    J --> K["Story Component"]

    subgraph Metro["Metro resolver (Storybook mode only)"]
        M1["./LinkingContext from\n@react-navigation/native"] -->|alias| M2[".rnstorybook/router/LinkingContext.ts"]
        M3["./UnhandledLinkingContext from\nexpo-router internals"] -->|alias| M4[".rnstorybook/router/UnhandledLinkingContext.ts"]
        M5["tty"] -->|always| M6["empty module"]
    end
Loading
Prompt To Fix All With AI
Fix the following 5 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 5
apps/mobile/.rnstorybook/StorybookRouterProvider.tsx:30-54
**Dark theme hard-coded for all Storybook stories**

`NAV_THEME.dark` is passed to both `ThemeProvider` and the fallback `NavigationContainer`, so every story always renders with the dark navigation theme regardless of device appearance. Components that branch on `useTheme()` from react-navigation (e.g. `colorScheme === 'dark'`) will never receive the light variant in Storybook, making light-mode visual regressions invisible during development.

### Issue 2 of 5
apps/mobile/.rnstorybook/StorybookRouterProvider.tsx:1-5
**Import from internal build artifact path**

`expo-router/build/link/preview/PreviewRouteContext` is not a public export — it is a compiled build artefact. Any expo-router internal refactor (file rename, path change, removal of the preview context) silently breaks this import, causing all stories that render `StorybookRouterProvider` to crash. If the type is only needed for `satisfies`, a locally-defined minimal type would be safer than coupling to a private path.

### Issue 3 of 5
apps/mobile/metro.config.js:35-37
**`tty` stub is unconditional across all Metro builds**

The `tty → { type: "empty" }` resolution fires regardless of `EXPO_PUBLIC_STORYBOOK`, so any dependency that ships a React-Native-safe `tty` shim would be silently stubbed out in production bundles too. Wrapping the check in `process.env.EXPO_PUBLIC_STORYBOOK === "true"` (the same guard already used for the context aliases below) would limit the override to the build mode that actually needs it.

### Issue 4 of 5
apps/mobile/package.json:68
**Version range inconsistency for `expo-font` and `expo-splash-screen`**

All other `expo-*` packages in this file pin to exact versions (e.g. `"expo-file-system": "56.0.7"`), but the two newly-added packages use the `~56` range. A future `bun install` in a clean environment could resolve different patch versions than those used during development, which is inconsistent with the rest of the lockfile strategy.

```suggestion
		"expo-font": "56.0.0",
```

### Issue 5 of 5
apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx:25
**Stale `--radius` value in the label comment**

This PR changes `--radius` from `0.5rem` to `0.625rem` in both `global.css` and `lib/theme.ts`, but the label on the `rounded` row still references the old value. Anyone reading the story will get the wrong reference pixel size for the base radius token.

```suggestion
	{ token: "rounded (--radius: 0.625rem)", className: "rounded" },
```

Reviews (1): Last reviewed commit: "REMED-009: storybook router context (fol..." | Re-trigger Greptile

Comment on lines +30 to +54
const content = (
<ThemeProvider value={NAV_THEME.dark}>
<UnhandledLinkingContext.Provider
value={storybookUnhandledLinkingContext}
>
<LinkingContext.Provider value={storybookLinkingContext}>
<PreviewRouteContext.Provider value={storybookRoute}>
{children}
</PreviewRouteContext.Provider>
</LinkingContext.Provider>
</UnhandledLinkingContext.Provider>
</ThemeProvider>
);

if (navigationRef) {
return content;
}

return (
<NavigationContainer
fallback={null}
linking={storybookLinkingOptions}
theme={NAV_THEME.dark}
>
{content}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Dark theme hard-coded for all Storybook stories

NAV_THEME.dark is passed to both ThemeProvider and the fallback NavigationContainer, so every story always renders with the dark navigation theme regardless of device appearance. Components that branch on useTheme() from react-navigation (e.g. colorScheme === 'dark') will never receive the light variant in Storybook, making light-mode visual regressions invisible during development.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/.rnstorybook/StorybookRouterProvider.tsx
Line: 30-54

Comment:
**Dark theme hard-coded for all Storybook stories**

`NAV_THEME.dark` is passed to both `ThemeProvider` and the fallback `NavigationContainer`, so every story always renders with the dark navigation theme regardless of device appearance. Components that branch on `useTheme()` from react-navigation (e.g. `colorScheme === 'dark'`) will never receive the light variant in Storybook, making light-mode visual regressions invisible during development.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +1 to +5
import {
PreviewRouteContext,
type PreviewRouteContextType,
} from "expo-router/build/link/preview/PreviewRouteContext";
import {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Import from internal build artifact path

expo-router/build/link/preview/PreviewRouteContext is not a public export — it is a compiled build artefact. Any expo-router internal refactor (file rename, path change, removal of the preview context) silently breaks this import, causing all stories that render StorybookRouterProvider to crash. If the type is only needed for satisfies, a locally-defined minimal type would be safer than coupling to a private path.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/.rnstorybook/StorybookRouterProvider.tsx
Line: 1-5

Comment:
**Import from internal build artifact path**

`expo-router/build/link/preview/PreviewRouteContext` is not a public export — it is a compiled build artefact. Any expo-router internal refactor (file rename, path change, removal of the preview context) silently breaks this import, causing all stories that render `StorybookRouterProvider` to crash. If the type is only needed for `satisfies`, a locally-defined minimal type would be safer than coupling to a private path.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread apps/mobile/metro.config.js Outdated
Comment on lines +35 to +37

// Storybook prepares stories before React decorators/root wrappers are always
// in play. Expo Router's vendored React Navigation linking contexts use
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 tty stub is unconditional across all Metro builds

The tty → { type: "empty" } resolution fires regardless of EXPO_PUBLIC_STORYBOOK, so any dependency that ships a React-Native-safe tty shim would be silently stubbed out in production bundles too. Wrapping the check in process.env.EXPO_PUBLIC_STORYBOOK === "true" (the same guard already used for the context aliases below) would limit the override to the build mode that actually needs it.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/metro.config.js
Line: 35-37

Comment:
**`tty` stub is unconditional across all Metro builds**

The `tty → { type: "empty" }` resolution fires regardless of `EXPO_PUBLIC_STORYBOOK`, so any dependency that ships a React-Native-safe `tty` shim would be silently stubbed out in production bundles too. Wrapping the check in `process.env.EXPO_PUBLIC_STORYBOOK === "true"` (the same guard already used for the context aliases below) would limit the override to the build mode that actually needs it.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread apps/mobile/package.json Outdated
"expo-dev-client": "56.0.14",
"expo-device": "56.0.4",
"expo-file-system": "56.0.7",
"expo-font": "~56",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Version range inconsistency for expo-font and expo-splash-screen

All other expo-* packages in this file pin to exact versions (e.g. "expo-file-system": "56.0.7"), but the two newly-added packages use the ~56 range. A future bun install in a clean environment could resolve different patch versions than those used during development, which is inconsistent with the rest of the lockfile strategy.

Suggested change
"expo-font": "~56",
"expo-font": "56.0.0",
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/package.json
Line: 68

Comment:
**Version range inconsistency for `expo-font` and `expo-splash-screen`**

All other `expo-*` packages in this file pin to exact versions (e.g. `"expo-file-system": "56.0.7"`), but the two newly-added packages use the `~56` range. A future `bun install` in a clean environment could resolve different patch versions than those used during development, which is inconsistent with the rest of the lockfile strategy.

```suggestion
		"expo-font": "56.0.0",
```

How can I resolve this? If you propose a fix, please make it concise.

const RADIUS_STEPS = [
{ token: "rounded-none", className: "rounded-none" },
{ token: "rounded-sm", className: "rounded-sm" },
{ token: "rounded (--radius: 0.5rem)", className: "rounded" },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Stale --radius value in the label comment

This PR changes --radius from 0.5rem to 0.625rem in both global.css and lib/theme.ts, but the label on the rounded row still references the old value. Anyone reading the story will get the wrong reference pixel size for the base radius token.

Suggested change
{ token: "rounded (--radius: 0.5rem)", className: "rounded" },
{ token: "rounded (--radius: 0.625rem)", className: "rounded" },
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx
Line: 25

Comment:
**Stale `--radius` value in the label comment**

This PR changes `--radius` from `0.5rem` to `0.625rem` in both `global.css` and `lib/theme.ts`, but the label on the `rounded` row still references the old value. Anyone reading the story will get the wrong reference pixel size for the base radius token.

```suggestion
	{ token: "rounded (--radius: 0.625rem)", className: "rounded" },
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (3)
apps/mobile/.rnstorybook/router/LinkingContext.ts (1)

21-23: ⚡ Quick win

Remove unused LinkingContext export.

The LinkingContext created here is never imported or used. StorybookRouterProvider.tsx imports LinkingContext from expo-router/react-navigation and provides it with storybookLinkingContext as the value. This local context export is dead code.

🧹 Proposed cleanup
 export const storybookLinkingContext = {
   options: storybookLinkingOptions,
 };
-
-export const LinkingContext = React.createContext(storybookLinkingContext);
-
-LinkingContext.displayName = "LinkingContext";
🤖 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 `@apps/mobile/.rnstorybook/router/LinkingContext.ts` around lines 21 - 23,
Remove the dead local export by deleting the React.createContext call and its
displayName assignment for LinkingContext (the symbols `LinkingContext` and
`storybookLinkingContext` in this file); `StorybookRouterProvider.tsx` already
imports and provides `LinkingContext` from `expo-router/react-navigation`, so
eliminate the unused `LinkingContext` export and any references to it in this
file to clean up dead code.
apps/mobile/.rnstorybook/router/UnhandledLinkingContext.ts (1)

14-19: ⚡ Quick win

Remove unused UnhandledLinkingContext export.

Similar to LinkingContext.ts, this locally created context is never imported. StorybookRouterProvider.tsx imports UNSTABLE_UnhandledLinkingContext from expo-router/react-navigation and provides it with storybookUnhandledLinkingContext as the value. This local context export is dead code.

🧹 Proposed cleanup
 export const storybookUnhandledLinkingContext: StorybookUnhandledLinkingContextValue =
   {
     lastUnhandledLink: undefined,
     setLastUnhandledLink: () => {},
   };
-
-export const UnhandledLinkingContext =
-  React.createContext<StorybookUnhandledLinkingContextValue>(
-    storybookUnhandledLinkingContext,
-  );
-
-UnhandledLinkingContext.displayName = "UnhandledLinkingContext";
🤖 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 `@apps/mobile/.rnstorybook/router/UnhandledLinkingContext.ts` around lines 14 -
19, Remove the unused local context export UnhandledLinkingContext: delete its
declaration and displayName assignment in UnhandledLinkingContext.ts since
StorybookRouterProvider.tsx uses UNSTABLE_UnhandledLinkingContext from
expo-router/react-navigation and supplies storybookUnhandledLinkingContext
directly; ensure storybookUnhandledLinkingContext remains exported if used
elsewhere and run project-wide search for UnhandledLinkingContext to confirm no
other references remain.
apps/mobile/lib/theme.ts (1)

75-77: 💤 Low value

Consider aligning font token naming with CSS.

The CSS defines --font-sans and --font-mono, but the JS object uses fontBody and fontMono. While the values serve different purposes (CSS font stacks vs. RN-specific font names), the inconsistent naming could confuse maintainers. Consider renaming to fontSans and fontMono for clarity, or documenting why fontBody was chosen.

🤖 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 `@apps/mobile/lib/theme.ts` around lines 75 - 77, The theme object uses
fontBody and fontMono which diverge from the CSS tokens (--font-sans,
--font-mono); rename the JS token fontBody to fontSans (keep fontMono) in
apps/mobile/lib/theme.ts to align naming, then update all references/usages and
any TypeScript types/interfaces that expect fontBody (e.g., imports, Theme
types, components reading theme.fontBody) to use theme.fontSans instead so code
and CSS naming are consistent; if you prefer to keep fontBody, add a clear
comment explaining the mapping between fontBody -> --font-sans instead of
renaming.
🤖 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 `@apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx`:
- Line 25: The displayed token comment for rounded is outdated: update the token
string in Spacing.stories (the object with token: "rounded (--radius: 0.5rem)")
to reflect the actual value from global.css (change 0.5rem to 0.625rem) so the
token reads "rounded (--radius: 0.625rem) — locate the token object in
apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx and edit the
token property accordingly.

In `@apps/mobile/app/_layout.tsx`:
- Around line 34-50: useFonts can return an error and leave fontsLoaded false,
which stalls the splash and returns null; update the hook to capture the error
(e.g. const [fontsLoaded, fontsError] = useFonts(...)), change the useEffect to
call SplashScreen.hideAsync() when either fontsLoaded or fontsError is truthy,
and stop gating the render purely on fontsLoaded (render the app if fontsLoaded
|| fontsError); also log the fontsError (console.error or process logger) so
failures are visible; refer to useFonts, fontsLoaded, fontsError, and
SplashScreen.hideAsync in the _layout component.

In `@apps/mobile/global.css`:
- Line 38: Remove the unused CSS variable declaration --color-radius from the
mobile theme variants in apps/mobile/global.css (both the light variant where it
appears around the existing --radius declaration and the dark variant), leaving
the existing --radius token intact for uniwind/Tailwind usage; ensure you delete
both occurrences and run a quick search to confirm there are no
var(--color-radius) references before committing.

In `@apps/mobile/metro.config.js`:
- Around line 80-83: The global resolver override currently always returns an
empty module for moduleName === "tty"; restrict this behavior to Storybook runs
by gating the stub with a Storybook-only check (e.g., detect a STORYBOOK env var
or other existing Storybook indicator) before returning { type: "empty" } in
config.resolver.resolveRequest; add an isStorybook flag (derived from
process.env or process.argv) and only apply the tty shim when that flag is true
so normal app/resolution behavior remains unchanged outside Storybook.

In `@apps/mobile/package.json`:
- Line 68: Update the dependency entries for expo-font and expo-splash-screen
from tilde ranges (e.g., "expo-font": "~56") to exact version strings to match
the project's convention (same style as "expo" and "expo-router"); locate the
dependencies in package.json and replace the "~56" / "~56" entries for
"expo-font" and "expo-splash-screen" with the corresponding exact version
numbers used elsewhere in the file.

---

Nitpick comments:
In `@apps/mobile/.rnstorybook/router/LinkingContext.ts`:
- Around line 21-23: Remove the dead local export by deleting the
React.createContext call and its displayName assignment for LinkingContext (the
symbols `LinkingContext` and `storybookLinkingContext` in this file);
`StorybookRouterProvider.tsx` already imports and provides `LinkingContext` from
`expo-router/react-navigation`, so eliminate the unused `LinkingContext` export
and any references to it in this file to clean up dead code.

In `@apps/mobile/.rnstorybook/router/UnhandledLinkingContext.ts`:
- Around line 14-19: Remove the unused local context export
UnhandledLinkingContext: delete its declaration and displayName assignment in
UnhandledLinkingContext.ts since StorybookRouterProvider.tsx uses
UNSTABLE_UnhandledLinkingContext from expo-router/react-navigation and supplies
storybookUnhandledLinkingContext directly; ensure
storybookUnhandledLinkingContext remains exported if used elsewhere and run
project-wide search for UnhandledLinkingContext to confirm no other references
remain.

In `@apps/mobile/lib/theme.ts`:
- Around line 75-77: The theme object uses fontBody and fontMono which diverge
from the CSS tokens (--font-sans, --font-mono); rename the JS token fontBody to
fontSans (keep fontMono) in apps/mobile/lib/theme.ts to align naming, then
update all references/usages and any TypeScript types/interfaces that expect
fontBody (e.g., imports, Theme types, components reading theme.fontBody) to use
theme.fontSans instead so code and CSS naming are consistent; if you prefer to
keep fontBody, add a clear comment explaining the mapping between fontBody ->
--font-sans instead of renaming.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e0e37cf9-e4be-4d06-8737-b98ec1bff4ab

📥 Commits

Reviewing files that changed from the base of the PR and between c7883c7 and 2762ff6.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (20)
  • apps/mobile/.rnstorybook/.gitignore
  • apps/mobile/.rnstorybook/StorybookRouterProvider.tsx
  • apps/mobile/.rnstorybook/index.tsx
  • apps/mobile/.rnstorybook/main.js
  • apps/mobile/.rnstorybook/preview.tsx
  • apps/mobile/.rnstorybook/router/LinkingContext.ts
  • apps/mobile/.rnstorybook/router/UnhandledLinkingContext.ts
  • apps/mobile/.rnstorybook/stories/DesignSystem/Colors.stories.tsx
  • apps/mobile/.rnstorybook/stories/DesignSystem/Icons.stories.tsx
  • apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx
  • apps/mobile/.rnstorybook/stories/DesignSystem/Typography.stories.tsx
  • apps/mobile/app/_layout.tsx
  • apps/mobile/components/HelloWorld/HelloWorld.stories.tsx
  • apps/mobile/components/HelloWorld/HelloWorld.tsx
  • apps/mobile/components/HelloWorld/index.ts
  • apps/mobile/design/manifest.json
  • apps/mobile/global.css
  • apps/mobile/lib/theme.ts
  • apps/mobile/metro.config.js
  • apps/mobile/package.json

Comment thread apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx Outdated
Comment thread apps/mobile/app/_layout.tsx Outdated
Comment thread apps/mobile/global.css Outdated
Comment thread apps/mobile/metro.config.js Outdated
Comment on lines +80 to +83
config.resolver.resolveRequest = (ctx, moduleName, platform) => {
if (moduleName === "tty") {
return { type: "empty" };
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether `tty` is imported outside Storybook-related code.
# Expected: either no matches, or matches confined to Storybook/tooling paths.
rg -n -C2 'from\s+["'"'"']tty["'"'"']|require\(\s*["'"'"']tty["'"'"']\s*\)' -g '!**/node_modules/**'

Repository: superset-sh/superset

Length of output: 46


Scope the tty Metro resolver shim to Storybook mode only

The repo has no direct import/require('tty') usage outside node_modules (so this likely isn’t hiding app-level code today), but the resolveRequest override is global and can still suppress issues coming from transitive dependencies or future code. Gate the tty empty stub to Storybook to keep the behavior aligned with intent.

Proposed fix
 config.resolver.resolveRequest = (ctx, moduleName, platform) =&gt; {
-	if (moduleName === "tty") {
+	if (
+		process.env.EXPO_PUBLIC_STORYBOOK === "true" &amp;&amp;
+		moduleName === "tty"
+	) {
 		return { type: "empty" };
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
config.resolver.resolveRequest = (ctx, moduleName, platform) => {
if (moduleName === "tty") {
return { type: "empty" };
}
config.resolver.resolveRequest = (ctx, moduleName, platform) => {
if (
process.env.EXPO_PUBLIC_STORYBOOK === "true" &&
moduleName === "tty"
) {
return { type: "empty" };
}
🤖 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 `@apps/mobile/metro.config.js` around lines 80 - 83, The global resolver
override currently always returns an empty module for moduleName === "tty";
restrict this behavior to Storybook runs by gating the stub with a
Storybook-only check (e.g., detect a STORYBOOK env var or other existing
Storybook indicator) before returning { type: "empty" } in
config.resolver.resolveRequest; add an isStorybook flag (derived from
process.env or process.argv) and only apply the tty shim when that flag is true
so normal app/resolution behavior remains unchanged outside Storybook.

Comment thread apps/mobile/package.json Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 21 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/mobile/.rnstorybook/StorybookRouterProvider.tsx">

<violation number="1" location="apps/mobile/.rnstorybook/StorybookRouterProvider.tsx:4">
P2: Avoid importing from `expo-router/build/*` internals here; this is a private build-artifact path and can break on routine expo-router updates.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

import {
PreviewRouteContext,
type PreviewRouteContextType,
} from "expo-router/build/link/preview/PreviewRouteContext";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Avoid importing from expo-router/build/* internals here; this is a private build-artifact path and can break on routine expo-router updates.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/.rnstorybook/StorybookRouterProvider.tsx, line 4:

<comment>Avoid importing from `expo-router/build/*` internals here; this is a private build-artifact path and can break on routine expo-router updates.</comment>

<file context>
@@ -0,0 +1,57 @@
+import {
+	PreviewRouteContext,
+	type PreviewRouteContextType,
+} from "expo-router/build/link/preview/PreviewRouteContext";
+import {
+	LinkingContext,
</file context>

Comment thread apps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsx Outdated
@@ -0,0 +1,191 @@
import type { Meta, StoryObj } from "@storybook/react-native";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Works great! I'd say we should remove the Colors / Icons / Spacing etc. now that we've verified storybook is working I think it's lower leverage 🤔

Comment thread apps/mobile/design/manifest.json Outdated
@@ -0,0 +1,70 @@
{
"version": "5.1.0",
"created": "2026-05-21",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dropping this too now that we've implemented storybook is useful 👍🏼

Comment thread apps/mobile/lib/theme.ts Outdated
/**
* Superset mobile theme — ember warm palette (Path A, 2026-05-22).
*
* Mirrors `apps/mobile/global.css` key-for-key. Tailwind class consumers
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the ember looks nice, but it helps me a lot to see what is from the base design system and then what you've changed between PR's - would it be possible to use the exact colors from https://reactnativereusables.com/, and then later we can change them so the diff is easier to process?

Thanks in advance!

Comment thread apps/mobile/global.css Outdated

/* ============================================================
Superset mobile — ember warm palette (Path A, 2026-05-22)
Source of truth: designs/tokens/tokens.css at worktree root
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: We tend to be on team no comments, mind trimming these out of the change? comments like the one in apps/mobile/.rnstorybook/main.js are fine but I'd also recommend trimming them down to maybe a line or two

@@ -0,0 +1,48 @@
import type { Meta, StoryObj } from "@storybook/react-native";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

remove these

Comment thread apps/mobile/package.json Outdated
"expo-dev-client": "56.0.14",
"expo-device": "56.0.4",
"expo-file-system": "56.0.7",
"expo-font": "~56",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we started pinning versions

Comment thread apps/mobile/metro.config.js Outdated
// throwing default getters, so Storybook mode resolves those context modules to
// safe Storybook-owned defaults while the root still provides a real
// NavigationContainer.
const storybookContextAliases = {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we shouldn't keep this in the metro config ... put them in the preview provider:
https://storybook.js.org/docs/writing-stories/mocking-data-and-modules/mocking-providers

- Revert global.css and theme.ts to reactnativereusables default
  theme (stock shadcn/ui keys + values, no custom ember palette)
- Remove DesignSystem gallery stories (Colors, Icons, Spacing, Typography)
- Remove HelloWorld scaffold component
- Remove design/manifest.json (superseded by Storybook)
- Trim comments in global.css and main.js
- Pin expo-font and expo-splash-screen to exact versions
- Move tty stub from metro.config.js to Storybook module mock
- Handle useFonts error so splash screen never stalls
@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 26, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

Matches global.css and theme.ts exactly to origin/main — no custom
theme changes in this PR, only Storybook tooling additions.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
apps/mobile/lib/theme.ts (1)

7-50: ⚡ Quick win

Consider adding an explicit TypeScript type for the THEME object.

The THEME object currently relies on inferred typing, which can miss property name typos or structural changes. Defining an explicit type would improve type safety and provide better IntelliSense for downstream consumers.

♻️ Suggested type definition
+type ThemePalette = {
+	background: string;
+	foreground: string;
+	card: string;
+	cardForeground: string;
+	popover: string;
+	popoverForeground: string;
+	primary: string;
+	primaryForeground: string;
+	secondary: string;
+	secondaryForeground: string;
+	muted: string;
+	mutedForeground: string;
+	accent: string;
+	accentForeground: string;
+	destructive: string;
+	border: string;
+	input: string;
+	ring: string;
+	radius: string;
+};
+
-export const THEME = {
+export const THEME: Record<"light" | "dark", ThemePalette> = {
 	light: {
🤖 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 `@apps/mobile/lib/theme.ts` around lines 7 - 50, Add an explicit TypeScript
type for THEME to ensure property names and shapes are checked: define a
ThemeColor or ThemePalette interface listing each color/radius property as
string, define a Theme type mapping "light" and "dark" to that interface, and
annotate the exported THEME constant with that Theme type (and export the type
if other modules consume it). Update THEME to conform to the new type so any
missing/typoed keys on THEME (the exported THEME object) are flagged by the
compiler.
🤖 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 `@apps/mobile/.rnstorybook/preview.tsx`:
- Around line 12-17: The preview decorator is wrapping stories with a
NavigationContainer which will collide with the root-level
StorybookRouterProvider's NavigationContainer and cause nested navigation
errors; remove the NavigationContainer wrapper in preview.tsx and keep the
existing View (preserve className logic with cn("flex-1 bg-background",
!isFullscreen && "p-4")), Story, and PortalHost so stories rely on the root
StorybookRouterProvider for navigation context instead of creating a new
NavigationContainer.

In `@apps/mobile/global.css`:
- Around line 40-63: The dark-mode CSS token values under the `@variant` dark
block use incorrect HSL values that diverge from the react-native-reusables /
NativeWind defaults; update the tokens --color-background, --color-card,
--color-border, --color-secondary, --color-muted, --color-accent,
--color-destructive, --color-ring and --color-chart-1 through --color-chart-5 in
the `@variant` dark block to match the upstream defaults (replace your HSL values
with the corresponding default HSL/oklch values from RNR/NativenWind .dark:root)
so the mobile theme aligns with the expected palette. Ensure you update each
named variable (--color-background, --color-card, --color-border,
--color-secondary, --color-muted, --color-accent, --color-destructive,
--color-ring, --color-chart-1..5) exactly in the apps/mobile/global.css `@variant`
dark section.
- Around line 14-37: Update the `@variant` light token values in
apps/mobile/global.css to match the react-native-reusables default light tokens:
replace the current HSL values for the listed CSS variables under `@variant` light
(e.g., --color-foreground, --color-primary, --color-secondary,
--color-muted-foreground, --color-accent-foreground, --color-border,
--color-input, --color-ring) with the corresponding default HSL values (e.g.,
use hsl(240 10% 3.9%) for --color-foreground, hsl(240 5.9% 10%) for
--color-primary, hsl(240 4.8% 95.9%) for --color-secondary, hsl(240 3.8% 46.1%)
for --color-muted-foreground, hsl(240 5.9% 10%) for --color-accent-foreground,
hsl(240 5.9% 90%) for --color-border and --color-input, and the correct
--color-ring default) so the variables exactly match the react-native-reusables
defaults.

---

Nitpick comments:
In `@apps/mobile/lib/theme.ts`:
- Around line 7-50: Add an explicit TypeScript type for THEME to ensure property
names and shapes are checked: define a ThemeColor or ThemePalette interface
listing each color/radius property as string, define a Theme type mapping
"light" and "dark" to that interface, and annotate the exported THEME constant
with that Theme type (and export the type if other modules consume it). Update
THEME to conform to the new type so any missing/typoed keys on THEME (the
exported THEME object) are flagged by the compiler.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 53d06169-ab1a-47c5-b567-1b56129dec11

📥 Commits

Reviewing files that changed from the base of the PR and between 2762ff6 and 52add93.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • apps/mobile/.rnstorybook/main.js
  • apps/mobile/.rnstorybook/mocks/tty.js
  • apps/mobile/.rnstorybook/preview.tsx
  • apps/mobile/app/_layout.tsx
  • apps/mobile/global.css
  • apps/mobile/lib/theme.ts
  • apps/mobile/metro.config.js
  • apps/mobile/package.json
💤 Files with no reviewable changes (2)
  • apps/mobile/metro.config.js
  • apps/mobile/.rnstorybook/main.js
✅ Files skipped from review due to trivial changes (1)
  • apps/mobile/.rnstorybook/mocks/tty.js

Comment thread apps/mobile/.rnstorybook/preview.tsx Outdated
Comment thread apps/mobile/global.css Outdated
Comment thread apps/mobile/global.css Outdated
Comment on lines +40 to +63
@variant dark {
--color-radius: 0.5rem;
--color-background: hsl(240 10% 3.9%);
--color-background: hsl(0 0% 3.9%);
--color-foreground: hsl(0 0% 98%);
--color-card: hsl(240 10% 3.9%);
--color-card: hsl(0 0% 3.9%);
--color-card-foreground: hsl(0 0% 98%);
--color-popover: hsl(240 10% 3.9%);
--color-popover: hsl(0 0% 3.9%);
--color-popover-foreground: hsl(0 0% 98%);
--color-primary: hsl(0 0% 98%);
--color-primary-foreground: hsl(240 5.9% 10%);
--color-secondary: hsl(240 3.7% 15.9%);
--color-primary-foreground: hsl(0 0% 9%);
--color-secondary: hsl(0 0% 14.9%);
--color-secondary-foreground: hsl(0 0% 98%);
--color-muted: hsl(240 3.7% 15.9%);
--color-muted-foreground: hsl(240 5% 64.9%);
--color-accent: hsl(240 3.7% 15.9%);
--color-muted: hsl(0 0% 14.9%);
--color-muted-foreground: hsl(0 0% 63.9%);
--color-accent: hsl(0 0% 14.9%);
--color-accent-foreground: hsl(0 0% 98%);
--color-destructive: hsl(0 62.8% 30.6%);
--color-destructive-foreground: hsl(0 0% 98%);
--color-border: hsl(240 3.7% 15.9%);
--color-input: hsl(240 3.7% 15.9%);
--color-ring: hsl(240 4.9% 83.9%);
--color-destructive: hsl(0 70.9% 59.4%);
--color-border: hsl(0 0% 14.9%);
--color-input: hsl(0 0% 14.9%);
--color-ring: hsl(300 0% 45%);
--color-chart-1: hsl(220 70% 50%);
--color-chart-2: hsl(160 60% 45%);
--color-chart-3: hsl(30 80% 55%);
--color-chart-4: hsl(280 65% 60%);
--color-chart-5: hsl(340 75% 55%);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

What are the default theme color values in reactnativereusables for dark mode?

💡 Result:

Default dark mode theme color values in react-native-reusables (the NAV_THEME used for navigation) are: - background: hsl(240 10% 3.9%) [1] - border: hsl(240 3.7% 15.9%) [1] - card: hsl(240 10% 3.9%) [1] - notification: hsl(0 72% 51%) [1] - primary: hsl(0 0% 98%) [1] - text: hsl(0 0% 98%) [1] These match the library’s dark CSS variable values (e.g., --background: 240 10% 3.9%, --card: 240 10% 3.9%, --border: 240 3.7% 15.9%, --foreground/--text: 0 0% 98%) shown in community excerpts of the default theme setup. [2][3]

Citations:


🌐 Web query:

react-native-reusables default dark theme CSS variables --background --card --border --primary --secondary --muted --accent --destructive --ring --chart-1 dark

💡 Result:

The “default dark theme CSS variables” in React Native Reusables (RNR) / NativeWind shadcn-style theme are set under the .dark:root selector, and they define these variables: --background, --card, --border, --primary, --secondary, --muted, --accent, --destructive, --ring, and (additional ones like --foreground, etc.). [1][2] Dark theme variable values (HSL components, as stored in the CSS variables): --background: 240 10% 3.9% [1] --card: 240 10% 3.9% [1] --border: 240 3.7% 15.9% [1] --primary: 0 0% 98% [1] --secondary: 240 3.7% 15.9% [1] --muted: 240 3.7% 15.9% [1] --accent: 240 3.7% 15.9% [1] --destructive: 0 72% 51% [1] --ring: 240 4.9% 83.9% [1] Example snippet showing the dark selector and these variables:.dark:root { --background: 240 10% 3.9%; --card: 240 10% 3.9%; --border: 240 3.7% 15.9%; --primary: 0 0% 98%; --secondary: 240 3.7% 15.9%; --muted: 240 3.7% 15.9%; --accent: 240 3.7% 15.9%; --destructive: 0 72% 51%; --ring: 240 4.9% 83.9%; } [1] Note: in RNR/NativenWind usage, the .dark:root selector form is important for the theme variables to apply correctly (rather than .dark alone). [3][2]

Citations:


🌐 Web query:

react-native-reusables default dark theme chart-1 chart-2 chart-3 chart-4 chart-5 values

💡 Result:

react-native-reusables (via shadcn/ui chart theming) default dark theme chart token values are: chart-1: oklch(0.488 0.243 264.376) [1] chart-2: oklch(0.696 0.17 162.48) [1] chart-3: oklch(0.769 0.188 70.08) [1] chart-4: oklch(0.627 0.265 303.9) [1] chart-5: oklch(0.645 0.246 16.439) [1] These are set in the chart component’s CSS variables under the “.dark” selector for the chart tokens (--chart-1 through --chart-5). [1]

Citations:


Fix: dark-mode token colors don’t match react-native-reusables defaults (apps/mobile/global.css @variant dark, lines 40-63)

Several --color-* values differ from the default RNR/NativenWind .dark:root theme:

  • --color-background / --color-card: hsl(0 0% 3.9%) vs default hsl(240 10% 3.9%)
  • --color-border / --color-secondary / --color-muted / --color-accent: hsl(0 0% 14.9%) vs default hsl(240 3.7% 15.9%)
  • --color-destructive: hsl(0 70.9% 59.4%) vs default hsl(0 72% 51%)
  • --color-ring: hsl(300 0% 45%) vs default hsl(240 4.9% 83.9%)
  • --color-chart-1--color-chart-5: your HSL values don’t match the default chart tokens (which use oklch(...) values)
🧰 Tools
🪛 Biome (2.4.15)

[error] 41-41: Duplicate custom properties can lead to unexpected behavior and may override previous declarations unintentionally.

(lint/suspicious/noDuplicateCustomProperties)


[error] 41-41: Duplicate custom properties can lead to unexpected behavior and may override previous declarations unintentionally.

(lint/suspicious/noDuplicateCustomProperties)


[error] 41-41: Duplicate custom properties can lead to unexpected behavior and may override previous declarations unintentionally.

(lint/suspicious/noDuplicateCustomProperties)

🪛 Stylelint (17.12.0)

[error] 40-40: Unexpected unknown at-rule "@variant" (scss/at-rule-no-unknown)

(scss/at-rule-no-unknown)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mobile/global.css` around lines 40 - 63, The dark-mode CSS token values
under the `@variant` dark block use incorrect HSL values that diverge from the
react-native-reusables / NativeWind defaults; update the tokens
--color-background, --color-card, --color-border, --color-secondary,
--color-muted, --color-accent, --color-destructive, --color-ring and
--color-chart-1 through --color-chart-5 in the `@variant` dark block to match the
upstream defaults (replace your HSL values with the corresponding default
HSL/oklch values from RNR/NativenWind .dark:root) so the mobile theme aligns
with the expected palette. Ensure you update each named variable
(--color-background, --color-card, --color-border, --color-secondary,
--color-muted, --color-accent, --color-destructive, --color-ring,
--color-chart-1..5) exactly in the apps/mobile/global.css `@variant` dark section.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 17 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/mobile/.rnstorybook/StorybookRouterProvider.tsx">

<violation number="1" location="apps/mobile/.rnstorybook/StorybookRouterProvider.tsx:4">
P2: Avoid importing from `expo-router/build/*` internals here; this is a private build-artifact path and can break on routine expo-router updates.</violation>
</file>

<file name="apps/mobile/.rnstorybook/preview.tsx">

<violation number="1" location="apps/mobile/.rnstorybook/preview.tsx:12">
P2: Global NavigationContainer wrapper may cause nested navigation-container failures in stories that already include navigation roots.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread apps/mobile/.rnstorybook/preview.tsx Outdated
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…6 files)

Cascades the theme revert from superset-sh#4874 through atom + molecule components:
- state-live/warning/danger/success/neutral → green-600/amber-600/destructive/muted-foreground
- streaming-cursor → foreground
- tool-rule → muted-foreground / border

Per Token Migration Table in plans/pull-all-commented-feedback-polymorphic-yao.md
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…ganisms (Wave 5)

Theme migration (5 files):
- text-state-danger-fg → text-destructive
- text-state-warning-fg → text-amber-600
Per Token Migration Table cascade from superset-sh#4874.

Also drops the "4 sessions-list organisms" commit — those components belong
on the sessions-list PR (superset-sh#4912), not the views PR (superset-sh#4911).
@@ -0,0 +1 @@
module.exports = {};
Copy link
Copy Markdown
Contributor Author

@justincrich justincrich May 26, 2026

Choose a reason for hiding this comment

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

Placeholder for mocks to be

justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…mber-600

Cascades from superset-sh#4874 theme revert per Token Migration Table.
@justincrich justincrich requested a review from saddlepaddle May 26, 2026 18:37
- Remove duplicate NavigationContainer from preview.tsx decorator.
  StorybookRouterProvider already provides one. SDK 56 blocks direct
  @react-navigation/native imports inside the expo-router tree.
- Add Welcome.stories.tsx so Storybook has at least one story on the
  tooling branch (prevents EmptyIndexError crash).
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…6 files)

Cascades the theme revert from superset-sh#4874 through atom + molecule components:
- state-live/warning/danger/success/neutral → green-600/amber-600/destructive/muted-foreground
- streaming-cursor → foreground
- tool-rule → muted-foreground / border

Per Token Migration Table in plans/pull-all-commented-feedback-polymorphic-yao.md
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…ganisms (Wave 5)

Theme migration (5 files):
- text-state-danger-fg → text-destructive
- text-state-warning-fg → text-amber-600
Per Token Migration Table cascade from superset-sh#4874.

Also drops the "4 sessions-list organisms" commit — those components belong
on the sessions-list PR (superset-sh#4912), not the views PR (superset-sh#4911).
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…mber-600

Cascades from superset-sh#4874 theme revert per Token Migration Table.
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…s PR)

The Welcome story was added in PR superset-sh#4874 (tooling) to give Storybook at
least one entry — otherwise the empty stories index threw an
EmptyIndexError on the tooling branch in isolation.

This PR brings real component stories (8 first-party + 28 vendor
primitives), so the placeholder is no longer needed.
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…6 files)

Cascades the theme revert from superset-sh#4874 through atom + molecule components:
- state-live/warning/danger/success/neutral → green-600/amber-600/destructive/muted-foreground
- streaming-cursor → foreground
- tool-rule → muted-foreground / border

Per Token Migration Table in plans/pull-all-commented-feedback-polymorphic-yao.md
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…ganisms (Wave 5)

Theme migration (5 files):
- text-state-danger-fg → text-destructive
- text-state-warning-fg → text-amber-600
Per Token Migration Table cascade from superset-sh#4874.

Also drops the "4 sessions-list organisms" commit — those components belong
on the sessions-list PR (superset-sh#4912), not the views PR (superset-sh#4911).
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…mber-600

Cascades from superset-sh#4874 theme revert per Token Migration Table.
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…6 files)

Cascades the theme revert from superset-sh#4874 through atom + molecule components:
- state-live/warning/danger/success/neutral → green-600/amber-600/destructive/muted-foreground
- streaming-cursor → foreground
- tool-rule → muted-foreground / border

Per Token Migration Table in plans/pull-all-commented-feedback-polymorphic-yao.md
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…ganisms (Wave 5)

Theme migration (5 files):
- text-state-danger-fg → text-destructive
- text-state-warning-fg → text-amber-600
Per Token Migration Table cascade from superset-sh#4874.

Also drops the "4 sessions-list organisms" commit — those components belong
on the sessions-list PR (superset-sh#4912), not the views PR (superset-sh#4911).
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
…mber-600

Cascades from superset-sh#4874 theme revert per Token Migration Table.
justincrich added a commit to justincrich/superset that referenced this pull request May 26, 2026
Captures current Mobile Chat v2 state: Sprint 01 7-PR stack
(superset-sh#4874-superset-sh#4912) in review, Sprint 02 all 17 tasks shipped on
chat-mobile-sprint-2 (no PR yet, 53 WIP files), Sprints 03-07 planned
(53 tasks, ~108h). Maps every related PR -- in-stack, precursor, and
superseded -- to a sprint or sprint AC. Documents concrete handoffs the
Claude tooling can take: /kb-run-sprint per sprint, Sprint 02 WIP
triage, Sprint 01 stack landing, Sprint 06 parallel waves, ROADMAP
syncing, Maestro driving.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants