feat(mobile): wave-1 chat-view atoms — 10 atoms (Sprint 1 / PR 3 of 7)#4870
Open
justincrich wants to merge 25 commits into
Open
feat(mobile): wave-1 chat-view atoms — 10 atoms (Sprint 1 / PR 3 of 7)#4870justincrich wants to merge 25 commits into
justincrich wants to merge 25 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds on-device Storybook for the Expo app, wires navigation/preview contexts, wraps Metro, gates app root, and introduces multiple reusable RN components with accessibility/animation plus extensive stories and audits. Also updates theme usage in screen components and adds scripts/dependencies. ChangesReact Native Storybook enablement and UI components
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
This was referenced May 23, 2026
|
Ready to review this PR? Stage has broken it down into 6 individual chapters for you: 
Chapters generated by Stage for commit 36bbbb6 on May 26, 2026 8:38pm UTC. |
|
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. |
Contributor
Greptile SummaryIntroduces 10 chat-domain atom components (FabBase, HitTargetWrapper, ProgressDots, ScrollFade, ToastBase, and 5 refined existing atoms) built on the ember warm-palette theme, with a full Storybook RN v9 harness that works around expo-router's throwing context defaults via a Metro resolver alias and a navigation-container shim.
Confidence Score: 4/5Safe to merge after fixing the useFonts error handling in _layout.tsx; all atom components and the Storybook harness are well-implemented. The font-loading gate in app/_layout.tsx only calls SplashScreen.hideAsync() on success — if useFonts errors (even if rare with bundled assets), the app hangs permanently at the splash screen with no recovery. Everything else — the 10 atom components, theme tokens, and Storybook harness — is solid. apps/mobile/app/_layout.tsx — the useFonts error branch needs to also hide the splash screen.
|
| Filename | Overview |
|---|---|
| apps/mobile/app/_layout.tsx | Adds font loading gate with SplashScreen integration and Storybook root toggle; missing error handling for useFonts means a font-load failure permanently freezes the app at splash screen. |
| apps/mobile/metro.config.js | Adds Storybook + context-alias resolver with a tty empty-module stub that fires unconditionally in all builds instead of only Storybook mode; logic is otherwise well-structured. |
| apps/mobile/components/FabBase/FabBase.tsx | New 56/64pt FAB with pulsing live-ring, reduce-motion gating, loading state, and extended-pill variant; implementation looks correct. |
| apps/mobile/components/IconButton/IconButton.tsx | Icon-only button composing vendor Button; 5 variants × 4 sizes × 2 shapes with correct hitSlop compensation for sub-44pt sizes. |
| apps/mobile/components/ToastBase/ToastBase.tsx | 5-variant transient notification surface composing ToolStatusRule + HitTargetWrapper; accessibility role/liveRegion interaction was flagged in a prior review thread and is tracked. |
| apps/mobile/components/ScrollFade/ScrollFade.tsx | Gradient scroll-edge fade with animated hide/show; inlined surface colors (acknowledged duplication from global.css) may drift from tokens over time — flagged in prior thread. |
| apps/mobile/components/ProgressDots/ProgressDots.tsx | Staggered 3-dot loading indicator with reduce-motion gating; accessibilityState.busy hardcoded true even when paused was flagged in a prior review thread. |
| apps/mobile/global.css | Full ember palette token rewrite — light and dark variants with state palette additions and chat-domain tokens; well-structured and consistent with lib/theme.ts. |
| apps/mobile/.rnstorybook/StorybookRouterProvider.tsx | Navigation context shim for Storybook; navigation theme hardcoded to dark, which will affect any future light-mode story variants. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["app/_layout.tsx\n(App entry)"] -->|"EXPO_PUBLIC_STORYBOOK=true"| B["StorybookRoot\n(.rnstorybook/index.tsx)"]
A -->|"normal"| C["RootLayout\n(screens/RootLayout)"]
B --> D["StorybookRouterProvider\n(NAV_THEME.dark hardcoded)"]
D --> E["StorybookUIRoot\n(storybook.requires)"]
subgraph Atoms ["10 Atom Components"]
F["FabBase\n(56/64pt FAB + liveRing)"]
G["IconButton\n(5 variants x 4 sizes x 2 shapes)"]
H["Pill\n(6 variants, dismiss, interactive)"]
I["StatusDot\n(5 variants + pulse halo)"]
J["StreamingCursor\n(steps(2) blink)"]
K["HitTargetWrapper\n(44pt tap zone)"]
L["ProgressDots\n(staggered 3-dot)"]
M["ScrollFade\n(gradient edge)"]
N["ToastBase\n(5 variants + ToolStatusRule)"]
O["ToolStatusRule\n(3px status bar)"]
end
E -->|stories| Atoms
C -->|app usage| Atoms
N --> O
N --> K
subgraph Metro ["metro.config.js resolver"]
P["tty to empty\n(unconditional)"]
Q["expo-router LinkingContext\nto .rnstorybook stub\n(Storybook only)"]
end
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/mobile/app/_layout.tsx:35-51
**Font error ignored — app permanently frozen at splash screen**
`useFonts` returns `[loaded, error]`. When fonts fail to load for any reason (even unlikely for bundled assets), `fontsLoaded` stays `false` forever. Because `SplashScreen.hideAsync()` is only called when `fontsLoaded` is `true`, and the component returns `null` while `!fontsLoaded`, the app will be permanently stuck at the splash screen with no recovery path. The `fontError` result should also trigger `hideAsync` so the app renders (with system fallback fonts) rather than hanging.
### Issue 2 of 3
apps/mobile/metro.config.js:80-83
The `tty` empty-module stub fires unconditionally in every Metro build — including EAS production builds. Any production dependency that legitimately imports `tty` (rare, but possible in future packages) would silently receive an empty module rather than a hard error. Scoping the stub to Storybook mode prevents this from becoming a hidden production problem.
```suggestion
config.resolver.resolveRequest = (ctx, moduleName, platform) => {
if (moduleName === "tty" && process.env.EXPO_PUBLIC_STORYBOOK === "true") {
return { type: "empty" };
}
```
### Issue 3 of 3
apps/mobile/.rnstorybook/StorybookRouterProvider.tsx:30-34
The navigation theme is hardcoded to `NAV_THEME.dark`, so any story that renders navigation chrome (e.g., a future screen-level story) will always show the dark palette regardless of the story's intended light/dark variant. Passing `NAV_THEME.light` as an alternative or reading the active Storybook theme parameter would let light-mode atoms be previewed in a consistent context.
```suggestion
// TODO: read the active Storybook theme parameter when light-mode stories
// are introduced so navigation chrome matches the story palette.
const content = (
<ThemeProvider value={NAV_THEME.dark}>
<UnhandledLinkingContext.Provider
value={storybookUnhandledLinkingContext}
>
```
Reviews (2): Last reviewed commit: "feat(mobile): flip atoms gate → passed f..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
apps/mobile/global.css (1)
37-37: ⚡ Quick winDeduplicate
--color-radiusinto shared:root.Same value is declared in both variants; hoist it once to reduce override ambiguity and avoid duplicate-property lint noise.
Proposed fix
`@layer` theme { :root { + --color-radius: 0.625rem; + `@variant` light { - --color-radius: 0.625rem; @@ `@variant` dark { - --color-radius: 0.625rem;Also applies to: 85-85
🤖 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` at line 37, The CSS variable --color-radius is duplicated across theme variants; hoist a single declaration into the global :root selector (set --color-radius: 0.625rem) and remove the duplicate --color-radius entries from the variant selectors (the places that currently redeclare --color-radius) so only the :root defines it and variants inherit it.apps/mobile/app/_layout.tsx (1)
28-31: ⚡ Quick winType
StorybookRootexplicitly to preventanypropagation.
require("../.rnstorybook").defaultis untyped and leaksanyintoRoot, weakening TS checks.As per coding guidelines: `**/*.{ts,tsx}`: Avoid `any` type unless necessary; maintain type safety in TypeScript files.Proposed fix
-import { useEffect } from "react"; +import { type ComponentType, useEffect } from "react"; @@ -const StorybookRoot = +const StorybookRoot: ComponentType | null = process.env.EXPO_PUBLIC_STORYBOOK === "true" - ? require("../.rnstorybook").default + ? (require("../.rnstorybook").default as ComponentType) : null; @@ - const Root = StorybookRoot ?? RootLayout; + const Root: ComponentType = StorybookRoot ?? RootLayout;Also applies to: 51-52
🤖 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/app/_layout.tsx` around lines 28 - 31, The Storybook module is currently untyped (leaking any); explicitly type StorybookRoot (and the Root variable that uses it) as a React component type to prevent any propagation: declare StorybookRoot with a type like React.ComponentType<unknown> | null and cast the required default export to React.ComponentType<unknown> when assigning, and ensure you import the ComponentType/React type if missing; apply the same explicit typing to the other occurrence referenced (the Root usage around lines 51-52).apps/mobile/components/StatusDot/StatusDot.tsx (1)
121-124: ⚡ Quick winAccessibility: consider
accessiblefor consistent screen-reader discoverabilityReact Native doesn’t require
accessible={true}in addition toaccessibilityLabel/accessibilityRolefor aViewto be announced/focused;accessiblemainly marks it as a single discoverable accessibility element (and may not guarantee focus behavior by itself). Still, whenaccessibilityLabelis provided, settingaccessible={true}can make the dot’s announcement/discoverability more consistent.🤖 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/components/StatusDot/StatusDot.tsx` around lines 121 - 124, The StatusDot View currently sets accessibilityLabel and accessibilityRole but omits accessible, which can make screen-reader discoverability inconsistent; update the View in the StatusDot component to set accessible={true} whenever accessibilityLabel is provided (e.g., use accessible={!!accessibilityLabel}) so the element is treated as a single discoverable accessibility element alongside the existing accessibilityLabel and accessibilityRole props.apps/mobile/components/ui/alert.stories.tsx (1)
10-24: ⚡ Quick winPreserve literal icon keys to keep
iconprop type-safe.
ICON_MAPis typed asRecord<string, LucideIcon>, which widens keys tostring;iconthen accepts any string and can index toundefined.Suggested fix
-const ICON_MAP: Record<string, LucideIcon> = { +const ICON_MAP = { Info: InfoIcon, WifiOff, AlertTriangle, -}; +} satisfies Record<string, LucideIcon>;As per coding guidelines: "Avoid
anytype unless necessary; maintain type safety in TypeScript files".🤖 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/components/ui/alert.stories.tsx` around lines 10 - 24, ICON_MAP is currently typed as Record<string, LucideIcon>, which widens keys and makes the AlertShowcase prop icon unsafe; change ICON_MAP to preserve literal keys (e.g., declare the object with explicit literal keys and/or use a const assertion) so its type is the exact set of keys, then type the AlertShowcase icon prop as keyof typeof ICON_MAP (or use an explicit union of the literal keys) so only valid icons can be passed; update references to ICON_MAP and the AlertShowcase signature accordingly.
🤖 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/components/ScrollFade/ScrollFade.tsx`:
- Around line 32-41: The component ScrollFade currently spreads ...props last
which allows callers to override decorative-only interaction/accessibility
invariants; stop callers from overriding pointerEvents and accessibility-hidden
flags by either omitting those keys from the incoming props (remove
pointerEvents, accessible/accessibilityLabel, accessibilityElementsHidden,
importantForAccessibility from ...props) or by applying the fixed attributes
after the spread so the component-controlled values win; update the JSX where
...props is applied in ScrollFade (and the similar spread at the other
occurrence) to enforce pointerEvents="none" and the appropriate
accessibility-hidden props (e.g., accessibilityElementsHidden and
importantForAccessibility) as final attributes so callers cannot override them.
- Around line 92-94: Replace the literal "transparent" stop so the gradient
interpolates from the same base color as opaque to its alpha-zero variant:
derive an alpha-0 color from the existing opaque value (e.g., convert opaque
hex/#rgb or rgba string into an rgba(r,g,b,0) string) and use that derived
transparent color when building gradientColors (the variables direction, opaque
and gradientColors in ScrollFade/ScrollFade.tsx). Update the assignment of
gradientColors to use the derived transparent endpoint instead of the literal
"transparent" so hue is preserved during interpolation.
In `@apps/mobile/components/StreamingCursor/StreamingCursor.tsx`:
- Around line 87-95: The blinking animation computes half = resolvedDuration / 2
and then passes half - 1 directly into withTiming which can become negative for
tiny resolvedDuration (0–2ms); clamp that value before use. Create a local
segmentDuration = Math.max(0, half - 1) and replace the two occurrences of
withTiming(..., { duration: half - 1, ... }) with withTiming(..., { duration:
segmentDuration, ... }) so opacity.value (and the withSequence/withRepeat block)
never receives a negative duration.
In `@apps/mobile/components/ToastBase/ToastBase.tsx`:
- Line 136: The accessibilityLabel fallback currently uses accessibilityLabel ??
title ?? body which drops body when title exists; update the ToastBase component
so the fallback produces a combined label including both title and body (e.g.
join present pieces of title and body with a separator) instead of choosing only
title; change the accessibilityLabel expression in ToastBase (use
accessibilityLabel ?? [title, body].filter(Boolean).join(' - ') or equivalent)
so stacked toasts announce both title and message.
In `@apps/mobile/components/ui/checkbox.stories.tsx`:
- Line 16: The story initializes local state with const [checked, setChecked] =
useState(initialChecked) which doesn't update when the Storybook arg
initialChecked changes; add a useEffect in the story that watches initialChecked
and calls setChecked(initialChecked) to resync the local checked state whenever
the arg changes so the Checkbox story reflects control updates (keep references
to checked, setChecked, initialChecked and the story component).
In `@apps/mobile/components/ui/radio-group.stories.tsx`:
- Around line 15-17: The story initializes local state with
useState(initialValue) which won't update when the Storybook control changes;
update the local state whenever the initialValue prop changes by adding an
effect that calls setValue(initialValue) (or adopt a controlled pattern) so the
RadioGroup (value, setValue) reflects control updates; reference the useState
hook initializing value/setValue and the RadioGroup value/onValueChange props to
locate where to add the useEffect.
In `@apps/mobile/components/ui/switch.stories.tsx`:
- Around line 16-23: The story initializes checked with useState(initialChecked)
so updates to the Storybook control don't propagate; update the story to
synchronize local state with the control by observing initialChecked and calling
setChecked when it changes (e.g., add a useEffect that sets checked from
initialChecked), keeping the Switch props (checked,
onCheckedChange={setChecked}, disabled) unchanged so the control and component
stay in sync.
In `@apps/mobile/components/ui/textarea.stories.tsx`:
- Around line 17-26: The Storybook story initializes local state with
useState(initialValue) but never updates it when the story control changes; add
a useEffect that watches the initialValue prop and calls setValue(initialValue)
to sync the internal value with Storybook controls so Textarea (value, setValue)
reflects changes after mount.
In `@apps/mobile/components/ui/toggle.stories.tsx`:
- Around line 18-24: The story's local state (pressed via
useState(initialPressed)) isn't updated when the Storybook arg initialPressed
changes; update the Toggle story to keep pressed in sync by watching
initialPressed and calling setPressed when it changes (e.g., add a useEffect
that updates pressed from initialPressed), ensuring the controlled Toggle
(pressed prop and onPressedChange={setPressed}) reflects arg updates.
---
Nitpick comments:
In `@apps/mobile/app/_layout.tsx`:
- Around line 28-31: The Storybook module is currently untyped (leaking any);
explicitly type StorybookRoot (and the Root variable that uses it) as a React
component type to prevent any propagation: declare StorybookRoot with a type
like React.ComponentType<unknown> | null and cast the required default export to
React.ComponentType<unknown> when assigning, and ensure you import the
ComponentType/React type if missing; apply the same explicit typing to the other
occurrence referenced (the Root usage around lines 51-52).
In `@apps/mobile/components/StatusDot/StatusDot.tsx`:
- Around line 121-124: The StatusDot View currently sets accessibilityLabel and
accessibilityRole but omits accessible, which can make screen-reader
discoverability inconsistent; update the View in the StatusDot component to set
accessible={true} whenever accessibilityLabel is provided (e.g., use
accessible={!!accessibilityLabel}) so the element is treated as a single
discoverable accessibility element alongside the existing accessibilityLabel and
accessibilityRole props.
In `@apps/mobile/components/ui/alert.stories.tsx`:
- Around line 10-24: ICON_MAP is currently typed as Record<string, LucideIcon>,
which widens keys and makes the AlertShowcase prop icon unsafe; change ICON_MAP
to preserve literal keys (e.g., declare the object with explicit literal keys
and/or use a const assertion) so its type is the exact set of keys, then type
the AlertShowcase icon prop as keyof typeof ICON_MAP (or use an explicit union
of the literal keys) so only valid icons can be passed; update references to
ICON_MAP and the AlertShowcase signature accordingly.
In `@apps/mobile/global.css`:
- Line 37: The CSS variable --color-radius is duplicated across theme variants;
hoist a single declaration into the global :root selector (set --color-radius:
0.625rem) and remove the duplicate --color-radius entries from the variant
selectors (the places that currently redeclare --color-radius) so only the :root
defines it and variants inherit it.
🪄 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: 0c55a270-0b6b-41e7-bc89-6544bffb1d9a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (90)
apps/mobile/.rnstorybook/.gitignoreapps/mobile/.rnstorybook/index.tsxapps/mobile/.rnstorybook/main.jsapps/mobile/.rnstorybook/preview.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Colors.stories.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Icons.stories.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Typography.stories.tsxapps/mobile/app/_layout.tsxapps/mobile/components/FabBase/FabBase.stories.tsxapps/mobile/components/FabBase/FabBase.tsxapps/mobile/components/FabBase/index.tsapps/mobile/components/HelloWorld/HelloWorld.stories.tsxapps/mobile/components/HelloWorld/HelloWorld.tsxapps/mobile/components/HelloWorld/index.tsapps/mobile/components/HitTargetWrapper/HitTargetWrapper.stories.tsxapps/mobile/components/HitTargetWrapper/HitTargetWrapper.tsxapps/mobile/components/HitTargetWrapper/index.tsapps/mobile/components/IconButton/IconButton.stories.tsxapps/mobile/components/IconButton/IconButton.tsxapps/mobile/components/IconButton/index.tsapps/mobile/components/Pill/Pill.stories.tsxapps/mobile/components/Pill/Pill.tsxapps/mobile/components/Pill/index.tsapps/mobile/components/ProgressDots/ProgressDots.stories.tsxapps/mobile/components/ProgressDots/ProgressDots.tsxapps/mobile/components/ProgressDots/index.tsapps/mobile/components/ScrollFade/ScrollFade.stories.tsxapps/mobile/components/ScrollFade/ScrollFade.tsxapps/mobile/components/ScrollFade/index.tsapps/mobile/components/StatusDot/StatusDot.stories.tsxapps/mobile/components/StatusDot/StatusDot.tsxapps/mobile/components/StatusDot/index.tsapps/mobile/components/StreamingCursor/StreamingCursor.stories.tsxapps/mobile/components/StreamingCursor/StreamingCursor.tsxapps/mobile/components/StreamingCursor/index.tsapps/mobile/components/ToastBase/ToastBase.stories.tsxapps/mobile/components/ToastBase/ToastBase.tsxapps/mobile/components/ToastBase/index.tsapps/mobile/components/ToolStatusRule/ToolStatusRule.stories.tsxapps/mobile/components/ToolStatusRule/ToolStatusRule.tsxapps/mobile/components/ToolStatusRule/index.tsapps/mobile/components/ui/AUDIT.mdapps/mobile/components/ui/accordion.stories.tsxapps/mobile/components/ui/alert-dialog.stories.tsxapps/mobile/components/ui/alert.stories.tsxapps/mobile/components/ui/aspect-ratio.stories.tsxapps/mobile/components/ui/avatar.stories.tsxapps/mobile/components/ui/badge.stories.tsxapps/mobile/components/ui/button.stories.tsxapps/mobile/components/ui/card.stories.tsxapps/mobile/components/ui/checkbox.stories.tsxapps/mobile/components/ui/collapsible.stories.tsxapps/mobile/components/ui/context-menu.stories.tsxapps/mobile/components/ui/dialog.stories.tsxapps/mobile/components/ui/dropdown-menu.stories.tsxapps/mobile/components/ui/hover-card.stories.tsxapps/mobile/components/ui/icon.stories.tsxapps/mobile/components/ui/input.stories.tsxapps/mobile/components/ui/label.stories.tsxapps/mobile/components/ui/menubar.stories.tsxapps/mobile/components/ui/popover.stories.tsxapps/mobile/components/ui/progress.stories.tsxapps/mobile/components/ui/radio-group.stories.tsxapps/mobile/components/ui/select.stories.tsxapps/mobile/components/ui/separator.stories.tsxapps/mobile/components/ui/skeleton.stories.tsxapps/mobile/components/ui/switch.stories.tsxapps/mobile/components/ui/tabs.stories.tsxapps/mobile/components/ui/text.stories.tsxapps/mobile/components/ui/textarea.stories.tsxapps/mobile/components/ui/toggle-group.stories.tsxapps/mobile/components/ui/toggle.stories.tsxapps/mobile/components/ui/tooltip.stories.tsxapps/mobile/design/manifest.jsonapps/mobile/global.cssapps/mobile/lib/theme.tsapps/mobile/metro.config.jsapps/mobile/package.jsonapps/mobile/screens/(auth)/sign-in/components/DevSignInButton/DevSignInButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/OrganizationSwitcherSheet.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.stories.tsxapps/mobile/screens/(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.stories.tsxapps/mobile/screens/(authenticated)/components/OrgDropdown/OrgDropdown.stories.tsxapps/mobile/screens/(authenticated)/components/TabBarAccessory/TabBarAccessory.stories.tsxapps/mobile/screens/AUDIT.md
Comment on lines
+92
to
+94
| const gradientColors: [string, string] = | ||
| direction === "top" ? [opaque, "transparent"] : ["transparent", opaque]; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In expo-linear-gradient (React Native), does using the literal "transparent" color interpolate from transparent black, and is it recommended to use the same base color with alpha 0 for gradient fades?
💡 Result:
Yes: in React Native/Expo gradient color parsing, the keyword color "transparent" behaves like rgba(0,0,0,0) (i.e., transparent black). That means when you interpolate between a non-transparent color and "transparent", the RGB channels interpolate toward black as alpha approaches 0, which can make the midpoint look darker/gray rather than preserving the original hue. [1][2] Is it recommended to use the same base color with alpha 0 for gradient fades? Yes—if you want a fade that keeps the original color’s hue while only reducing alpha, use the equivalent rgba form for “transparent” that matches your intended RGB channels (e.g., for a white-to-transparent fade use rgba(255,255,255,0) rather than the keyword "transparent"). This matches the common pitfall described for gradients: “transparent is the same as black with zero opacity,” so a “white → transparent” gradient will pass through invisible black/gray intermediates unless you specify a transparent color with the same RGB channels. [1][2] Practical example for a white fade: Use colors like ["rgba(255,255,255,1)", "rgba(255,255,255,0)"] (or the reverse depending on direction) instead of mixing with the literal "transparent". [1][2] Note: Expo’s LinearGradient API accepts an array of color stops via the colors prop; the docs don’t specifically describe how the "transparent" keyword is expanded, but they do show it being used as a color stop (e.g., ['rgba(0,0,0,0.8)', 'transparent']). [3][4]
Citations:
- 1: https://stackoverflow.com/questions/48533133/expo-linear-gradient-transparent-is-showing-up-blackish
- 2: Transparent gradient. react-native-linear-gradient/react-native-linear-gradient#358
- 3: https://docs.expo.dev/versions/unversioned/sdk/linear-gradient
- 4: https://docs.expo.dev/versions/latest/sdk/linear-gradient/
Use alpha-matched color stops instead of literal "transparent".
File: apps/mobile/components/ScrollFade/ScrollFade.tsx
Lines: 92-94
const gradientColors: [string, string] =
direction === "top" ? [opaque, "transparent"] : ["transparent", opaque];In expo-linear-gradient / React Native, the "transparent" keyword is effectively rgba(0,0,0,0), so interpolating toward black can introduce a dark/gray tint instead of preserving the original hue. Use the same base RGB as opaque with alpha 0 (e.g., an rgba(..., 0) stop derived from opaque) for the transparent endpoint.
🤖 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/components/ScrollFade/ScrollFade.tsx` around lines 92 - 94,
Replace the literal "transparent" stop so the gradient interpolates from the
same base color as opaque to its alpha-zero variant: derive an alpha-0 color
from the existing opaque value (e.g., convert opaque hex/#rgb or rgba string
into an rgba(r,g,b,0) string) and use that derived transparent color when
building gradientColors (the variables direction, opaque and gradientColors in
ScrollFade/ScrollFade.tsx). Update the assignment of gradientColors to use the
derived transparent endpoint instead of the literal "transparent" so hue is
preserved during interpolation.
| <View | ||
| accessibilityRole="alert" | ||
| accessibilityLiveRegion="polite" | ||
| accessibilityLabel={accessibilityLabel ?? title ?? body} |
Contributor
There was a problem hiding this comment.
Default alert label drops body text when title is present.
The current fallback uses title ?? body, so stacked toasts may announce only the title and skip the main message.
Suggested fix
- accessibilityLabel={accessibilityLabel ?? title ?? body}
+ accessibilityLabel={
+ accessibilityLabel ?? (title ? `${title}. ${body}` : body)
+ }📝 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
| accessibilityLabel={accessibilityLabel ?? title ?? body} | |
| accessibilityLabel={ | |
| accessibilityLabel ?? (title ? `${title}. ${body}` : body) | |
| } |
🤖 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/components/ToastBase/ToastBase.tsx` at line 136, The
accessibilityLabel fallback currently uses accessibilityLabel ?? title ?? body
which drops body when title exists; update the ToastBase component so the
fallback produces a combined label including both title and body (e.g. join
present pieces of title and body with a separator) instead of choosing only
title; change the accessibilityLabel expression in ToastBase (use
accessibilityLabel ?? [title, body].filter(Boolean).join(' - ') or equivalent)
so stacked toasts announce both title and message.
| disabled: boolean; | ||
| label: string; | ||
| }) { | ||
| const [checked, setChecked] = useState(initialChecked); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print with line numbers around the referenced line.
ls -la apps/mobile/components/ui/checkbox.stories.tsx
nl -ba apps/mobile/components/ui/checkbox.stories.tsx | sed -n '1,120p'Repository: superset-sh/superset
Length of output: 221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n apps/mobile/components/ui/checkbox.stories.tsx | sed -n '1,140p'Repository: superset-sh/superset
Length of output: 1909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n apps/mobile/components/ui/checkbox.stories.tsx | sed -n '1,140p'
echo "----"
rg -n "initialChecked|useState\\(|setChecked\\(|args\\.|checked" apps/mobile/components/ui/checkbox.stories.tsxRepository: superset-sh/superset
Length of output: 2228
Sync checkbox checked state when initialChecked args change.
Line 16 initializes local state from initialChecked only once; changing the Storybook initialChecked control after mount won’t update the checkbox.
Proposed fix
-import { useState } from "react";
+import { useEffect, useState } from "react";
@@
const [checked, setChecked] = useState(initialChecked);
+ useEffect(() => {
+ setChecked(initialChecked);
+ }, [initialChecked]);📝 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
| const [checked, setChecked] = useState(initialChecked); | |
| import { useEffect, useState } from "react"; | |
| // ... (other imports and meta setup) | |
| export const CheckboxShowcase: Story = (args: any) => { | |
| const { initialChecked } = args; | |
| const [checked, setChecked] = useState(initialChecked); | |
| useEffect(() => { | |
| setChecked(initialChecked); | |
| }, [initialChecked]); | |
| // ... (rest of component) | |
| } |
🤖 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/components/ui/checkbox.stories.tsx` at line 16, The story
initializes local state with const [checked, setChecked] =
useState(initialChecked) which doesn't update when the Storybook arg
initialChecked changes; add a useEffect in the story that watches initialChecked
and calls setChecked(initialChecked) to resync the local checked state whenever
the arg changes so the Checkbox story reflects control updates (keep references
to checked, setChecked, initialChecked and the story component).
Comment on lines
+15
to
+17
| const [value, setValue] = useState(initialValue); | ||
| return ( | ||
| <RadioGroup value={value} onValueChange={setValue} className="gap-3"> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/radio-group.stories.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,120p'Repository: superset-sh/superset
Length of output: 187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/radio-group.stories.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "== File: $FILE =="
cat -n "$FILE" | sed -n '1,160p'Repository: superset-sh/superset
Length of output: 2716
Sync local selected value with initialValue Storybook control changes.
useState(initialValue) only uses the prop once; when the initialValue control changes, RadioGroup stays bound to the stale value state.
Suggested fix
-import { useState } from "react";
+import { useEffect, useState } from "react";
@@
const [value, setValue] = useState(initialValue);
+ useEffect(() => {
+ setValue(initialValue);
+ }, [initialValue]);
return (
<RadioGroup value={value} onValueChange={setValue} className="gap-3">📝 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
| const [value, setValue] = useState(initialValue); | |
| return ( | |
| <RadioGroup value={value} onValueChange={setValue} className="gap-3"> | |
| import { useEffect, useState } from "react"; | |
| // ... other imports ... | |
| export const RadioGroupShowcase = ({ options, initialValue }) => { | |
| const [value, setValue] = useState(initialValue); | |
| useEffect(() => { | |
| setValue(initialValue); | |
| }, [initialValue]); | |
| return ( | |
| <RadioGroup value={value} onValueChange={setValue} className="gap-3"> | |
| {/* ... rest of component */} | |
| </RadioGroup> | |
| ); | |
| }; |
🤖 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/components/ui/radio-group.stories.tsx` around lines 15 - 17, The
story initializes local state with useState(initialValue) which won't update
when the Storybook control changes; update the local state whenever the
initialValue prop changes by adding an effect that calls setValue(initialValue)
(or adopt a controlled pattern) so the RadioGroup (value, setValue) reflects
control updates; reference the useState hook initializing value/setValue and the
RadioGroup value/onValueChange props to locate where to add the useEffect.
Comment on lines
+16
to
+23
| const [checked, setChecked] = useState(initialChecked); | ||
| return ( | ||
| <View className="flex-row items-center gap-3"> | ||
| <Switch | ||
| checked={checked} | ||
| onCheckedChange={setChecked} | ||
| disabled={disabled} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/switch.stories.tsx"
echo "== File exists =="
ls -la "$FILE"
echo
echo "== Lines 1-120 =="
nl -ba "$FILE" | sed -n '1,120p'
echo
echo "== Search for SwitchShowcase and initialChecked usage =="
rg -n "SwitchShowcase|initialChecked|useState" "$FILE"Repository: superset-sh/superset
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/switch.stories.tsx"
echo "== Lines 1-200 =="
awk '{printf "%4d\t%s\n", NR, $0}' "$FILE" | sed -n '1,200p'
echo
echo "== Search for SwitchShowcase/initialChecked/useState =="
rg -n "SwitchShowcase|initialChecked|useState" "$FILE" || true
echo
echo "== Search for args/controls wiring =="
rg -n "argTypes|args|controls|Meta|StoryObj|render" "$FILE" || trueRepository: superset-sh/superset
Length of output: 2990
Sync Storybook initialChecked control with Switch state
checked is initialized from initialChecked only once (useState(initialChecked)), so changing the Storybook control won’t update the rendered switch after the first render.
Suggested fix
-import { useState } from "react";
+import { useEffect, useState } from "react";
@@
const [checked, setChecked] = useState(initialChecked);
+ useEffect(() => {
+ setChecked(initialChecked);
+ }, [initialChecked]);
return (🤖 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/components/ui/switch.stories.tsx` around lines 16 - 23, The story
initializes checked with useState(initialChecked) so updates to the Storybook
control don't propagate; update the story to synchronize local state with the
control by observing initialChecked and calling setChecked when it changes
(e.g., add a useEffect that sets checked from initialChecked), keeping the
Switch props (checked, onCheckedChange={setChecked}, disabled) unchanged so the
control and component stay in sync.
Comment on lines
+17
to
+26
| const [value, setValue] = useState(initialValue); | ||
| return ( | ||
| <View className="w-full"> | ||
| <Textarea | ||
| value={value} | ||
| onChangeText={setValue} | ||
| placeholder={placeholder} | ||
| editable={editable} | ||
| numberOfLines={numberOfLines} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant section with line numbers
ls -la apps/mobile/components/ui/textarea.stories.tsx
nl -ba apps/mobile/components/ui/textarea.stories.tsx | sed -n '1,120p'Repository: superset-sh/superset
Length of output: 221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/textarea.stories.tsx"
echo "== Lines 1-120: $FILE =="
cat -n "$FILE" | sed -n '1,120p'
echo "== Lines 10-40: $FILE =="
cat -n "$FILE" | sed -n '10,40p'Repository: superset-sh/superset
Length of output: 3867
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "useState\\(initialValue\\)" apps/mobile/components/ui || true
rg -n "useEffect\\(\\(\\) => \\{\\s*setValue\\(" apps/mobile/components/ui || true
rg -n "useState\\(" apps/mobile/components/ui/*.stories.tsx apps/mobile/components/ui/**/*.stories.tsx || trueRepository: superset-sh/superset
Length of output: 1370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILES=(
apps/mobile/components/ui/input.stories.tsx
apps/mobile/components/ui/radio-group.stories.tsx
apps/mobile/components/ui/toggle.stories.tsx
apps/mobile/components/ui/textarea.stories.tsx
apps/mobile/components/ui/tabs.stories.tsx
apps/mobile/components/ui/switch.stories.tsx
apps/mobile/components/ui/checkbox.stories.tsx
apps/mobile/components/ui/collapsible.stories.tsx
apps/mobile/components/ui/progress.stories.tsx
)
for f in "${FILES[@]}"; do
echo "===== $f ====="
if [ -f "$f" ]; then
# print first ~120 lines with line numbers
cat -n "$f" | sed -n '1,140p'
else
echo "MISSING: $f"
fi
doneRepository: superset-sh/superset
Length of output: 21371
Sync textarea internal value when Storybook initialValue changes (controls won’t update state after mount).
File: apps/mobile/components/ui/textarea.stories.tsx (lines 17–26)
Suggested fix
-import { useState } from "react";
+import { useEffect, useState } from "react";
@@
const [value, setValue] = useState(initialValue);
+ useEffect(() => {
+ setValue(initialValue);
+ }, [initialValue]);
return (📝 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
| const [value, setValue] = useState(initialValue); | |
| return ( | |
| <View className="w-full"> | |
| <Textarea | |
| value={value} | |
| onChangeText={setValue} | |
| placeholder={placeholder} | |
| editable={editable} | |
| numberOfLines={numberOfLines} | |
| /> | |
| const [value, setValue] = useState(initialValue); | |
| useEffect(() => { | |
| setValue(initialValue); | |
| }, [initialValue]); | |
| return ( | |
| <View className="w-full"> | |
| <Textarea | |
| value={value} | |
| onChangeText={setValue} | |
| placeholder={placeholder} | |
| editable={editable} | |
| numberOfLines={numberOfLines} | |
| /> |
🤖 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/components/ui/textarea.stories.tsx` around lines 17 - 26, The
Storybook story initializes local state with useState(initialValue) but never
updates it when the story control changes; add a useEffect that watches the
initialValue prop and calls setValue(initialValue) to sync the internal value
with Storybook controls so Textarea (value, setValue) reflects changes after
mount.
Comment on lines
+18
to
+24
| const [pressed, setPressed] = useState(initialPressed); | ||
| return ( | ||
| <Toggle variant={variant} pressed={pressed} onPressedChange={setPressed}> | ||
| {withIcon ? <ToggleIcon as={Bold} /> : null} | ||
| <Text>{label}</Text> | ||
| </Toggle> | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify arg-driven state desync pattern in this story file.
rg -nP 'useState\(initialPressed\)|initialPressed:\s*\{\s*control:\s*"boolean"\s*\}' apps/mobile/components/ui/toggle.stories.tsxRepository: superset-sh/superset
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Inspect the story file around the flagged lines to confirm how args drive rendering.
sed -n '1,120p' apps/mobile/components/ui/toggle.stories.tsxRepository: superset-sh/superset
Length of output: 1630
🏁 Script executed:
#!/bin/bash
rg -n "initialPressed" apps/mobile/components/ui/toggle.stories.tsxRepository: superset-sh/superset
Length of output: 315
Sync pressed with the initialPressed Storybook arg
useState(initialPressed) only uses initialPressed on first render, so updating the boolean control won’t reflect in the controlled <Toggle /> after mount (Line 18). Keep local state in sync with prop changes.
Proposed fix
import { useState } from "react";
+import { useEffect } from "react";
@@
const [pressed, setPressed] = useState(initialPressed);
+ useEffect(() => {
+ setPressed(initialPressed);
+ }, [initialPressed]);
return (📝 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
| const [pressed, setPressed] = useState(initialPressed); | |
| return ( | |
| <Toggle variant={variant} pressed={pressed} onPressedChange={setPressed}> | |
| {withIcon ? <ToggleIcon as={Bold} /> : null} | |
| <Text>{label}</Text> | |
| </Toggle> | |
| ); | |
| import { useState, useEffect } from "react"; | |
| const [pressed, setPressed] = useState(initialPressed); | |
| useEffect(() => { | |
| setPressed(initialPressed); | |
| }, [initialPressed]); | |
| return ( | |
| <Toggle variant={variant} pressed={pressed} onPressedChange={setPressed}> | |
| {withIcon ? <ToggleIcon as={Bold} /> : null} | |
| <Text>{label}</Text> | |
| </Toggle> | |
| ); |
🤖 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/components/ui/toggle.stories.tsx` around lines 18 - 24, The
story's local state (pressed via useState(initialPressed)) isn't updated when
the Storybook arg initialPressed changes; update the Toggle story to keep
pressed in sync by watching initialPressed and calling setPressed when it
changes (e.g., add a useEffect that updates pressed from initialPressed),
ensuring the controlled Toggle (pressed prop and onPressedChange={setPressed})
reflects arg updates.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 91 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/app/_layout.tsx">
<violation number="1" location="apps/mobile/app/_layout.tsx:34">
P1: Startup hangs indefinitely when font loading fails because `useFonts` error state is ignored</violation>
<violation number="2" location="apps/mobile/app/_layout.tsx:45">
P2: Empty catch block swallows `SplashScreen.hideAsync()` errors, masking startup-blocking failures without any diagnostic context</violation>
</file>
<file name="apps/mobile/components/ScrollFade/ScrollFade.tsx">
<violation number="1" location="apps/mobile/components/ScrollFade/ScrollFade.tsx:92">
P2: `"transparent"` in `LinearGradient` interpolates as transparent black, causing visible gray/dirty banding on light surfaces instead of a seamless blend. The gradient should fade to a transparent version of the same surface color (`hsla(..., 0)`) rather than the literal `"transparent"` string.</violation>
<violation number="2" location="apps/mobile/components/ScrollFade/ScrollFade.tsx:117">
P2: Spreading `...props` last on `Animated.View` allows callers to override the decorative-only safety props (`pointerEvents`, `accessibilityElementsHidden`, `importantForAccessibility`), violating the component's documented "Always `aria-hidden` + `pointerEvents: none` — decorative chrome only" invariant.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| useEffect(() => { | ||
| if (fontsLoaded) { | ||
| SplashScreen.hideAsync().catch(() => {}); |
Contributor
There was a problem hiding this comment.
P2: Empty catch block swallows SplashScreen.hideAsync() errors, masking startup-blocking failures without any diagnostic context
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/app/_layout.tsx, line 45:
<comment>Empty catch block swallows `SplashScreen.hideAsync()` errors, masking startup-blocking failures without any diagnostic context</comment>
<file context>
@@ -1,6 +1,53 @@
+
+ useEffect(() => {
+ if (fontsLoaded) {
+ SplashScreen.hideAsync().catch(() => {});
+ }
+ }, [fontsLoaded]);
</file context>
Suggested change
| SplashScreen.hideAsync().catch(() => {}); | |
| SplashScreen.hideAsync().catch((err) => { | |
| console.warn("Failed to hide splash screen:", err); | |
| }); |
Comment on lines
+92
to
+93
| const gradientColors: [string, string] = | ||
| direction === "top" ? [opaque, "transparent"] : ["transparent", opaque]; |
Contributor
There was a problem hiding this comment.
P2: "transparent" in LinearGradient interpolates as transparent black, causing visible gray/dirty banding on light surfaces instead of a seamless blend. The gradient should fade to a transparent version of the same surface color (hsla(..., 0)) rather than the literal "transparent" string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/components/ScrollFade/ScrollFade.tsx, line 92:
<comment>`"transparent"` in `LinearGradient` interpolates as transparent black, causing visible gray/dirty banding on light surfaces instead of a seamless blend. The gradient should fade to a transparent version of the same surface color (`hsla(..., 0)`) rather than the literal `"transparent"` string.</comment>
<file context>
@@ -0,0 +1,127 @@
+}: ScrollFadeProps) {
+ const scheme = (useColorScheme() ?? "dark") as "light" | "dark";
+ const opaque = surfaceColor(surface, scheme);
+ const gradientColors: [string, string] =
+ direction === "top" ? [opaque, "transparent"] : ["transparent", opaque];
+
</file context>
Suggested change
| const gradientColors: [string, string] = | |
| direction === "top" ? [opaque, "transparent"] : ["transparent", opaque]; | |
| const transparent = opaque.replace(/^hsl\((.*)\)$/, "hsla($1, 0)"); | |
| const gradientColors: [string, string] = | |
| direction === "top" ? [opaque, transparent] : [transparent, opaque]; |
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`.
justincrich
force-pushed
the
chat-mobile-sprint-1-atoms
branch
from
d6766cf to
a965c5d
Compare
justincrich
changed the title
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
changed the title
justincrich
force-pushed
the
chat-mobile-sprint-1-atoms
branch
from
a965c5d to
6f17bf7
Compare
justincrich
added a commit
to justincrich/superset
that referenced
this pull request
May 24, 2026
…ed-UI items Sprint 01 (Pixel-Perfect UI Components) shipped 2026-05-24 as a 7-PR stack (superset-sh#4870..superset-sh#4912) covering 10 atoms + 26 molecules + 14 organisms + 30 view stories. Pixel-perfect manifest gates all flipped passed for mobile-ios + mobile-android. Status: 🟠 In flight → ✅ Completed; PR cell populated with all 7 stack URLs. The original Sprint 01 inventory included 9 components that were not built standalone. They are deferred to the integration sprint that wires them, where they emerge as domain wrappers / assemblies over the as-shipped primitives (no rework — additive to each sprint's task table): Sprint 02 (+3): NewChatFab, SessionSearchBar, FilterButton domain wrappers Sprint 03 (+4): MessageMarkdown, PlanBlock, ReasoningBlock, SubagentExecutionMessage Sprint 04 (+3): TiptapPromptEditor + extensions (the largest deferral — WebView-shell editor; ComposerRow currently uses Textarea), PermissionModePicker, ThinkingLevelPicker domain wrappers Sprint 05 (+1): PendingQuestionSheet assembly Sprint 06 (+2): PushPrePromptScreen, RebableInSettingsBanner Sprint 07 (+1): HostOfflineBanner Sprint counts updated in Sprint Sequence table; Overview Current Sprint set to Sprint 02 (next). Each integration sprint now lists deferred-UI rows in bold at the top of its Tasks table with a build-first carryover note. No structural changes to the Phase 2 gate set, dependency graph, or PR-sequencing convention.
Comment on lines
+35
to
+51
| Geist_400Regular, | ||
| Geist_500Medium, | ||
| Geist_600SemiBold, | ||
| Geist_700Bold, | ||
| GeistMono_400Regular, | ||
| GeistMono_500Medium, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (fontsLoaded) { | ||
| SplashScreen.hideAsync().catch(() => {}); | ||
| } | ||
| }, [fontsLoaded]); | ||
|
|
||
| if (!fontsLoaded) return null; | ||
|
|
||
| const Root = StorybookRoot ?? RootLayout; |
Contributor
There was a problem hiding this comment.
useFonts returns [loaded, error]. When fonts fail to load for any reason (even unlikely for bundled assets), fontsLoaded stays false forever. Because SplashScreen.hideAsync() is only called when fontsLoaded is true, and the component returns null while !fontsLoaded, the app will be permanently stuck at the splash screen with no recovery path. The fontError result should also trigger hideAsync so the app renders (with system fallback fonts) rather than hanging.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/app/_layout.tsx
Line: 35-51
Comment:
**Font error ignored — app permanently frozen at splash screen**
`useFonts` returns `[loaded, error]`. When fonts fail to load for any reason (even unlikely for bundled assets), `fontsLoaded` stays `false` forever. Because `SplashScreen.hideAsync()` is only called when `fontsLoaded` is `true`, and the component returns `null` while `!fontsLoaded`, the app will be permanently stuck at the splash screen with no recovery path. The `fontError` result should also trigger `hideAsync` so the app renders (with system fallback fonts) rather than hanging.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
apps/mobile/components/ui/checkbox.stories.tsx (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRe-sync local
checkedstate wheninitialCheckedchanges.Line 16 only uses
initialCheckedfor first render, so Storybook control updates can get out of sync.Proposed fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [checked, setChecked] = useState(initialChecked); + useEffect(() => { + setChecked(initialChecked); + }, [initialChecked]);🤖 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/components/ui/checkbox.stories.tsx` at line 16, The local state variable initialized with useState (checked, setChecked) is only set from initialChecked on first render, so Storybook controls can drift; add a sync that updates checked whenever initialChecked changes by using an effect that calls setChecked(initialChecked) with initialChecked in the dependency array (keep the existing useState for local updates but ensure useEffect mirrors external prop changes).apps/mobile/components/StreamingCursor/StreamingCursor.tsx (1)
87-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp segment duration before passing to
withTiming.Line 92 and Line 94 can pass negative durations when
durationMsis very small (e.g.,1), which makes animation behavior undefined.Proposed fix
const half = resolvedDuration / 2; + const segmentDuration = Math.max(0, half - 1); opacity.value = 1; opacity.value = withRepeat( withSequence( withTiming(0.1, { duration: 1, easing: Easing.linear }), - withTiming(0.1, { duration: half - 1, easing: Easing.linear }), + withTiming(0.1, { duration: segmentDuration, easing: Easing.linear }), withTiming(1, { duration: 1, easing: Easing.linear }), - withTiming(1, { duration: half - 1, easing: Easing.linear }), + withTiming(1, { duration: segmentDuration, easing: Easing.linear }), ), -1, );🤖 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/components/StreamingCursor/StreamingCursor.tsx` around lines 87 - 95, The withTiming calls in StreamingCursor.tsx can receive negative durations when resolvedDuration or durationMs is very small (variables: resolvedDuration, half, durationMs used to compute half - 1), causing undefined animation behavior; clamp the computed segment durations before passing them to withTiming (e.g., compute safeSegment = Math.max(0, half - 1) or Math.max(0, resolvedDuration - someOffset) as appropriate) and use those safe values in the withTiming calls that build opacity.value so no negative durations are ever passed to withTiming.apps/mobile/components/ui/toggle.stories.tsx (1)
18-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
pressedsynchronized withinitialPressedStorybook args.
useState(initialPressed)won’t react to control changes after mount, so the story can drift from args.Proposed fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [pressed, setPressed] = useState(initialPressed); + useEffect(() => { + setPressed(initialPressed); + }, [initialPressed]);🤖 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/components/ui/toggle.stories.tsx` around lines 18 - 24, The story initializes local state with useState(initialPressed) which won't update when Storybook args change; update the story to synchronize pressed with initialPressed by adding an effect that calls setPressed(initialPressed) whenever initialPressed changes (keep using pressed/setPressed and pass onPressedChange to Toggle), or convert pressed to be fully controlled from the arg by using initialPressed directly — reference the pressed/setPressed state and the initialPressed prop in the Toggle story to ensure the UI follows arg updates.
🧹 Nitpick comments (1)
apps/mobile/global.css (1)
38-39: ⚡ Quick winCentralize
--color-radiusto avoid duplicate custom-property lint failures.
--color-radiusis declared with the same value in both variants. Move it to a shared scope (:rootor@theme) and keep it single-source to avoid duplicate-property lint noise.Proposed fix
`@layer` theme { :root { + --color-radius: 0.625rem; + `@variant` light { - --color-radius: 0.625rem; ... } `@variant` dark { - --color-radius: 0.625rem; ... } } }Also applies to: 97-97
🤖 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 38 - 39, Move the duplicate --color-radius custom property into a single shared scope (e.g., :root or your global `@theme` block) and remove the repeated declarations from the variant-specific blocks; specifically, add --color-radius: 0.625rem to the global scope and delete the duplicate lines where --color-radius currently appears (the occurrences inside the variant blocks) so the property is defined once and reused.
🤖 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/Typography.stories.tsx`:
- Around line 102-104: The label currently renders the literal string
variant="{variant}" instead of the variable; update the Text children to
interpolate the actual variant value (e.g. change the children to a template
string like {`variant="${variant}"`} so each row shows the real variant),
locating the usage in Typography.stories.tsx where the Text component is
rendered with the variant variable.
In `@apps/mobile/app/_layout.tsx`:
- Around line 34-50: The layout currently waits forever when fonts fail because
it only checks fontsLoaded; update the useFonts handling to also read fontError
and, inside the useEffect watching [fontsLoaded, fontError], call
SplashScreen.hideAsync() when either fontsLoaded is true or fontError is present
(and log/report the error), and stop returning null on fontError by rendering
the app (or a minimal fallback) so the splash is not permanent; adjust the
conditional that currently does "if (!fontsLoaded) return null" to allow render
when fontError exists and ensure any catch on SplashScreen.hideAsync
surfaces/logs the error.
In `@apps/mobile/components/ui/alert.stories.tsx`:
- Around line 10-24: ICON_MAP is typed as Record<string, LucideIcon> so keyof
typeof ICON_MAP collapses to string, making the AlertShowcase icon prop unsafe;
change ICON_MAP to a strongly typed literal (e.g. const ICON_MAP: { Info:
LucideIcon; WifiOff: LucideIcon; AlertTriangle: LucideIcon } = { ... } or
declare it as const with the explicit keys) so that keyof typeof ICON_MAP
becomes the union "Info" | "WifiOff" | "AlertTriangle", and keep AlertShowcase's
icon: keyof typeof ICON_MAP to enforce the safe union type.
In `@apps/mobile/components/ui/collapsible.stories.tsx`:
- Around line 22-23: The local state variable open (created via
useState(initialOpen)) doesn't update when the Storybook arg initialOpen
changes; add a sync effect in the component that runs when initialOpen changes
and updates state by calling setOpen(initialOpen) so the rendered open state
follows arg updates (i.e., add a useEffect watching initialOpen that invokes
setOpen(initialOpen)).
In `@apps/mobile/components/ui/input.stories.tsx`:
- Line 18: The story initializes local state with useState(initialValue) (value,
setValue) which only runs once, so changes to the initialValue arg won't update
the rendered Input; add an effect that watches initialValue and calls
setValue(initialValue) to sync when the arg changes (apply the same fix for the
second occurrence around the value/setValue block at the other story instance).
---
Duplicate comments:
In `@apps/mobile/components/StreamingCursor/StreamingCursor.tsx`:
- Around line 87-95: The withTiming calls in StreamingCursor.tsx can receive
negative durations when resolvedDuration or durationMs is very small (variables:
resolvedDuration, half, durationMs used to compute half - 1), causing undefined
animation behavior; clamp the computed segment durations before passing them to
withTiming (e.g., compute safeSegment = Math.max(0, half - 1) or Math.max(0,
resolvedDuration - someOffset) as appropriate) and use those safe values in the
withTiming calls that build opacity.value so no negative durations are ever
passed to withTiming.
In `@apps/mobile/components/ui/checkbox.stories.tsx`:
- Line 16: The local state variable initialized with useState (checked,
setChecked) is only set from initialChecked on first render, so Storybook
controls can drift; add a sync that updates checked whenever initialChecked
changes by using an effect that calls setChecked(initialChecked) with
initialChecked in the dependency array (keep the existing useState for local
updates but ensure useEffect mirrors external prop changes).
In `@apps/mobile/components/ui/toggle.stories.tsx`:
- Around line 18-24: The story initializes local state with
useState(initialPressed) which won't update when Storybook args change; update
the story to synchronize pressed with initialPressed by adding an effect that
calls setPressed(initialPressed) whenever initialPressed changes (keep using
pressed/setPressed and pass onPressedChange to Toggle), or convert pressed to be
fully controlled from the arg by using initialPressed directly — reference the
pressed/setPressed state and the initialPressed prop in the Toggle story to
ensure the UI follows arg updates.
---
Nitpick comments:
In `@apps/mobile/global.css`:
- Around line 38-39: Move the duplicate --color-radius custom property into a
single shared scope (e.g., :root or your global `@theme` block) and remove the
repeated declarations from the variant-specific blocks; specifically, add
--color-radius: 0.625rem to the global scope and delete the duplicate lines
where --color-radius currently appears (the occurrences inside the variant
blocks) so the property is defined once and reused.
🪄 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: ed137688-f44d-494f-bb9a-2b2e01a46bd0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (93)
apps/mobile/.rnstorybook/.gitignoreapps/mobile/.rnstorybook/StorybookRouterProvider.tsxapps/mobile/.rnstorybook/index.tsxapps/mobile/.rnstorybook/main.jsapps/mobile/.rnstorybook/preview.tsxapps/mobile/.rnstorybook/router/LinkingContext.tsapps/mobile/.rnstorybook/router/UnhandledLinkingContext.tsapps/mobile/.rnstorybook/stories/DesignSystem/Colors.stories.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Icons.stories.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Spacing.stories.tsxapps/mobile/.rnstorybook/stories/DesignSystem/Typography.stories.tsxapps/mobile/app/_layout.tsxapps/mobile/components/FabBase/FabBase.stories.tsxapps/mobile/components/FabBase/FabBase.tsxapps/mobile/components/FabBase/index.tsapps/mobile/components/HelloWorld/HelloWorld.stories.tsxapps/mobile/components/HelloWorld/HelloWorld.tsxapps/mobile/components/HelloWorld/index.tsapps/mobile/components/HitTargetWrapper/HitTargetWrapper.stories.tsxapps/mobile/components/HitTargetWrapper/HitTargetWrapper.tsxapps/mobile/components/HitTargetWrapper/index.tsapps/mobile/components/IconButton/IconButton.stories.tsxapps/mobile/components/IconButton/IconButton.tsxapps/mobile/components/IconButton/index.tsapps/mobile/components/Pill/Pill.stories.tsxapps/mobile/components/Pill/Pill.tsxapps/mobile/components/Pill/index.tsapps/mobile/components/ProgressDots/ProgressDots.stories.tsxapps/mobile/components/ProgressDots/ProgressDots.tsxapps/mobile/components/ProgressDots/index.tsapps/mobile/components/ScrollFade/ScrollFade.stories.tsxapps/mobile/components/ScrollFade/ScrollFade.tsxapps/mobile/components/ScrollFade/index.tsapps/mobile/components/StatusDot/StatusDot.stories.tsxapps/mobile/components/StatusDot/StatusDot.tsxapps/mobile/components/StatusDot/index.tsapps/mobile/components/StreamingCursor/StreamingCursor.stories.tsxapps/mobile/components/StreamingCursor/StreamingCursor.tsxapps/mobile/components/StreamingCursor/index.tsapps/mobile/components/ToastBase/ToastBase.stories.tsxapps/mobile/components/ToastBase/ToastBase.tsxapps/mobile/components/ToastBase/index.tsapps/mobile/components/ToolStatusRule/ToolStatusRule.stories.tsxapps/mobile/components/ToolStatusRule/ToolStatusRule.tsxapps/mobile/components/ToolStatusRule/index.tsapps/mobile/components/ui/AUDIT.mdapps/mobile/components/ui/accordion.stories.tsxapps/mobile/components/ui/alert-dialog.stories.tsxapps/mobile/components/ui/alert.stories.tsxapps/mobile/components/ui/aspect-ratio.stories.tsxapps/mobile/components/ui/avatar.stories.tsxapps/mobile/components/ui/badge.stories.tsxapps/mobile/components/ui/button.stories.tsxapps/mobile/components/ui/card.stories.tsxapps/mobile/components/ui/checkbox.stories.tsxapps/mobile/components/ui/collapsible.stories.tsxapps/mobile/components/ui/context-menu.stories.tsxapps/mobile/components/ui/dialog.stories.tsxapps/mobile/components/ui/dropdown-menu.stories.tsxapps/mobile/components/ui/hover-card.stories.tsxapps/mobile/components/ui/icon.stories.tsxapps/mobile/components/ui/input.stories.tsxapps/mobile/components/ui/label.stories.tsxapps/mobile/components/ui/menubar.stories.tsxapps/mobile/components/ui/popover.stories.tsxapps/mobile/components/ui/progress.stories.tsxapps/mobile/components/ui/radio-group.stories.tsxapps/mobile/components/ui/select.stories.tsxapps/mobile/components/ui/separator.stories.tsxapps/mobile/components/ui/skeleton.stories.tsxapps/mobile/components/ui/switch.stories.tsxapps/mobile/components/ui/tabs.stories.tsxapps/mobile/components/ui/text.stories.tsxapps/mobile/components/ui/textarea.stories.tsxapps/mobile/components/ui/toggle-group.stories.tsxapps/mobile/components/ui/toggle.stories.tsxapps/mobile/components/ui/tooltip.stories.tsxapps/mobile/design/manifest.jsonapps/mobile/global.cssapps/mobile/lib/theme.tsapps/mobile/metro.config.jsapps/mobile/package.jsonapps/mobile/screens/(auth)/sign-in/components/DevSignInButton/DevSignInButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/OrganizationSwitcherSheet.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.stories.tsxapps/mobile/screens/(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.stories.tsxapps/mobile/screens/(authenticated)/components/OrgDropdown/OrgDropdown.stories.tsxapps/mobile/screens/(authenticated)/components/TabBarAccessory/TabBarAccessory.stories.tsxapps/mobile/screens/AUDIT.md
✅ Files skipped from review due to trivial changes (18)
- apps/mobile/components/ScrollFade/index.ts
- apps/mobile/components/FabBase/index.ts
- apps/mobile/.rnstorybook/.gitignore
- apps/mobile/components/IconButton/index.ts
- apps/mobile/.rnstorybook/router/UnhandledLinkingContext.ts
- apps/mobile/components/ProgressDots/index.ts
- apps/mobile/components/HelloWorld/index.ts
- apps/mobile/components/StatusDot/index.ts
- apps/mobile/components/ui/aspect-ratio.stories.tsx
- apps/mobile/components/StreamingCursor/index.ts
- apps/mobile/components/ToastBase/index.ts
- apps/mobile/components/ToolStatusRule/index.ts
- apps/mobile/components/HitTargetWrapper/index.ts
- apps/mobile/components/ui/toggle-group.stories.tsx
- apps/mobile/screens/AUDIT.md
- apps/mobile/.rnstorybook/main.js
- apps/mobile/components/ui/AUDIT.md
- apps/mobile/components/ui/select.stories.tsx
Comment on lines
+10
to
+24
| const ICON_MAP: Record<string, LucideIcon> = { | ||
| Info: InfoIcon, | ||
| WifiOff, | ||
| AlertTriangle, | ||
| }; | ||
|
|
||
| function AlertShowcase({ | ||
| variant, | ||
| icon, | ||
| title, | ||
| description, | ||
| }: { | ||
| variant: "default" | "destructive"; | ||
| icon: keyof typeof ICON_MAP; | ||
| title: string; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/alert.stories.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "=== File: $FILE ==="
nl -ba "$FILE" | sed -n '1,120p'Repository: superset-sh/superset
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Extra: ensure no other ICON_MAP declarations in repo that might affect analysis
rg -n "const ICON_MAP" -S --hidden .Repository: superset-sh/superset
Length of output: 469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ui/alert.stories.tsx"
python3 - <<'PY'
import itertools, pathlib
p = pathlib.Path("apps/mobile/components/ui/alert.stories.tsx")
lines = p.read_text().splitlines()
for i in range(1, min(len(lines), 140)+1):
if 1 <= i <= 60:
pass
# print with line numbers for relevant range 1-80
for i in range(1, 80):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: superset-sh/superset
Length of output: 2222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Sanity check: show TypeScript key usage in this file
rg -n "ICON_MAP|keyof typeof ICON_MAP|icon\:" apps/mobile/components/ui/alert.stories.tsxRepository: superset-sh/superset
Length of output: 203
Strongly type ICON_MAP keys to keep icon as a safe union (not string)
ICON_MAP is declared as Record<string, LucideIcon>, so keyof typeof ICON_MAP becomes string, making the icon prop accept any string even though only Info | WifiOff | AlertTriangle exist.
Suggested fix
-const ICON_MAP: Record<string, LucideIcon> = {
+const ICON_MAP = {
Info: InfoIcon,
WifiOff,
AlertTriangle,
-};
+} as const satisfies Record<string, LucideIcon>;🤖 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/components/ui/alert.stories.tsx` around lines 10 - 24, ICON_MAP
is typed as Record<string, LucideIcon> so keyof typeof ICON_MAP collapses to
string, making the AlertShowcase icon prop unsafe; change ICON_MAP to a strongly
typed literal (e.g. const ICON_MAP: { Info: LucideIcon; WifiOff: LucideIcon;
AlertTriangle: LucideIcon } = { ... } or declare it as const with the explicit
keys) so that keyof typeof ICON_MAP becomes the union "Info" | "WifiOff" |
"AlertTriangle", and keep AlertShowcase's icon: keyof typeof ICON_MAP to enforce
the safe union type.
Comment on lines
+22
to
+23
| const [open, setOpen] = useState(initialOpen); | ||
| return ( |
Contributor
There was a problem hiding this comment.
Sync open state when Storybook initialOpen arg changes.
Line 22 initializes local state once, so control changes to initialOpen won’t update the rendered state after mount.
Proposed fix
-import { useState } from "react";
+import { useEffect, useState } from "react";
@@
const [open, setOpen] = useState(initialOpen);
+ useEffect(() => {
+ setOpen(initialOpen);
+ }, [initialOpen]);🤖 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/components/ui/collapsible.stories.tsx` around lines 22 - 23, The
local state variable open (created via useState(initialOpen)) doesn't update
when the Storybook arg initialOpen changes; add a sync effect in the component
that runs when initialOpen changes and updates state by calling
setOpen(initialOpen) so the rendered open state follows arg updates (i.e., add a
useEffect watching initialOpen that invokes setOpen(initialOpen)).
| initialValue: string; | ||
| showLabel: boolean; | ||
| }) { | ||
| const [value, setValue] = useState(initialValue); |
Contributor
There was a problem hiding this comment.
Sync local state with initialValue arg changes.
useState(initialValue) only hydrates once, so updating the initialValue control won’t update the rendered Input after mount.
Proposed fix
-import { useState } from "react";
+import { useEffect, useState } from "react";
@@
const [value, setValue] = useState(initialValue);
+ useEffect(() => {
+ setValue(initialValue);
+ }, [initialValue]);Also applies to: 49-53
🤖 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/components/ui/input.stories.tsx` at line 18, The story
initializes local state with useState(initialValue) (value, setValue) which only
runs once, so changes to the initialValue arg won't update the rendered Input;
add an effect that watches initialValue and calls setValue(initialValue) to sync
when the arg changes (apply the same fix for the second occurrence around the
value/setValue block at the other story instance).
Contributor
There was a problem hiding this comment.
2 issues found across 94 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/app/_layout.tsx">
<violation number="1" location="apps/mobile/app/_layout.tsx:34">
P1: Startup hangs indefinitely when font loading fails because `useFonts` error state is ignored</violation>
<violation number="2" location="apps/mobile/app/_layout.tsx:34">
P2: Font loading error path is unhandled, which can leave the app permanently blocked behind the splash screen.</violation>
<violation number="3" location="apps/mobile/app/_layout.tsx:45">
P2: Empty catch block swallows `SplashScreen.hideAsync()` errors, masking startup-blocking failures without any diagnostic context</violation>
</file>
<file name="apps/mobile/components/ScrollFade/ScrollFade.tsx">
<violation number="1" location="apps/mobile/components/ScrollFade/ScrollFade.tsx:92">
P2: `"transparent"` in `LinearGradient` interpolates as transparent black, causing visible gray/dirty banding on light surfaces instead of a seamless blend. The gradient should fade to a transparent version of the same surface color (`hsla(..., 0)`) rather than the literal `"transparent"` string.</violation>
<violation number="2" location="apps/mobile/components/ScrollFade/ScrollFade.tsx:117">
P2: Spreading `...props` last on `Animated.View` allows callers to override the decorative-only safety props (`pointerEvents`, `accessibilityElementsHidden`, `importantForAccessibility`), violating the component's documented "Always `aria-hidden` + `pointerEvents: none` — decorative chrome only" invariant.</violation>
</file>
<file name="apps/mobile/components/ProgressDots/ProgressDots.tsx">
<violation number="1" location="apps/mobile/components/ProgressDots/ProgressDots.tsx:126">
P2: Animation does not reset to its intended baseline of 0.4 after being paused or when reduce-motion is toggled off, causing dots to pulse between the stale value (0.8) and 1 instead of between 0.4 and 1.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }; | ||
|
|
||
| function Dot({ index, variant, size, animate }: DotProps) { | ||
| const opacity = useSharedValue(0.4); |
Contributor
There was a problem hiding this comment.
P2: Animation does not reset to its intended baseline of 0.4 after being paused or when reduce-motion is toggled off, causing dots to pulse between the stale value (0.8) and 1 instead of between 0.4 and 1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/components/ProgressDots/ProgressDots.tsx, line 126:
<comment>Animation does not reset to its intended baseline of 0.4 after being paused or when reduce-motion is toggled off, causing dots to pulse between the stale value (0.8) and 1 instead of between 0.4 and 1.</comment>
<file context>
@@ -0,0 +1,158 @@
+};
+
+function Dot({ index, variant, size, animate }: DotProps) {
+ const opacity = useSharedValue(0.4);
+
+ useEffect(() => {
</file context>
- 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
Matches global.css and theme.ts exactly to origin/main — no custom theme changes in this PR, only Storybook tooling additions.
justincrich
force-pushed
the
chat-mobile-sprint-1-atoms
branch
from
6f17bf7 to
5804853
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
apps/mobile/components/ToastBase/ToastBase.tsx (1)
136-136:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude both title and body in the fallback announcement.
At Line 136,
accessibilityLabel ?? title ?? bodydrops the body whenevertitleis present.Proposed fix
- accessibilityLabel={accessibilityLabel ?? title ?? body} + accessibilityLabel={ + accessibilityLabel ?? (title ? `${title}. ${body}` : body) + }🤖 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/components/ToastBase/ToastBase.tsx` at line 136, The accessibilityLabel fallback currently uses "accessibilityLabel ?? title ?? body", which omits the body when a title exists; update the ToastBase accessibilityLabel logic (in the ToastBase component where accessibilityLabel, title, and body are used) to concatenate title and body when accessibilityLabel is not provided—e.g., join non-empty title and body with a separator so both appear in the announced string while still preferring an explicit accessibilityLabel.apps/mobile/components/ScrollFade/ScrollFade.tsx (2)
32-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent overriding decorative-only accessibility/interaction invariants.
...propsis spread last, so callers can overridepointerEventsand accessibility-hidden flags (Line 107–110), breaking the decorative-only contract.Proposed fix
-export type ScrollFadeProps = Omit<ViewProps, "children"> & { +export type ScrollFadeProps = Omit< + ViewProps, + "children" | "pointerEvents" | "accessibilityElementsHidden" | "importantForAccessibility" +> & { @@ return ( <Animated.View + {...props} accessibilityElementsHidden importantForAccessibility="no-hide-descendants" pointerEvents="none" @@ - {...props} >Also applies to: 106-118
🤖 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/components/ScrollFade/ScrollFade.tsx` around lines 32 - 41, The component allows callers to override decorative-only accessibility/interaction invariants by spreading ...props last; modify the ScrollFade component so that any incoming pointerEvents and accessibility-hidden flags cannot be overridden: either spread ...props first and then set pointerEvents="none" and accessibilityElementsHidden/importantForAccessibility/accessible (or whatever props you enforce) after the spread, or explicitly strip those keys from props before spreading (e.g., delete pointerEvents, accessibilityElementsHidden, importantForAccessibility, accessible) and then apply the fixed values; update the code around the ScrollFade render where ...props is used to ensure the enforced props are applied last and cannot be clobbered.
92-94:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse alpha-matched transparent stops instead of literal
"transparent".At Line 92–94, interpolating with
"transparent"can produce undesired dark tinting in gradients. Prefer a transparent stop derived from the same base color asopaque.In expo-linear-gradient / React Native, does using the literal "transparent" color interpolate toward transparent black, and is using an alpha-0 version of the same base color the recommended approach for hue-preserving fades?🤖 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/components/ScrollFade/ScrollFade.tsx` around lines 92 - 94, The gradient currently uses the literal "transparent" which can blend to transparent black and shift hue; update the gradientColors assignment in ScrollFade (the gradientColors constant computed from direction and opaque) to use an alpha-0 version of the same base color instead of the string "transparent" — derive a transparentStop from opaque (e.g., convert opaque to an rgba/hex with 0 alpha) and use [opaque, transparentStop] or [transparentStop, opaque] depending on direction, preserving the [string,string] type and existing variable names.apps/mobile/components/ui/alert.stories.tsx (1)
10-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow
ICON_MAPtyping soiconstays a strict union.
ICON_MAP: Record<string, LucideIcon>makeskeyof typeof ICON_MAPresolve tostring, soiconaccepts arbitrary values instead of just the defined icon keys.Suggested fix
-const ICON_MAP: Record<string, LucideIcon> = { +const ICON_MAP = { Info: InfoIcon, WifiOff, AlertTriangle, -}; +} as const satisfies Record<string, LucideIcon>;In TypeScript, what does `keyof Record<string, T>` evaluate to, and how can I preserve a literal key union for an object map while still validating values with `satisfies`?As per coding guidelines: "
**/*.{ts,tsx}: Avoidanytype unless necessary; maintain type safety in TypeScript files".🤖 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/components/ui/alert.stories.tsx` around lines 10 - 24, ICON_MAP is currently typed as Record<string, LucideIcon>, which makes keyof typeof ICON_MAP resolve to string and lets the AlertShowcase icon prop accept arbitrary strings; fix this by declaring ICON_MAP with literal keys so the keys remain a strict union (e.g. create ICON_MAP as a const object with the actual keys and use "satisfies Record<... , LucideIcon>" to validate the values), then derive the icon prop type as IconKey = keyof typeof ICON_MAP and use icon: IconKey in AlertShowcase so only the defined keys are allowed.apps/mobile/components/ui/switch.stories.tsx (1)
2-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync
checkedwheninitialCheckedarg changes.
checkedis initialized once frominitialChecked(Line 16), so Storybook control updates won’t propagate after mount.Suggested fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [checked, setChecked] = useState(initialChecked); + useEffect(() => { + setChecked(initialChecked); + }, [initialChecked]); return (🤖 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/components/ui/switch.stories.tsx` around lines 2 - 23, SwitchShowcase initializes local state checked from the initialChecked prop once, so Storybook control updates don't propagate; modify SwitchShowcase to watch initialChecked and call setChecked when it changes (e.g., add a useEffect that depends on initialChecked) so the local state stays in sync with the prop while keeping the existing useState and onCheckedChange behavior for user interaction.apps/mobile/components/ui/toggle.stories.tsx (1)
3-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync
pressedtoinitialPressedarg updates.
pressedis initialized once frominitialPressed(Line 18), so Storybook control changes are ignored after the first render.Suggested fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [pressed, setPressed] = useState(initialPressed); + useEffect(() => { + setPressed(initialPressed); + }, [initialPressed]); return (🤖 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/components/ui/toggle.stories.tsx` around lines 3 - 24, ToggleShowcase currently initializes local state pressed from initialPressed and never updates it when the story arg changes; add a useEffect in the ToggleShowcase component that watches initialPressed and calls setPressed(initialPressed) to sync prop updates, keeping the existing controlled Toggle usage (Toggle, onPressedChange, setPressed) intact so Storybook controls update the rendered Toggle.apps/mobile/components/ui/textarea.stories.tsx (1)
2-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
valuein sync withinitialValuecontrol changes.
valueis only initialized frominitialValue(Line 17), so changing the Storybook control later won’t update the rendered textarea.Suggested fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [value, setValue] = useState(initialValue); + useEffect(() => { + setValue(initialValue); + }, [initialValue]); return (🤖 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/components/ui/textarea.stories.tsx` around lines 2 - 26, TextareaShowcase initializes local state value from initialValue but doesn’t update when the Storybook control changes; update the component to watch initialValue and call setValue when it changes (e.g., add an effect inside TextareaShowcase that runs on initialValue change and updates value) so the Textarea value stays in sync with the initialValue prop.
🤖 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/components/StatusDot/StatusDot.tsx`:
- Around line 121-124: The View in the StatusDot component using props
accessibilityLabel and accessibilityRole should be made explicitly accessible
when an accessibilityLabel is provided; update the View (in the StatusDot
component) to include accessible={!!accessibilityLabel} so the element becomes a
screen-reader focusable/controlable element, keeping the existing
accessibilityLabel and accessibilityRole logic intact.
In `@apps/mobile/components/ui/icon.stories.tsx`:
- Around line 69-72: The intro copy in the Text JSX (the Text variant="muted"
block inside the icon story) wrongly says “Click an icon name” which is
misleading on mobile; update the string to a non-interactive or
touch-appropriate phrasing such as “Tap an icon in the catalog to inspect” or
“Select an icon in the catalog to inspect,” replacing the existing sentence in
the Text component within the icon story component.
---
Duplicate comments:
In `@apps/mobile/components/ScrollFade/ScrollFade.tsx`:
- Around line 32-41: The component allows callers to override decorative-only
accessibility/interaction invariants by spreading ...props last; modify the
ScrollFade component so that any incoming pointerEvents and accessibility-hidden
flags cannot be overridden: either spread ...props first and then set
pointerEvents="none" and
accessibilityElementsHidden/importantForAccessibility/accessible (or whatever
props you enforce) after the spread, or explicitly strip those keys from props
before spreading (e.g., delete pointerEvents, accessibilityElementsHidden,
importantForAccessibility, accessible) and then apply the fixed values; update
the code around the ScrollFade render where ...props is used to ensure the
enforced props are applied last and cannot be clobbered.
- Around line 92-94: The gradient currently uses the literal "transparent" which
can blend to transparent black and shift hue; update the gradientColors
assignment in ScrollFade (the gradientColors constant computed from direction
and opaque) to use an alpha-0 version of the same base color instead of the
string "transparent" — derive a transparentStop from opaque (e.g., convert
opaque to an rgba/hex with 0 alpha) and use [opaque, transparentStop] or
[transparentStop, opaque] depending on direction, preserving the [string,string]
type and existing variable names.
In `@apps/mobile/components/ToastBase/ToastBase.tsx`:
- Line 136: The accessibilityLabel fallback currently uses "accessibilityLabel
?? title ?? body", which omits the body when a title exists; update the
ToastBase accessibilityLabel logic (in the ToastBase component where
accessibilityLabel, title, and body are used) to concatenate title and body when
accessibilityLabel is not provided—e.g., join non-empty title and body with a
separator so both appear in the announced string while still preferring an
explicit accessibilityLabel.
In `@apps/mobile/components/ui/alert.stories.tsx`:
- Around line 10-24: ICON_MAP is currently typed as Record<string, LucideIcon>,
which makes keyof typeof ICON_MAP resolve to string and lets the AlertShowcase
icon prop accept arbitrary strings; fix this by declaring ICON_MAP with literal
keys so the keys remain a strict union (e.g. create ICON_MAP as a const object
with the actual keys and use "satisfies Record<... , LucideIcon>" to validate
the values), then derive the icon prop type as IconKey = keyof typeof ICON_MAP
and use icon: IconKey in AlertShowcase so only the defined keys are allowed.
In `@apps/mobile/components/ui/switch.stories.tsx`:
- Around line 2-23: SwitchShowcase initializes local state checked from the
initialChecked prop once, so Storybook control updates don't propagate; modify
SwitchShowcase to watch initialChecked and call setChecked when it changes
(e.g., add a useEffect that depends on initialChecked) so the local state stays
in sync with the prop while keeping the existing useState and onCheckedChange
behavior for user interaction.
In `@apps/mobile/components/ui/textarea.stories.tsx`:
- Around line 2-26: TextareaShowcase initializes local state value from
initialValue but doesn’t update when the Storybook control changes; update the
component to watch initialValue and call setValue when it changes (e.g., add an
effect inside TextareaShowcase that runs on initialValue change and updates
value) so the Textarea value stays in sync with the initialValue prop.
In `@apps/mobile/components/ui/toggle.stories.tsx`:
- Around line 3-24: ToggleShowcase currently initializes local state pressed
from initialPressed and never updates it when the story arg changes; add a
useEffect in the ToggleShowcase component that watches initialPressed and calls
setPressed(initialPressed) to sync prop updates, keeping the existing controlled
Toggle usage (Toggle, onPressedChange, setPressed) intact so Storybook controls
update the rendered Toggle.
🪄 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: 4144424a-5401-437c-bf77-4eb2c9e94de3
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (79)
apps/mobile/.rnstorybook/main.jsapps/mobile/.rnstorybook/mocks/tty.jsapps/mobile/.rnstorybook/preview.tsxapps/mobile/app/_layout.tsxapps/mobile/components/FabBase/FabBase.stories.tsxapps/mobile/components/FabBase/FabBase.tsxapps/mobile/components/FabBase/index.tsapps/mobile/components/HitTargetWrapper/HitTargetWrapper.stories.tsxapps/mobile/components/HitTargetWrapper/HitTargetWrapper.tsxapps/mobile/components/HitTargetWrapper/index.tsapps/mobile/components/IconButton/IconButton.stories.tsxapps/mobile/components/IconButton/IconButton.tsxapps/mobile/components/IconButton/index.tsapps/mobile/components/Pill/Pill.stories.tsxapps/mobile/components/Pill/Pill.tsxapps/mobile/components/Pill/index.tsapps/mobile/components/ProgressDots/ProgressDots.stories.tsxapps/mobile/components/ProgressDots/ProgressDots.tsxapps/mobile/components/ProgressDots/index.tsapps/mobile/components/ScrollFade/ScrollFade.stories.tsxapps/mobile/components/ScrollFade/ScrollFade.tsxapps/mobile/components/ScrollFade/index.tsapps/mobile/components/StatusDot/StatusDot.stories.tsxapps/mobile/components/StatusDot/StatusDot.tsxapps/mobile/components/StatusDot/index.tsapps/mobile/components/StreamingCursor/StreamingCursor.stories.tsxapps/mobile/components/StreamingCursor/StreamingCursor.tsxapps/mobile/components/StreamingCursor/index.tsapps/mobile/components/ToastBase/ToastBase.stories.tsxapps/mobile/components/ToastBase/ToastBase.tsxapps/mobile/components/ToastBase/index.tsapps/mobile/components/ToolStatusRule/ToolStatusRule.stories.tsxapps/mobile/components/ToolStatusRule/ToolStatusRule.tsxapps/mobile/components/ToolStatusRule/index.tsapps/mobile/components/ui/AUDIT.mdapps/mobile/components/ui/accordion.stories.tsxapps/mobile/components/ui/alert-dialog.stories.tsxapps/mobile/components/ui/alert.stories.tsxapps/mobile/components/ui/aspect-ratio.stories.tsxapps/mobile/components/ui/avatar.stories.tsxapps/mobile/components/ui/badge.stories.tsxapps/mobile/components/ui/button.stories.tsxapps/mobile/components/ui/card.stories.tsxapps/mobile/components/ui/checkbox.stories.tsxapps/mobile/components/ui/collapsible.stories.tsxapps/mobile/components/ui/context-menu.stories.tsxapps/mobile/components/ui/dialog.stories.tsxapps/mobile/components/ui/dropdown-menu.stories.tsxapps/mobile/components/ui/hover-card.stories.tsxapps/mobile/components/ui/icon.stories.tsxapps/mobile/components/ui/input.stories.tsxapps/mobile/components/ui/label.stories.tsxapps/mobile/components/ui/menubar.stories.tsxapps/mobile/components/ui/popover.stories.tsxapps/mobile/components/ui/progress.stories.tsxapps/mobile/components/ui/radio-group.stories.tsxapps/mobile/components/ui/select.stories.tsxapps/mobile/components/ui/separator.stories.tsxapps/mobile/components/ui/skeleton.stories.tsxapps/mobile/components/ui/switch.stories.tsxapps/mobile/components/ui/tabs.stories.tsxapps/mobile/components/ui/text.stories.tsxapps/mobile/components/ui/textarea.stories.tsxapps/mobile/components/ui/toggle-group.stories.tsxapps/mobile/components/ui/toggle.stories.tsxapps/mobile/components/ui/tooltip.stories.tsxapps/mobile/metro.config.jsapps/mobile/package.jsonapps/mobile/screens/(auth)/sign-in/components/DevSignInButton/DevSignInButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/OrganizationSwitcherSheet.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.stories.tsxapps/mobile/screens/(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.stories.tsxapps/mobile/screens/(authenticated)/components/OrgDropdown/OrgDropdown.stories.tsxapps/mobile/screens/(authenticated)/components/TabBarAccessory/TabBarAccessory.stories.tsxapps/mobile/screens/AUDIT.md
💤 Files with no reviewable changes (2)
- apps/mobile/.rnstorybook/main.js
- apps/mobile/metro.config.js
✅ Files skipped from review due to trivial changes (10)
- apps/mobile/components/HitTargetWrapper/index.ts
- apps/mobile/.rnstorybook/mocks/tty.js
- apps/mobile/components/IconButton/index.ts
- apps/mobile/screens/(auth)/sign-in/components/DevSignInButton/DevSignInButton.stories.tsx
- apps/mobile/components/ScrollFade/index.ts
- apps/mobile/components/ProgressDots/index.ts
- apps/mobile/components/ToolStatusRule/index.ts
- apps/mobile/screens/AUDIT.md
- apps/mobile/components/ui/AUDIT.md
- apps/mobile/components/ui/hover-card.stories.tsx
Comment on lines
+121
to
+124
| <View | ||
| accessibilityLabel={accessibilityLabel} | ||
| accessibilityRole={accessibilityLabel ? "image" : undefined} | ||
| className="items-center justify-center" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React Native 0.85.x, does a ViewwithaccessibilityLabelandaccessibilityRolerequireaccessible={true} to be exposed as a screen-reader focusable element?
💡 Result:
In React Native 0.85.x, a standard View component does not automatically become a screen-reader focusable element simply by having accessibilityLabel or accessibilityRole props applied [1][2]. To ensure a View is exposed as a focusable element to assistive technologies, you must explicitly set the accessible prop to true [1][3][4]. When accessible={true} is set, the View (and its children) are treated as a single, discoverable unit by screen readers like VoiceOver and TalkBack [1][2][5]. Without this prop, a basic View is ignored by assistive technologies, meaning any accessibilityLabel or accessibilityRole assigned to it will typically not be announced or accessible to the user [6][4][5]. Note that certain interactive components (such as TouchableOpacity, Pressable, or Switch) are accessible={true} by default [7][8][5]. For these components, you do not need to manually add the accessible prop unless you wish to override the default grouping behavior [7]. However, for a generic View used as a container or custom control, the explicit accessible={true} prop is required to make it focusable [1][4].
Citations:
- 1: https://reactnative.dev/docs/accessibility.md
- 2: https://reactnative.dev/docs/accessibility
- 3: https://reactnative.dev/docs/next/accessibility
- 4: https://oneuptime.com/blog/post/2026-01-15-react-native-accessible-components/view
- 5: https://stackoverflow.com/questions/53226986/what-is-the-difference-between-accessible-accessibilitylabel-and-accessibilityh
- 6: https://www.accessibilitychecker.org/blog/react-native-accessibility/
- 7: https://stackoverflow.com/questions/61914642/when-should-we-use-accessibilityrole-in-react-native
- 8: https://reactnative.dev/docs/0.83/accessibility
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
ls -la apps/mobile/components/StatusDot/StatusDot.tsx
# Show the relevant region around the referenced lines
sed -n '100,160p' apps/mobile/components/StatusDot/StatusDot.tsx | nl -baRepository: superset-sh/superset
Length of output: 221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("apps/mobile/components/StatusDot/StatusDot.tsx")
lines = p.read_text(encoding="utf-8").splitlines()
start, end = 105, 145
for i in range(start, end+1):
if 1 <= i <= len(lines):
print(f"{i:4d} {lines[i-1]}")
PYRepository: superset-sh/superset
Length of output: 1189
Make the labeled dot View explicitly accessible.
File: apps/mobile/components/StatusDot/StatusDot.tsx (lines 121-126)
A plain React Native View isn’t reliably exposed as a screen-reader focusable element from accessibilityLabel/accessibilityRole alone; set accessible when accessibilityLabel is provided.
🔧 Proposed fix
return (
<View
+ accessible={Boolean(accessibilityLabel)}
accessibilityLabel={accessibilityLabel}
accessibilityRole={accessibilityLabel ? "image" : undefined}
className="items-center justify-center"
{...props}
>📝 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
| <View | |
| accessibilityLabel={accessibilityLabel} | |
| accessibilityRole={accessibilityLabel ? "image" : undefined} | |
| className="items-center justify-center" | |
| <View | |
| accessible={Boolean(accessibilityLabel)} | |
| accessibilityLabel={accessibilityLabel} | |
| accessibilityRole={accessibilityLabel ? "image" : undefined} | |
| className="items-center justify-center" | |
| {...props} | |
| > |
🤖 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/components/StatusDot/StatusDot.tsx` around lines 121 - 124, The
View in the StatusDot component using props accessibilityLabel and
accessibilityRole should be made explicitly accessible when an
accessibilityLabel is provided; update the View (in the StatusDot component) to
include accessible={!!accessibilityLabel} so the element becomes a screen-reader
focusable/controlable element, keeping the existing accessibilityLabel and
accessibilityRole logic intact.
Comment on lines
+69
to
+72
| <Text variant="muted" className="mb-4"> | ||
| Click an icon name in the catalog to inspect. All icons are | ||
| lucide-react-native, themed via the Icon wrapper (size + color | ||
| className). |
Contributor
There was a problem hiding this comment.
Misleading interaction copy in catalog intro.
Line 70 says users can “Click an icon name,” but icon names aren’t interactive in this story. Update the copy to avoid confusion.
✏️ Proposed copy fix
- <Text variant="muted" className="mb-4">
- Click an icon name in the catalog to inspect. All icons are
+ <Text variant="muted" className="mb-4">
+ Browse the icon catalog below. All icons are
lucide-react-native, themed via the Icon wrapper (size + color
className).
</Text>📝 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
| <Text variant="muted" className="mb-4"> | |
| Click an icon name in the catalog to inspect. All icons are | |
| lucide-react-native, themed via the Icon wrapper (size + color | |
| className). | |
| <Text variant="muted" className="mb-4"> | |
| Browse the icon catalog below. All icons are | |
| lucide-react-native, themed via the Icon wrapper (size + color | |
| className). | |
| </Text> |
🤖 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/components/ui/icon.stories.tsx` around lines 69 - 72, The intro
copy in the Text JSX (the Text variant="muted" block inside the icon story)
wrongly says “Click an icon name” which is misleading on mobile; update the
string to a non-interactive or touch-appropriate phrasing such as “Tap an icon
in the catalog to inspect” or “Select an icon in the catalog to inspect,”
replacing the existing sentence in the Text component within the icon story
component.
- 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).
Comment on lines
+171
to
+177
| <HitTargetWrapper | ||
| accessibilityLabel={dismissAccessibilityLabel ?? "Dismiss"} | ||
| shape="circle" | ||
| onPress={onDismiss} | ||
| > | ||
| <Icon as={X} className="size-3.5 text-muted-foreground" /> | ||
| </HitTargetWrapper> |
Collaborator
There was a problem hiding this comment.
This HitTargetWrapper usage is the only one in the codebase — and it's really an icon button, so IconButton already covers it.
The dismiss ✕ here is exactly what the sibling IconButton atom does: an icon-only Pressable with a guaranteed 44pt target. IconButton is actually more complete for touch — it combines the h-touch-min w-touch-min box and a hitSlop top-up for smaller sizes, whereas HitTargetWrapper is box-only.
Suggested change
| <HitTargetWrapper | |
| accessibilityLabel={dismissAccessibilityLabel ?? "Dismiss"} | |
| shape="circle" | |
| onPress={onDismiss} | |
| > | |
| <Icon as={X} className="size-3.5 text-muted-foreground" /> | |
| </HitTargetWrapper> | |
| <IconButton | |
| icon={X} | |
| accessibilityLabel={dismissAccessibilityLabel ?? "Dismiss"} | |
| variant="ghost" | |
| onPress={onDismiss} | |
| /> |
(swap import { HitTargetWrapper } → import { IconButton } from "@/components/IconButton"; add shape="pill" if you want the round look the circle wrapper gave.)
Since ToastBase is HitTargetWrapper's only consumer, this leaves the atom with zero usages. Its one distinct niche vs. IconButton would be wrapping a non-icon decorative element (drag handle, a chevron that isn't an Icon) in a laid-out 44pt tap zone — but nothing exercises that today. Suggest dropping HitTargetWrapper unless a concrete non-icon consumer is coming in the molecules/organisms waves (YAGNI).
For context on the hitSlop question: a laid-out box (what both atoms use) is a defensible choice over bare hitSlop — hitSlop doesn't render press feedback over the extended region and can overlap a neighbor's slop. So the concept is fine; it's the duplication with IconButton that's the issue here.
Contributor
Author
There was a problem hiding this comment.
Good find ... swapped out to the icon button. Also did an analysis for dupe components and I THINK this is the only one for this PR. I'll keep an eye out for the other ones.
…migration)
Migration verification surface for the ember theme rewrite. Every primitive in
apps/mobile/components/ui/* now has a sidecar Components/{Primitive} story so
the live app components are visually inspectable on iOS Simulator + Android
Emulator under both light and dark ember themes.
Per the vendor-immutable rule, no primitive source file is edited — the stories
import the real components the app already uses. Discovered token-bypass
divergences (shadow-black/5, bg-black/50 backdrops, text-white in destructive
variants) are documented in apps/mobile/components/ui/AUDIT.md with the
upstream-PR resolution path. Local source edits to vendor primitives are
explicitly not the fix.
- 28 stories: accordion, alert, alert-dialog, aspect-ratio, avatar, badge,
button, card, checkbox, collapsible, context-menu, dialog, dropdown-menu,
hover-card, icon (catalog with 22 chat-domain lucide icons), input, label,
menubar, popover, progress, radio-group, select, separator, skeleton, switch,
tabs, text, textarea, toggle, toggle-group, tooltip
- AUDIT.md: 3 token-bypass findings catalogued, accepted as upstream-PR work
- .rnstorybook/preview.tsx: PortalHost added to decorator so primitives that
render via portal (Popover, Dialog, AlertDialog, DropdownMenu, ContextMenu,
Menubar, Select, Tooltip, HoverCard) work in Storybook
- Story conventions match the workflow spec: title='Components/{Name}',
argTypes wired for all controllable props, realistic chat-domain mock data
(model names, session statuses, branch · host disambiguation)
Verified: bun typecheck (exit 0), biome check (no warnings).
This is the migration verification layer for Sprint 01 Phase 0 (token rewrite,
2c21dbc). Pixel-perfect manifest atoms gate stays pending — the gate flips
only after the human reviewer walks the Storybook inventory on simulator.
…udit + stories) Completes the theme-migration story coverage for live first-party app components in apps/mobile/screens/**/components/. Per the audit pass: CRITICAL fixes (active bugs under ember): - OrganizationHeaderButton.tsx:25 — replaced hardcoded `hsl(240 5% 64.9%)` (OLD cool-neutral palette) with `theme.mutedForeground` (warm). Chevron now renders correctly under the warm-neutral surface ramp. MEDIUM fixes (token bypass / wrong source): - SocialButton.tsx — replaced `useColorScheme()` + hardcoded `"white"/"black"` icon color with `useTheme().foreground`. Previously the icon read the OS appearance setting (which may disagree with `Uniwind.setTheme()`); now tracks the active app theme. Google brand colors retained per brand-asset policy. DOCUMENTED exceptions: - SocialButton Google icon brand colors (#4285F4 / #34A853 / #FBBC05 / #EA4335) — kept as official Google brand assets. - OrganizationSwitcherSheet forces `colorScheme: dark` in SwiftUI environment. Documented as design choice; revisit if app supports system light/dark. Story coverage (8 stories) — sidecar to source per existing convention: - OrganizationAvatar — fully presentational; matrix story for sizes 20/28/36/48/64 - SocialButton — both providers; brand-color verification - DevSignInButton — base render only - OrganizationHeaderButton — *Preview wrapper renders inner content* (real component lives inside expo-router Stack.Toolbar — not isolatable) - OrganizationSwitcherSheet — *Inner content* + NativeSheet iOS-only story (real sheet uses @expo/ui/swift-ui — Android renders disclaimer) - OrgDropdown — *Preview wrapper* with mock orgs (real reads authClient + Electric collections — not isolatable) - TabBarAccessory — *Preview wrapper* with mock org (real reads useOrganizations) - AuthenticatedTabBar — placeholder documenting integration dependencies (TabBarView SwiftUI bridge + expo-router/ui Tabs context + Electric) Storybook glob — extended `.rnstorybook/main.js` to include `../screens/**/*.stories.?(ts|tsx|js|jsx)` so screen-component stories are picked up by sb-rn-get-stories. Audit doc — apps/mobile/screens/AUDIT.md catalogs all 8 components, the 2 fixes applied, the 2 documented exceptions, the 6 clean components, and the isolation caveats per integration-heavy component. This completes the migration of the live app component set to the ember theme. The Storybook walkthrough on iOS Simulator + Android Emulator is the verification surface. Verified: bun typecheck (exit 0), biome check (clean).
justincrich
force-pushed
the
chat-mobile-sprint-1-atoms
branch
from
5804853 to
8f564c1
Compare
…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.
Adds 5 chat-specific atom components + stories as foundation for the chat
mobile sprint. Token-driven (state palette, ember accent), 44pt touch targets,
no hardcoded values. These do NOT yet match the designs/atoms/{name}/ HTML
mockups exactly — a follow-up pass will refine each against the design spec
before chat screens consume them.
Why "foundation" not "migration": these are new components, not migration of
existing live app components to the new theme. They were started during the
build pass but the user redirected scope to migration-first; committing as
foundation rather than discarding so the work survives for future refinement.
- components/IconButton/ — Pressable + Icon wrapper with 44pt min hit target,
5 variants (default/primary/secondary/ghost/destructive), 3 sizes.
Real-design refinement TODO: match designs/atoms/icon-button/README.md
variant taxonomy (--ghost is default per spec, also needs --soft and
--neutral variants; pill shape modifier).
- components/Pill/ — chat-domain pill (model chip, mode chip, suggested answer)
with 6 variants (default/selected/warning/danger/success/live) and
interactive/non-interactive split.
Real-design refinement TODO: match designs/atoms/pill/README.md — needs
6 variants per spec (default/strong/accent/live/warning/danger), 3 sizes
with monospace + uppercase modifiers, separate dismiss button.
- components/StatusDot/ — single colored circle (live/warning/danger/success/
neutral + ember). 3 sizes.
Real-design refinement TODO: match designs/atoms/status-dot/README.md —
6px/8px/10px exact sizes (sm 8px is default), live variant needs pulse
animation, warning needs box-shadow ring.
- components/StreamingCursor/ — Reanimated ▌ blink. Configurable duration +
color. accessibilityElementsHidden per spec.
Real-design refinement TODO: match designs/atoms/streaming-cursor/README.md
— 2px width × 1em height with 6px glow shadow, 1s steps(2) animation,
--default / --steady / --paused variants.
- components/ToolStatusRule/ — 3px (default) left rule with running/completed/
failed/pending/neutral/ember variants.
Real-design refinement TODO: match designs/atoms/tool-status-rule/README.md
— 5 status variants (--running/--done/--pending/--error/--neutral), needs
--vertical/--horizontal orientation, glow shadow on running + pending.
All atoms have Components/{Name} stories with argTypes wired. Verified:
bun typecheck (exit 0), biome check (no warnings).
ScrollFade and HitTargetWrapper from the original 7-atom plan are NOT
included — they were not started. Will be built fresh from designs/atoms/
spec when chat screens need them.
Phase 4b BUILD PLAN written to apps/mobile/design/manifest.json (plan
gate → passed for mobile-ios + mobile-android). Scope cut to chat-view
per designs/AUDIT.md; sessions-list/nav surfaces deferred.
Refined against design specs (designs/atoms/<name>/README.md):
- IconButton → 5 variants × 4 sizes × 2 shapes; composes vendor Button
- Pill → 6 variants × 3 sizes; mono/uppercase mods + dismiss
✕ button; composes vendor Badge
- StatusDot → xs/sm/md (6/8/10px); live-pulse + warning-ring halos,
reduced-motion via AccessibilityInfo
- StreamingCursor → default/steady/paused variants; steps(2) emulation;
reduced-motion fallback
- ToolStatusRule → 5 status variants × vertical/horizontal; glow on
running + pending; fixed 3px per spec
Created:
- FabBase — 56/64pt circular FAB; live-ring pulse halo;
composes existing IconButton/Pressable patterns
- HitTargetWrapper — 44pt invisible tap zone; square/circle shapes;
debug-outline modifier
- ProgressDots — 3-dot staggered pulse; 4 variants × 3 sizes;
reduced-motion fallback
- ScrollFade — theme-aware gradient (expo-linear-gradient);
top/bottom × 3 surfaces × 3 sizes; 120ms hide
transition via Reanimated
- ToastBase — 5 variants × inline/stacked; composes
ToolStatusRule + Icon + Text + HitTargetWrapper
Composition priority observed: existing internal components (vendor
Button/Badge/Icon/Text + first-party atoms) → rn-primitives → custom.
NativeWind className preferred over inline style where reasonable; inline
style retained only for Reanimated animated values and RN custom-colored
shadows.
Stories use Components/ prefix per pixel-perfect convention; all props
wired to argTypes controls with descriptive options.
Deps:
- apps/mobile: + expo-linear-gradient@56.0.4 (latest stable for SDK 56)
Verification:
- bun run typecheck (apps/mobile): clean
- bunx biome check (touched files): clean
ScrollFade imported THEME from @/lib/theme to pick the gradient opaque
stop. lib/theme.ts transitively imports DarkTheme/DefaultTheme from
expo-router/react-navigation, which pulls in UnhandledLinkingContext.
Storybook RN does not wrap stories in expo-router's NavigationContainer,
so any story that touches ScrollFade crashed with:
ERROR [Error: Couldn't find an UnhandledLinkingContext context.]
Fix: inline the 3 surface color values (page/soft/overlay × light/dark)
locally in ScrollFade.tsx. Keep in sync with --color-background /
--color-card / --color-popover in global.css. Decouples ScrollFade from
the nav-coupled theme module.
…ents out of args Two prep-time blockers cleared: 1. preview.tsx imported NavigationContainer from expo-router/react-navigation. Storybook 9 RN evaluates preview.tsx during createPreparedStoryMapping (before decorators apply). Loading expo-router's nav module reads the default UnhandledLinkingContext value, whose getters throw "Couldn't find an UnhandledLinkingContext context." Decorators apply at render time, not prep time — so the wrapping I added in commit 0e805a1 was structurally unable to fix the chain. Worse, it BECAME the trigger. Fix: remove the nav import + decorator from preview.tsx. Keep only the neutral View + PortalHost wrapper. Documented why nav can't live here. 2. ToastBase WithAction + StackedWithCTA stories put `<Button>` React elements directly in `args.action`. Storybook tries to JSON-serialize args to persist control state; React elements have circular refs that trigger "cycle in arg" warnings repeatedly. Fix: move the action element into the story's `render` function. Args stay JSON-serializable; the render fn supplies the live React element at render time. Verified: bundler running from apps/mobile cwd; full bundle compiles 21MB HTTP 200; sim screenshot shows DesignSystem "Semantic pairs" story rendering (background/foreground, card/card-foreground swatches visible). No UnhandledLinkingContext error in fresh bundler log; no cycle warning in fresh re-bundles.
justincrich
force-pushed
the
chat-mobile-sprint-1-atoms
branch
from
8f564c1 to
9929588
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
apps/mobile/components/ui/switch.stories.tsx (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync state when
initialCheckedarg changes.Line 16 hydrates once, so changing the Storybook control won’t update the rendered switch.
Proposed fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [checked, setChecked] = useState(initialChecked); + useEffect(() => { + setChecked(initialChecked); + }, [initialChecked]);Also applies to: 16-16
🤖 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/components/ui/switch.stories.tsx` at line 2, The story's local state initialized via useState from the initialChecked arg is only set once, so changing the Storybook control doesn't update the Switch; update the story to synchronize the local state with the incoming initialChecked prop by adding an effect that watches initialChecked (e.g., useEffect(() => setChecked(initialChecked), [initialChecked])) or adopt a controlled pattern, ensuring you reference the existing state setter (setChecked) and the initialChecked arg so the rendered switch updates when the control changes.apps/mobile/components/ui/alert.stories.tsx (1)
10-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve a strict icon key union for story args.
Line 10 makes
keyof typeof ICON_MAPcollapse tostring, so Line 23 no longer constrainsiconto the intended keys.Proposed fix
-const ICON_MAP: Record<string, LucideIcon> = { +const ICON_MAP = { Info: InfoIcon, WifiOff, AlertTriangle, -}; +} as const satisfies Record<string, LucideIcon>;🤖 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/components/ui/alert.stories.tsx` around lines 10 - 24, The icon prop's type collapsed to string because ICON_MAP is typed as Record<string, LucideIcon>; change ICON_MAP to a literal const object so its keys become a literal union and update usages: replace the Record declaration with a const ICON_MAP = { Info: InfoIcon, WifiOff: WifiOff, AlertTriangle: AlertTriangle } as const (or explicitly type it as { Info: LucideIcon; WifiOff: LucideIcon; AlertTriangle: LucideIcon }), then keep AlertShowcase's icon: keyof typeof ICON_MAP so icon is constrained to the intended "Info" | "WifiOff" | "AlertTriangle" union.apps/mobile/components/ui/input.stories.tsx (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep local input state in sync with
initialValuecontrols.Line 18 only seeds local state once, so Storybook
initialValueupdates are ignored after first render.Proposed fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [value, setValue] = useState(initialValue); + useEffect(() => { + setValue(initialValue); + }, [initialValue]);Also applies to: 18-18
🤖 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/components/ui/input.stories.tsx` at line 2, The story seeds a local state with useState only on first render so updates to the Storybook control initialValue are ignored; update the story component to sync local state when the control changes by adding a useEffect that listens to the initialValue prop/control and calls the state setter (e.g., setValue) to update the local value, ensuring the local state (value) and setValue are kept in sync with initialValue from the story controls.apps/mobile/components/ui/textarea.stories.tsx (1)
2-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync local state when
initialValuechanges.
useState(initialValue)only initializes once, so Storybook control updates toinitialValuewon’t refresh the textarea value after mount.Suggested fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [value, setValue] = useState(initialValue); + useEffect(() => { + setValue(initialValue); + }, [initialValue]);🤖 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/components/ui/textarea.stories.tsx` around lines 2 - 18, TextareaShowcase initializes local state with useState(initialValue) so updates to the initialValue control from Storybook won’t propagate; add a useEffect in the TextareaShowcase component that watches the initialValue prop and calls setValue(initialValue) when it changes to keep the textarea value in sync with control updates (keep value/setValue references and only update when initialValue changes).apps/mobile/components/StreamingCursor/StreamingCursor.tsx (1)
87-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp animation segment duration before passing it to
withTiming.Line 92 and Line 94 can pass a negative duration when
durationMsis between 0 and 2, becausehalf - 1drops below zero.💡 Proposed fix
const half = resolvedDuration / 2; + const segmentDuration = Math.max(0, half - 1); opacity.value = 1; opacity.value = withRepeat( withSequence( withTiming(0.1, { duration: 1, easing: Easing.linear }), - withTiming(0.1, { duration: half - 1, easing: Easing.linear }), + withTiming(0.1, { duration: segmentDuration, easing: Easing.linear }), withTiming(1, { duration: 1, easing: Easing.linear }), - withTiming(1, { duration: half - 1, easing: Easing.linear }), + withTiming(1, { duration: segmentDuration, easing: Easing.linear }), ), -1, );In react-native-reanimated v4, what does the official withTiming API specify for duration values below 0ms?🤖 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/components/StreamingCursor/StreamingCursor.tsx` around lines 87 - 95, The sequence can pass negative durations to withTiming when half = resolvedDuration / 2 is < 1; clamp the segment duration before calling withTiming by computing a non-negative value (e.g., const segment = Math.max(0, half - 1)) and use that for the two withTiming calls that currently pass half - 1; update the code that sets opacity.value (the withRepeat/withSequence block using withTiming) to use this clamped segment for both occurrences so no negative duration is ever passed to withTiming.
🤖 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/components/ProgressDots/ProgressDots.tsx`:
- Around line 97-103: The View used as the progressbar container in ProgressDots
should be made an explicit accessibility node by adding accessible={true} so
assistive tech treats the container (not its visually-hidden dot children) as
the focusable element; update the JSX for the element using containerVariants({
size }) / className and spread {...props} to include accessible={true} on that
View and keep the existing accessibilityRole, accessibilityLabel,
accessibilityState and accessibilityLiveRegion props unchanged.
---
Duplicate comments:
In `@apps/mobile/components/StreamingCursor/StreamingCursor.tsx`:
- Around line 87-95: The sequence can pass negative durations to withTiming when
half = resolvedDuration / 2 is < 1; clamp the segment duration before calling
withTiming by computing a non-negative value (e.g., const segment = Math.max(0,
half - 1)) and use that for the two withTiming calls that currently pass half -
1; update the code that sets opacity.value (the withRepeat/withSequence block
using withTiming) to use this clamped segment for both occurrences so no
negative duration is ever passed to withTiming.
In `@apps/mobile/components/ui/alert.stories.tsx`:
- Around line 10-24: The icon prop's type collapsed to string because ICON_MAP
is typed as Record<string, LucideIcon>; change ICON_MAP to a literal const
object so its keys become a literal union and update usages: replace the Record
declaration with a const ICON_MAP = { Info: InfoIcon, WifiOff: WifiOff,
AlertTriangle: AlertTriangle } as const (or explicitly type it as { Info:
LucideIcon; WifiOff: LucideIcon; AlertTriangle: LucideIcon }), then keep
AlertShowcase's icon: keyof typeof ICON_MAP so icon is constrained to the
intended "Info" | "WifiOff" | "AlertTriangle" union.
In `@apps/mobile/components/ui/input.stories.tsx`:
- Line 2: The story seeds a local state with useState only on first render so
updates to the Storybook control initialValue are ignored; update the story
component to sync local state when the control changes by adding a useEffect
that listens to the initialValue prop/control and calls the state setter (e.g.,
setValue) to update the local value, ensuring the local state (value) and
setValue are kept in sync with initialValue from the story controls.
In `@apps/mobile/components/ui/switch.stories.tsx`:
- Line 2: The story's local state initialized via useState from the
initialChecked arg is only set once, so changing the Storybook control doesn't
update the Switch; update the story to synchronize the local state with the
incoming initialChecked prop by adding an effect that watches initialChecked
(e.g., useEffect(() => setChecked(initialChecked), [initialChecked])) or adopt a
controlled pattern, ensuring you reference the existing state setter
(setChecked) and the initialChecked arg so the rendered switch updates when the
control changes.
In `@apps/mobile/components/ui/textarea.stories.tsx`:
- Around line 2-18: TextareaShowcase initializes local state with
useState(initialValue) so updates to the initialValue control from Storybook
won’t propagate; add a useEffect in the TextareaShowcase component that watches
the initialValue prop and calls setValue(initialValue) when it changes to keep
the textarea value in sync with control updates (keep value/setValue references
and only update when initialValue changes).
🪄 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: 95cee91f-f066-4dfc-8d09-80321138e1b6
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (75)
apps/mobile/.rnstorybook/preview.tsxapps/mobile/components/FabBase/FabBase.stories.tsxapps/mobile/components/FabBase/FabBase.tsxapps/mobile/components/FabBase/index.tsapps/mobile/components/HitTargetWrapper/HitTargetWrapper.stories.tsxapps/mobile/components/HitTargetWrapper/HitTargetWrapper.tsxapps/mobile/components/HitTargetWrapper/index.tsapps/mobile/components/IconButton/IconButton.stories.tsxapps/mobile/components/IconButton/IconButton.tsxapps/mobile/components/IconButton/index.tsapps/mobile/components/Pill/Pill.stories.tsxapps/mobile/components/Pill/Pill.tsxapps/mobile/components/Pill/index.tsapps/mobile/components/ProgressDots/ProgressDots.stories.tsxapps/mobile/components/ProgressDots/ProgressDots.tsxapps/mobile/components/ProgressDots/index.tsapps/mobile/components/ScrollFade/ScrollFade.stories.tsxapps/mobile/components/ScrollFade/ScrollFade.tsxapps/mobile/components/ScrollFade/index.tsapps/mobile/components/StatusDot/StatusDot.stories.tsxapps/mobile/components/StatusDot/StatusDot.tsxapps/mobile/components/StatusDot/index.tsapps/mobile/components/StreamingCursor/StreamingCursor.stories.tsxapps/mobile/components/StreamingCursor/StreamingCursor.tsxapps/mobile/components/StreamingCursor/index.tsapps/mobile/components/ToastBase/ToastBase.stories.tsxapps/mobile/components/ToastBase/ToastBase.tsxapps/mobile/components/ToastBase/index.tsapps/mobile/components/ToolStatusRule/ToolStatusRule.stories.tsxapps/mobile/components/ToolStatusRule/ToolStatusRule.tsxapps/mobile/components/ToolStatusRule/index.tsapps/mobile/components/ui/AUDIT.mdapps/mobile/components/ui/accordion.stories.tsxapps/mobile/components/ui/alert-dialog.stories.tsxapps/mobile/components/ui/alert.stories.tsxapps/mobile/components/ui/aspect-ratio.stories.tsxapps/mobile/components/ui/avatar.stories.tsxapps/mobile/components/ui/badge.stories.tsxapps/mobile/components/ui/button.stories.tsxapps/mobile/components/ui/card.stories.tsxapps/mobile/components/ui/checkbox.stories.tsxapps/mobile/components/ui/collapsible.stories.tsxapps/mobile/components/ui/context-menu.stories.tsxapps/mobile/components/ui/dialog.stories.tsxapps/mobile/components/ui/dropdown-menu.stories.tsxapps/mobile/components/ui/hover-card.stories.tsxapps/mobile/components/ui/icon.stories.tsxapps/mobile/components/ui/input.stories.tsxapps/mobile/components/ui/label.stories.tsxapps/mobile/components/ui/menubar.stories.tsxapps/mobile/components/ui/popover.stories.tsxapps/mobile/components/ui/progress.stories.tsxapps/mobile/components/ui/radio-group.stories.tsxapps/mobile/components/ui/select.stories.tsxapps/mobile/components/ui/separator.stories.tsxapps/mobile/components/ui/skeleton.stories.tsxapps/mobile/components/ui/switch.stories.tsxapps/mobile/components/ui/tabs.stories.tsxapps/mobile/components/ui/text.stories.tsxapps/mobile/components/ui/textarea.stories.tsxapps/mobile/components/ui/toggle-group.stories.tsxapps/mobile/components/ui/toggle.stories.tsxapps/mobile/components/ui/tooltip.stories.tsxapps/mobile/package.jsonapps/mobile/screens/(auth)/sign-in/components/DevSignInButton/DevSignInButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.stories.tsxapps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/OrganizationSwitcherSheet.stories.tsxapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.stories.tsxapps/mobile/screens/(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.stories.tsxapps/mobile/screens/(authenticated)/components/OrgDropdown/OrgDropdown.stories.tsxapps/mobile/screens/(authenticated)/components/TabBarAccessory/TabBarAccessory.stories.tsxapps/mobile/screens/AUDIT.md
✅ Files skipped from review due to trivial changes (10)
- apps/mobile/components/ProgressDots/index.ts
- apps/mobile/components/StreamingCursor/index.ts
- apps/mobile/components/FabBase/index.ts
- apps/mobile/components/Pill/index.ts
- apps/mobile/components/ScrollFade/index.ts
- apps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.tsx
- apps/mobile/components/ToolStatusRule/index.ts
- apps/mobile/components/StatusDot/index.ts
- apps/mobile/screens/AUDIT.md
- apps/mobile/components/ui/AUDIT.md
Comment on lines
+97
to
+103
| <View | ||
| accessibilityRole="progressbar" | ||
| accessibilityLabel={accessibilityLabel} | ||
| accessibilityState={{ busy: true }} | ||
| accessibilityLiveRegion="polite" | ||
| className={cn(containerVariants({ size }), className)} | ||
| {...props} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ProgressDots/ProgressDots.tsx"
echo "== File =="; echo "$FILE"
echo
echo "== Lines 70-140 =="
nl -ba "$FILE" | sed -n '70,140p'
echo
echo "== rg accessibility props in file =="
rg -n "accessibilityRole|accessibilityLabel|accessibilityState|accessibilityLiveRegion|accessible" "$FILE"Repository: superset-sh/superset
Length of output: 209
🌐 Web query:
React Native accessibleprop required when usingaccessibilityRole="progressbar" screen reader focusable element
💡 Result:
In React Native, while the accessible prop is not strictly required by the framework's compiler to use accessibilityRole="progressbar", it is a critical best practice to set accessible={true} on the component [1][2]. When you define an element with accessibilityRole="progressbar" (or the modern role="progressbar"), you are signaling to assistive technologies that the element represents a progress indicator [3][4]. Setting accessible={true} ensures that the container is treated as a single, focusable unit by the screen reader, allowing it to correctly announce the progress information provided by the accessibilityValue prop [1][2][5]. Key requirements for a fully accessible progress bar in React Native include: 1. accessibilityRole="progressbar" (or role="progressbar"): Identifies the element as a progress bar to the screen reader [3][4][2]. 2. accessible={true}: Ensures the element is recognized as a single accessible component and is focusable by assistive technology [1][2]. 3. accessibilityValue: Essential for communicating the progress state. This should include min, max, and now values (e.g., {min: 0, max: 100, now: 50}) [6][7][1]. 4. accessibilityLabel: Provides a descriptive label (e.g., "Uploading file") so the user understands what the progress bar represents [7][2]. Without setting accessible={true}, some platforms or screen readers may fail to correctly focus the element or properly announce the accessibilityValue state changes [5]. Therefore, it should be considered a functional requirement for accessibility compliance [1][2].
Citations:
- 1: https://github.com/callstack/react-native-paper/blob/78b2d750/src/components/__tests__/__snapshots__/ProgressBar.test.tsx.snap
- 2: https://developer.amazon.com/docs/react-native-vega/0.72/accessibility.html
- 3: https://reactnative.dev/docs/accessibility
- 4: https://reactnative.dev/docs/accessibility.md
- 5: https://www.accessibilitychecker.org/blog/react-native-accessibility/
- 6: https://nehasharma.dev/posts/react-native-accessibility-guide
- 7: https://compound.thephoenixgroup.com/latest/components/components/progress-bar/accessibility-54bpyNtu-54bpyNtu
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ProgressDots/ProgressDots.tsx"
echo "== wc -l =="
wc -l "$FILE"
echo
echo "== Lines 80-130 (context) =="
sed -n '80,130p' "$FILE" | cat -n
echo
echo "== rg accessibility props in file =="
rg -n "accessibilityRole|accessibilityLabel|accessibilityState|accessibilityLiveRegion|accessible|accessibilityValue" "$FILE"Repository: superset-sh/superset
Length of output: 2040
🌐 Web query:
React Native docs: accessibleprop purpose and default behavior whenaccessibilityRole/accessibilityLabel are set on a View
💡 Result:
In React Native, the accessible prop is a boolean used to indicate whether a View should be discoverable and treated as a single, unified element by assistive technologies like screen readers [1][2][3]. When you set accessible={true} on a View, it informs the underlying platform (mapping to isAccessibilityElement on iOS and focusable on Android) that the component—and its contained children—should be grouped together as one interactive, focusable unit [1][3][4]. Without explicitly setting accessible={true}, screen readers will navigate through the individual children within that View, provided those children are themselves accessible [1][5]. Regarding the interaction between these props: 1. Grouping Behavior: Setting accessible={true} effectively hides individual child elements from assistive technologies, treating the parent View as the sole focusable entity [6][4][5]. This is useful for combining related text or visual elements into a single meaningful description [6][4]. 2. Role and Label Context: When you set accessibilityRole and accessibilityLabel on a View, they describe the purpose and content of that component [1][6][5]. If you set these props but do not set accessible={true}, the screen reader may still be able to focus the individual children if they contain accessible content, or the parent View might be ignored entirely depending on the platform's heuristics [1][3]. 3. Default Behavior: By default, touchable elements (e.g., TouchableOpacity, Button) are inherently accessible (accessible={true}) [1][2][3]. For standard View components, they are not accessible by default unless you explicitly set the accessible prop or attach specific accessibility metadata that triggers focusability [1][3][5]. In summary, use accessible={true} when you want to override the default "child-by-child" navigation behavior and force the OS to treat the View as a single semantic unit, applying the accessibilityLabel and accessibilityRole to that entire group [6][4].
Citations:
- 1: https://reactnative.dev/docs/next/accessibility
- 2: https://reactnative.dev/docs/accessibility.md
- 3: https://reactnative.dev/docs/accessibility
- 4: https://www.accessibilitychecker.org/blog/react-native-accessibility/
- 5: https://stackoverflow.com/questions/61914642/when-should-we-use-accessibilityrole-in-react-native
- 6: https://oneuptime.com/blog/post/2026-01-15-react-native-accessible-components/view
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile/components/ProgressDots/ProgressDots.tsx"
echo "== rg accessibility* in file =="
rg -n "accessibility" "$FILE"
echo
echo "== Lines 1-120 =="
sed -n '1,120p' "$FILE"
echo
echo "== Lines 120-200 =="
sed -n '120,200p' "$FILE"Repository: superset-sh/superset
Length of output: 4348
🌐 Web query:
React Native: does setting accessibilityRole and accessibilityLabel on a plain View make it accessible/focusable even without setting the accessible prop?
💡 Result:
In React Native, setting accessibilityRole or accessibilityLabel on a plain View does not automatically make the component accessible or focusable by screen readers [1][2][3]. To make a View discoverable by assistive technologies, you must explicitly set the accessible prop to true [2][3]. When accessible={true} is missing: - The component is treated as a non-accessibility element by the underlying operating system, regardless of whether other accessibility props are present [4][2]. - The accessibilityLabel and accessibilityRole are ignored because the system does not recognize the element as an accessibility node [4]. To ensure an element is accessible, you should: 1. Set accessible={true} on the View [2][5]. 2. Provide an accessibilityLabel to describe the element's content [1][4]. 3. Provide an accessibilityRole to communicate the element's purpose [1][5]. Note that touchable components (like TouchableOpacity or Pressable) are accessible by default, so they do not require the explicit accessible prop to be focusable [2][3][6]. However, for standard View components, the explicit prop is necessary [2][3].
Citations:
- 1: https://reactnative.dev/docs/next/accessibility
- 2: https://reactnative.dev/docs/accessibility.md
- 3: https://reactnative.dev/docs/view
- 4: https://www.accessibilitychecker.org/blog/react-native-accessibility/
- 5: https://oneuptime.com/blog/post/2026-01-15-react-native-accessible-components/view
- 6: https://stackoverflow.com/questions/61914642/when-should-we-use-accessibilityrole-in-react-native
Make the progressbar container explicitly accessible.
accessibilityRole/accessibilityLabel on a plain React Native View may not be treated as an accessibility node unless accessible is set. Since the dot children are hidden from assistive tech, the container should be the focusable element.
🔧 Proposed fix
<View
+ accessible
accessibilityRole="progressbar"
accessibilityLabel={accessibilityLabel}
accessibilityState={{ busy: true }}
accessibilityLiveRegion="polite"
className={cn(containerVariants({ size }), className)}
{...props}📝 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
| <View | |
| accessibilityRole="progressbar" | |
| accessibilityLabel={accessibilityLabel} | |
| accessibilityState={{ busy: true }} | |
| accessibilityLiveRegion="polite" | |
| className={cn(containerVariants({ size }), className)} | |
| {...props} | |
| <View | |
| accessible | |
| accessibilityRole="progressbar" | |
| accessibilityLabel={accessibilityLabel} | |
| accessibilityState={{ busy: true }} | |
| accessibilityLiveRegion="polite" | |
| className={cn(containerVariants({ size }), className)} | |
| {...props} |
🤖 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/components/ProgressDots/ProgressDots.tsx` around lines 97 - 103,
The View used as the progressbar container in ProgressDots should be made an
explicit accessibility node by adding accessible={true} so assistive tech treats
the container (not its visually-hidden dot children) as the focusable element;
update the JSX for the element using containerVariants({ size }) / className and
spread {...props} to include accessible={true} on that View and keep the
existing accessibilityRole, accessibilityLabel, accessibilityState and
accessibilityLiveRegion props unchanged.
…e behavior HitTargetWrapper was a 44pt Pressable wrapper with no consumers beyond ToastBase's dismiss button. IconButton already provides the same 44pt touch target plus hitSlop, variants, sizes, shapes, and loading state. ToastBase dismiss now uses IconButton (variant=ghost, shape=pill, size=xs) instead of HitTargetWrapper + Icon composition.
Contributor
Author
Atom Component Audit — 10 → 9 AtomsAudited all 10 atoms against rnreusable base components ( Verdict
Changes in this commit
Result: 9 atoms, 0 superficial wrappers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wave 1 of the chat-mobile UI build: 10 chat-domain atom components built on the ember theme from PR #4874 and the ported component foundation from PR #4875. Each atom has a folder co-located with its `.tsx`, `.stories.tsx` (with full argTypes controls), and `index.ts` barrel.
Refined (existing atoms updated to spec):
Created (new atoms):
Flips `atoms` gate to `passed` in `apps/mobile/design/manifest.json` for both mobile-ios and mobile-android platforms.
Stack position — 3 of 5
Depends on: PR #4875 (ported) → PR #4874 (tooling)
This branch contains PRs 1–2's commits + this PR's 5 commits. After PRs 1–2 merge, this PR will be rebased onto fresh `main` and the diff will shrink to atoms-only.
Test plan
Summary by CodeRabbit
New Features
Chores