feat: Overhaul "expo-stylesheet" Example App #105
Conversation
…nents in expo-stylesheet app
…nts in expo-stylesheet app
… in expo-stylesheet app
…t-menu', 'dialog'
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a large set of themed, cross-platform UI primitive wrappers and many demo screens to the expo-stylesheet app; introduces a Zustand color-scheme store and ThemeToggle, centralizes typed theme tokens, refactors/extends select and other components, removes several icon re-exports, updates app shell (layout, index, not-found), and adjusts Babel/p npm scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ThemeToggle
participant ColorStore as useColorScheme (Zustand)
participant Layout as _layout.tsx
User->>ThemeToggle: press toggle
ThemeToggle->>ColorStore: setColorScheme + persist (AsyncStorage)
ColorStore-->>Layout: colorScheme updated (store subscription)
Layout->>User: re-render with selected theme (DARK / LIGHT)
sequenceDiagram
autonumber
actor User
participant Screen as Demo Screen
participant Trigger
participant Portal as Portal/Overlay
User->>Trigger: activate (press/long-press/right-click)
Trigger-->>Portal: open()
Portal-->>User: render content (menu/dialog/popover)
User->>Portal: interact (select/toggle)
Portal-->>Screen: update local state (may keep open)
User->>Portal: dismiss
Portal-->>User: close
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 56
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/expo-stylesheet/app/_layout.tsx (1)
96-107: HardentoOptionsagainst non-string children.
headerTitlecan pass a non-string node; current implementation would throw. Make it tolerant.-function toOptions(name: string) { - const title = name +function toOptions(name: unknown) { + const raw = typeof name === 'string' ? name : String(name ?? '') + const title = raw .replace('(components)/', '') .split('-') .map(function (str: string) { return str.replace(/\b\w/g, function (char) { return char.toUpperCase(); }); }) .join(' '); return title; }
🧹 Nitpick comments (78)
apps/expo-stylesheet/app/+not-found.tsx (2)
12-12: Prefer plain apostrophe over HTML entity in RN text.Use a literal apostrophe; entities can be needlessly opaque in RN.
- <Text style={{ color: colors.text }}>This screen doesn't exist.</Text> + <Text style={{ color: colors.text }}>This screen doesn't exist.</Text>
15-15: Optional: prevent returning to the 404 page.Use
replaceso “home” replaces the not-found entry in history.- <Link href='/'> + <Link href='/' replace> <Text style={{ color: colors.text }}>Go to home screen!</Text> </Link>package.json (1)
8-8: Script addition looks good; consider consistent Turbo flags.Matches the PR objective. Optional: align with neighboring scripts by adding an explicit concurrency or reusing a package filter token (not required).
apps/expo-stylesheet/lib/useColorScheme.tsx (3)
4-14: Sync with system theme changes.Without a listener, switching OS theme won’t update the store until app reload.
Apply this diff to subscribe/unsubscribe inside the hook:
export function useColorScheme() { const colorScheme = useColorSchemeStore((state) => state.colorScheme); const setColorScheme = useColorSchemeStore((state) => state.setColorScheme); const toggleColorScheme = useColorSchemeStore((state) => state.toggleColorScheme); + React.useEffect(() => { + const sub = Appearance.addChangeListener(({ colorScheme }) => { + setColorScheme((colorScheme ?? 'dark') as 'light' | 'dark'); + }); + return () => sub.remove(); + }, [setColorScheme]); + return { colorScheme, setColorScheme, toggleColorScheme, }; }And add the import:
-import { Appearance } from 'react-native'; +import { Appearance } from 'react-native'; +import * as React from 'react';
22-29: Optional: Persist theme in the store instead of ad-hoc in components.Moving persistence into Zustand improves consistency and removes UI coupling.
Apply this diff (uses AsyncStorage across native/web):
-import { create } from 'zustand'; +import { create } from 'zustand'; +import { persist, createJSONStorage } from 'zustand/middleware'; +import AsyncStorage from '@react-native-async-storage/async-storage'; @@ -export const useColorSchemeStore = create<IUseColorSchemeStore>((set) => ({ - colorScheme: Appearance.getColorScheme() ?? 'dark', - setColorScheme: (colorScheme) => set({ colorScheme }), - toggleColorScheme: () => - set((state) => ({ - colorScheme: state.colorScheme === 'dark' ? 'light' : 'dark', - })), -})); +export const useColorSchemeStore = create<IUseColorSchemeStore>()( + persist( + (set, get) => ({ + colorScheme: Appearance.getColorScheme() ?? 'dark', + setColorScheme: (colorScheme) => set({ colorScheme }), + toggleColorScheme: () => + set((state) => ({ + colorScheme: state.colorScheme === 'dark' ? 'light' : 'dark', + })), + }), + { + name: 'theme', + storage: createJSONStorage(() => AsyncStorage), + version: 1, + } + ) +);
4-4: Rename custom hook to avoid collision with RN’s useColorScheme.No imports from ‘react-native’ were found in apps/expo-nativewind/lib/useColorScheme.tsx; consider renaming this hook (e.g. to useAppColorScheme) to prevent future confusion.
apps/expo-stylesheet/components/ThemeToggle.tsx (2)
25-28: Prevent 1px icon layout shift.Use the same size for both icons to avoid subtle jitter.
Apply this diff:
- <MoonStar color={colors.text} size={23} strokeWidth={1.25} /> + <MoonStar color={colors.text} size={24} strokeWidth={1.25} />
36-43: Header layout: avoidflex: 1expansion.In navigation headers,
flex: 1can overtake space. Prefer fixed touch target.Apply this diff:
iconContainer: { - flex: 1, - aspectRatio: 1, - paddingTop: 2, + width: 36, + height: 36, + paddingTop: 2, justifyContent: 'center', alignItems: 'flex-start', paddingHorizontal: Platform.OS === 'web' ? 20 : 0, },apps/expo-stylesheet/components/ui/hover-card.tsx (2)
32-43: Add exit animation for symmetry.Smooth fade-out improves UX and matches the entering animation.
Apply this diff:
-import Animated, { FadeIn } from 'react-native-reanimated'; +import Animated, { FadeIn, FadeOut } from 'react-native-reanimated'; @@ - <Animated.View entering={FadeIn}> + <Animated.View entering={FadeIn} exiting={FadeOut}>
17-25: Memoize flattened styles to avoid recomputing on every render.Small but cheap optimization when re-rendering frequently.
Apply this diff:
- const flattenContentStyles = StyleSheet.flatten([ - styles.base, - { - borderColor: colors.border, - backgroundColor: colors.card, - shadowColor: colors.text, - }, - style, - ]); + const flattenContentStyles = React.useMemo( + () => + StyleSheet.flatten([ + styles.base, + { + borderColor: colors.border, + backgroundColor: colors.card, + shadowColor: colors.text, + }, + style, + ]), + [colors.border, colors.card, colors.text, style] + );apps/expo-stylesheet/lib/constants.ts (1)
100-100: Prefer explicit Theme import over ambient ReactNavigation namespaceAvoid relying on ambient
ReactNavigation.Theme(can fail under isolatedModules/TS config). Import the type and use it directly.+import type { Theme } from '@react-navigation/native'; -export type ICustomTheme = ReactNavigation.Theme & { colors: ICustomThemeColor }; +export type ICustomTheme = Theme & { colors: ICustomThemeColor };apps/expo-stylesheet/app/(components)/slider.tsx (2)
40-43: Avoid manually constraining Range width; primitive already reflects valueWrapping
Slider.Rangein a width-limited View can double-apply progress on web and diverge from native behavior.- <View style={{ width: `${value}%`, height: '100%' }}> - <Slider.Range style={flattenRangeStyle} /> - </View> + <Slider.Range style={flattenRangeStyle} />
10-18: Micro: memoize flattened stylesMinor CPU churn on every render. Memoize by
[colors, value].- const flattenTrackStyle = StyleSheet.flatten([ + const flattenTrackStyle = React.useMemo(() => StyleSheet.flatten([ styles.track, { backgroundColor: colors.accent, borderColor: colors.border }, - ]); - const flattenRangeStyle = StyleSheet.flatten([styles.range, { backgroundColor: colors.primary }]); - const flattenThumbStyle = StyleSheet.flatten([ + ]), [colors]); + const flattenRangeStyle = React.useMemo( + () => StyleSheet.flatten([styles.range, { backgroundColor: colors.primary }]), + [colors] + ); + const flattenThumbStyle = React.useMemo(() => StyleSheet.flatten([ styles.thumb, - { backgroundColor: colors.primary, left: `${value}%` }, - ]); + { backgroundColor: colors.primary }, + Platform.OS === 'web' ? ({ left: `${value}%` } as const) : null, + ]), [colors, value]);apps/expo-stylesheet/components/ui/switch.tsx (1)
52-62: Translate distance is hard-coded to 18 (native)This breaks if height/width are customized. Consider computing from layout.
I can wire
onLayoutto read widths and settranslateX = trackWidth - thumbWidth - 2 * borderWidth.apps/expo-stylesheet/components/ui/card.tsx (2)
88-96: Consider Android elevation for parity with iOS/Web shadows.Optional: add a small
elevationon Android to make shadows visible.card: { borderRadius: 8, borderWidth: 1, shadowOpacity: 0.1, shadowOffset: { width: 0, height: 1 }, shadowRadius: 2, + // Android + elevation: 2, },
31-33: Optionally theme header/footer surfaces.Minor consistency nit: Header/Footer don’t currently use theme colors (e.g., border/background) unlike Card. If desired, merge
colors.border/colors.cardfor visual consistency.Also applies to: 81-83
apps/expo-stylesheet/components/ui/progress.tsx (1)
42-43: Don’t captureindicatorStyleinside the worklet.Pass static styles outside the animated closure to avoid non-serializable captures and keep UI-thread worklets minimal.
(Already reflected in the unified diff above by moving
indicatorStyleinto the style array ofAnimated.View.)Also applies to: 52-54
apps/expo-stylesheet/components/ui/table.tsx (2)
137-140: AvoidverticalAlign: 'middle'on non-Text elements.
verticalAlignis not cross-platform for RN Views and may be ignored. Consider aligning content via layout.cell: { padding: 16, - verticalAlign: 'middle', + // Align content vertically via layout + justifyContent: 'center', },If
Cellrenders Text, applytextAlignVertical: 'center'(Android) or adjustlineHeighton the inner Text instead.
110-116:fontSizeon the root table View may not affect cells.RN doesn’t inherit
fontSizeto nested Text by default. If you intended a default text size, wrap relevant sections inTextClassContext.Provideror set sizes per cell/head.apps/expo-stylesheet/components/ui/select.tsx (2)
130-139: Unify pressed/hovered styles across platforms (web currently lacks pressed feedback).On web,
Itemcan accept a Pressable-style callback too. Using the same callback improves parity.- const webIndicatorStyle = StyleSheet.flatten([styles.item, disabled && { opacity: 0.5 }, style]); - const nativeIndicatorStyle = ({ pressed }: { pressed: boolean }) => [ + const itemStyle = ({ pressed, hovered }: { pressed: boolean; hovered: boolean }) => [ styles.item, disabled && { opacity: 0.5 }, - pressed && { backgroundColor: colors.accent }, + (pressed || hovered) && { backgroundColor: colors.accent }, style, ]; ... - style={Platform.OS === 'web' ? webIndicatorStyle : nativeIndicatorStyle} + style={itemStyle}Also applies to: 142-154
31-37: Minor: avoid empty fragment around children.
<>{children}</>is redundant here.- <>{children}</> + {children}apps/expo-stylesheet/app/(components)/select.tsx (2)
78-85: Add accessibility to the floating opener.This invisible opener should be discoverable by assistive tech.
floatingButton: { position: 'absolute', top: 0, right: 0, width: 64, height: 64, },And on the element:
- <Pressable + <Pressable + accessibilityRole="button" + accessibilityLabel="Open fruit selector" style={styles.floatingButton} onPress={() => { triggerRef.current?.open(); }} />
38-45: Inline width styles could move to StyleSheet for consistency.Not required, but keeps styling centralized.
apps/expo-stylesheet/components/ui/aspect-ratio.tsx (1)
1-5: Prefer direct re-export (and props type) to reduce indirectionSlightly cleaner and tree-shake-friendly. If the props type is exported upstream, expose it too.
-import * as AspectRatioPrimitive from '@rn-primitives/aspect-ratio'; - -const AspectRatio = AspectRatioPrimitive.Root; - -export { AspectRatio }; +export { Root as AspectRatio } from '@rn-primitives/aspect-ratio'; +export type { RootProps as AspectRatioProps } from '@rn-primitives/aspect-ratio';apps/expo-stylesheet/app/(components)/aspect-ratio.tsx (1)
11-12: Minor cleanups: drop one-element style array; consider theming the colors
- style={[styles.innerBox]} → style={styles.innerBox}
- Optional: pull colors from theme/tokens instead of hard-coded blue/white for dark mode parity.
- <View style={[styles.innerBox]}> - <H1 style={{ color: '#fff' }}>16:9</H1> + <View style={styles.innerBox}> + <H1 /* consider theme color */ style={{ color: '#fff' }}>16:9</H1>Also applies to: 30-31
apps/expo-stylesheet/app/(components)/toggle-group.tsx (2)
12-20: Add native accessibility props in addition to aria for webEnsure VoiceOver/TalkBack announce state. Keep aria-label for web; add accessibilityLabel and accessibilityState for native.
- <ToggleGroupItem value='bold' aria-label='Toggle bold'> + <ToggleGroupItem + value='bold' + aria-label='Toggle bold' + accessibilityLabel='Toggle bold' + accessibilityState={{ selected: value.includes('bold') }} + > <ToggleGroupIcon icon={Bold} size={18} /> </ToggleGroupItem> - <ToggleGroupItem value='italic' aria-label='Toggle italic'> + <ToggleGroupItem + value='italic' + aria-label='Toggle italic' + accessibilityLabel='Toggle italic' + accessibilityState={{ selected: value.includes('italic') }} + > <ToggleGroupIcon icon={Italic} size={18} /> </ToggleGroupItem> - <ToggleGroupItem value='underline' aria-label='Toggle underline'> + <ToggleGroupItem + value='underline' + aria-label='Toggle underline' + accessibilityLabel='Toggle underline' + accessibilityState={{ selected: value.includes('underline') }} + > <ToggleGroupIcon icon={Underline} size={18} /> </ToggleGroupItem>
31-33: Redundant gap on containerContainer has a single child; gap has no effect. Remove it.
- gap: 48,apps/expo-stylesheet/app/(components)/switch.tsx (1)
27-28: Redundant gap on containerSingle child; gap is unnecessary. Safe to remove.
- gap: 48,apps/expo-stylesheet/app/(components)/accordion.tsx (1)
15-15: Remove one-element style arrayNo need for array when passing a single style object.
- <View style={[styles.container]}> + <View style={styles.container}>apps/expo-stylesheet/app/(components)/avatar.tsx (2)
11-11: Prevent layout shift: give the image an explicit size.
Fixes minor CLS on first paint and keeps the fallback/avatar box consistent.- <AvatarImage source={{ uri: GITHUB_AVATAR_URI }} /> + <AvatarImage source={{ uri: GITHUB_AVATAR_URI }} style={{ width: 96, height: 96 }} />
20-27: Portable spacing: be mindful ofgapon older RN runtimes.
If you must support older SDKs/devices, consider a row wrapper or margin utilities as a fallback; otherwise fine to keep.apps/expo-stylesheet/app/_layout.tsx (1)
63-91: Minor: avoid recreating inline header styles each render.
Cache withReact.useMemoif header re-renders become hot; optional at this scale.apps/expo-stylesheet/app/(components)/progress.tsx (2)
10-12: Clamp and cover full 0–100 range.
Random now yields 0–99. Also guard bounds.- function onPress() { - setProgress(Math.floor(Math.random() * 100)); - } + function onPress() { + const next = Math.floor(Math.random() * 101); + setProgress(Math.max(0, Math.min(100, next))); + }
17-18: Be explicit about max for readability.
Makes the progress contract obvious across platforms.- <Progress value={progress} style={styles.progress} /> + <Progress value={progress} max={100} style={styles.progress} />apps/expo-stylesheet/app/(components)/radio-group.tsx (2)
2-2: Import Platform for platform-specific label behavior.
Required for the change below.-import { View, StyleSheet } from 'react-native'; +import { View, StyleSheet, Platform } from 'react-native';
33-39: Let web use native label behavior; keep onPress for native.
Aligns with your Checkbox pattern and improves semantics on web.- <Label nativeID={`label-for-${value}`} onPress={onLabelPress}> + <Label + nativeID={`label-for-${value}`} + onPress={Platform.select({ web: undefined, default: onLabelPress })} + > {value} </Label>apps/expo-stylesheet/app/(components)/checkbox.tsx (1)
30-42: Optional: unique IDs if this pattern scales.
If multiple checkboxes render on a screen, generate IDs (e.g., via a counter or value slug) to avoid collisions.apps/expo-stylesheet/app/(components)/toggle.tsx (1)
6-6: Typo and naming consistency: rename component to ToggleScreenKeep screen component names consistent with others (SeparatorScreen, TabsScreen, etc.) and fix the typo.
-export default function ToggleUniversalcreen() { +export default function ToggleScreen() {apps/expo-stylesheet/app/(components)/tabs.tsx (2)
47-49: Use aria-labelledby instead of id for cross‑platform labelingRN doesn’t support id on native; align with the Name field’s pattern for accessibility consistency.
- <Label nativeID='username'>Username</Label> - <Input id='username' defaultValue='@peduarte' /> + <Label nativeID='username'>Username</Label> + <Input aria-labelledby='username' defaultValue='@peduarte' />
2-2: Avoid 'auto' margins on nativemarginHorizontal: 'auto' is web-only. Gate it by platform to prevent native warnings.
-import { View, StyleSheet } from 'react-native'; +import { View, StyleSheet, Platform } from 'react-native';tabs: { width: '100%', maxWidth: 400, - marginHorizontal: 'auto', + ...Platform.select({ web: { marginHorizontal: 'auto' } }), flexDirection: 'column', gap: 6, },Also applies to: 96-103
apps/expo-stylesheet/app/(components)/tooltip.tsx (1)
20-26: Floating Pressable is invisible and not accessibleThis control intercepts taps/clicks without any affordance or a11y semantics. Either hide it from a11y or make it an actual, labeled button.
- <Pressable - style={styles.floatingButton} - onPress={() => { + <Pressable + style={styles.floatingButton} + accessibilityRole='button' + accessibilityLabel='Open tooltip' + hitSlop={8} + testID='open-tooltip' + onPress={() => { // open programmatically triggerRef.current?.open(); }} />apps/expo-stylesheet/app/(components)/hover-card.tsx (1)
73-79: Floating Pressable lacks a11y semanticsSame as the tooltip screen: it’s invisible and unlabeled. Add a11y or make it decorative.
- <Pressable - style={styles.floatingButton} + <Pressable + style={styles.floatingButton} + accessibilityRole='button' + accessibilityLabel='Open hover card' + hitSlop={8} + testID='open-hover-card' onPress={() => { // open programmatically triggerRef.current?.open(); }} />apps/expo-stylesheet/components/ui/collapsible.tsx (1)
1-9: Minor: prefer direct re-exports for terseness and tree-shakingFunctionally identical, but avoids intermediate constants and can help DCE.
-import * as CollapsiblePrimitive from '@rn-primitives/collapsible'; - -const Collapsible = CollapsiblePrimitive.Root; - -const CollapsibleTrigger = CollapsiblePrimitive.Trigger; - -const CollapsibleContent = CollapsiblePrimitive.Content; - -export { Collapsible, CollapsibleTrigger, CollapsibleContent }; +export { + Root as Collapsible, + Trigger as CollapsibleTrigger, + Content as CollapsibleContent, +} from '@rn-primitives/collapsible';apps/expo-stylesheet/lib/utils.ts (1)
21-35: Make “array-like” detection more robust and document fallbackRelying only on key '0' is brittle. Check for consecutive numeric keys and ignore plain style objects that coincidentally have '0'.
- if (keysArray.includes('0')) return true; + const hasIndex0 = keysArray.includes('0'); + const allNumeric = keysArray.length > 0 && keysArray.every(k => String(+k) === k); + if (hasIndex0 && allNumeric) return true; return false;apps/expo-stylesheet/app/(components)/dropdown-menu.tsx (2)
35-35: Type-safe programmatic open on Trigger refAugment the ref type so
.open()is known to TS across platforms.- const triggerRef = React.useRef<React.ElementRef<typeof DropdownMenuTrigger>>(null); + const triggerRef = React.useRef< + (React.ElementRef<typeof DropdownMenuTrigger> & { open: () => void }) | null + >(null);
48-55: Invisible Pressable lacks a11y and may intercept unintended tapsAdd accessible props (and a small hitSlop) so it’s discoverable and intentional.
- <Pressable - style={styles.floatingButton} + <Pressable + accessibilityRole='button' + accessibilityLabel='Open dropdown menu' + hitSlop={8} + style={styles.floatingButton} onPress={() => { // open menu programmatically triggerRef.current?.open(); }} />apps/expo-stylesheet/app/(components)/popover.tsx (2)
13-13: Type-safe programmatic open on Trigger refMatch Trigger’s augmented methods for TS.
- const triggerRef = React.useRef<React.ElementRef<typeof PopoverTrigger>>(null); + const triggerRef = React.useRef< + (React.ElementRef<typeof PopoverTrigger> & { open: () => void }) | null + >(null);
26-35: Add a11y to the FAB PressableExpose role/label for screen readers.
- <Pressable + <Pressable + accessibilityRole='button' + accessibilityLabel='Open popover' style={({ pressed }) => [ styles.fab, { backgroundColor: pressed ? colors.accent : 'transparent' }, ]}apps/expo-stylesheet/app/(components)/table.tsx (2)
92-96: Provide a stable keyExtractor for FlatListAvoids fallback-to-index keys and unnecessary re-renders.
- <FlatList - data={INVOICES} + <FlatList + data={INVOICES} + keyExtractor={(item) => item.invoice} contentContainerStyle={{ paddingBottom: insets.bottom }}
198-203: Add Android elevation for button shadow parityCurrent shadow props don’t render on Android.
priceButton: { shadowOpacity: 0.1, shadowRadius: 2, marginRight: 12, paddingHorizontal: 8, + elevation: 2, },apps/expo-stylesheet/components/ui/popover.tsx (1)
50-60: Add Android elevation for content shadow parityEnsures shadows appear on Android.
content: { zIndex: 50, width: 288, borderRadius: 6, borderWidth: 1, padding: 16, shadowOpacity: 0.05, shadowOffset: { width: 0, height: 2 }, shadowRadius: 4, + elevation: 4, },apps/expo-stylesheet/app/index.tsx (3)
30-36: Stable keys + better keyboard UX for searchable list.Avoid index keys after filtering and make tapping items work while the keyboard is up.
<FlatList ref={ref} data={data} showsVerticalScrollIndicator={false} + keyExtractor={(item) => item} + keyboardShouldPersistTaps="handled" + keyboardDismissMode={Platform.OS === 'ios' ? 'on-drag' : 'none'} contentContainerStyle={styles.listContainer}
16-18: Memoize filtered data.Tiny list, but this keeps renders predictable as the screen grows.
- const data = !search - ? COMPONENTS - : COMPONENTS.filter((item) => item.toLowerCase().includes(search.toLowerCase())); + const data = React.useMemo( + () => + !search + ? COMPONENTS + : COMPONENTS.filter((item) => item.toLowerCase().includes(search.toLowerCase())), + [search] + );
13-13: Type the list ref.Improves IntelliSense and prevents accidental misuse.
- const ref = React.useRef(null); + const ref = React.useRef<FlatList<string>>(null);apps/expo-stylesheet/app/(components)/navigation-menu.tsx (1)
38-45: Include navigation in effect deps.Prevents stale subscription if the navigation object changes.
- React.useEffect(() => { + React.useEffect(() => { const sub = navigation.addListener('blur', () => { closeAll(); }); return sub; - }, []); + }, [navigation]);apps/expo-stylesheet/app/(components)/collapsible.tsx (1)
11-11: Type the theme for stronger safety (optional).- const { colors } = useTheme(); + const { colors } = useTheme() as ICustomTheme;Add import:
import { ChevronsDownUp, ChevronsUpDown } from 'lucide-react-native'; import { useTheme } from '@react-navigation/native'; +import { ICustomTheme } from '~/lib/constants';apps/expo-stylesheet/components/ui/tooltip.tsx (1)
53-63: Improve shadow rendering; avoid clipping and add Android elevation.
overflow: 'hidden'clips shadows; addelevationfor Android.const styles = StyleSheet.create({ content: { zIndex: 50, - overflow: 'hidden', + overflow: 'visible', borderRadius: 6, borderWidth: 1, paddingHorizontal: 12, paddingVertical: 6, shadowOpacity: 0.05, shadowRadius: 4, shadowOffset: { width: 0, height: 2 }, + // Android shadow + elevation: 4, }, });apps/expo-stylesheet/app/(components)/toolbar.tsx (3)
134-137: Make separator fill height reliably on native (avoid 'height: 100%').On native, percentage heights often don't resolve as expected in row layouts. Use alignSelf: 'stretch' instead.
separator: { - height: '100%', - width: 2, + alignSelf: 'stretch', + width: 2, },
37-47: Add accessibility labels to toggle items.Expose intent to screen readers and improve UX.
- <Toolbar.ToggleItem - value='bold' + <Toolbar.ToggleItem + value='bold' + accessibilityLabel='Toggle bold' style={StyleSheet.flatten([- <Toolbar.ToggleItem - value='italic' + <Toolbar.ToggleItem + value='italic' + accessibilityLabel='Toggle italic' style={StyleSheet.flatten([- <Toolbar.ToggleItem - value='left' + <Toolbar.ToggleItem + value='left' + accessibilityLabel='Align left' style={StyleSheet.flatten([- <Toolbar.ToggleItem - value='center' + <Toolbar.ToggleItem + value='center' + accessibilityLabel='Align center' style={StyleSheet.flatten([Also applies to: 48-58, 70-81, 82-91
46-46: Use selected icon color for contrast.Match icon color to selected state text color (e.g., colors.accentText) to ensure contrast on accent backgrounds.
- <Bold color={colors.text} /> + <Bold color={multipleValue.includes('bold') ? colors.accentText : colors.text} />- <Italic color={colors.text} /> + <Italic color={multipleValue.includes('italic') ? colors.accentText : colors.text} />- <AlignLeft color={colors.text} /> + <AlignLeft color={singleValue === 'left' ? colors.accentText : colors.text} />- <AlignCenter color={colors.text} /> + <AlignCenter color={singleValue === 'center' ? colors.accentText : colors.text} />Also applies to: 57-57, 79-79, 90-90
apps/expo-stylesheet/app/(components)/context-menu.tsx (1)
49-51: Prefer themed border color for consistency.Use colors.border rather than colors.text for the trigger outline to match the rest of the app.
- style={[styles.contextTrigger, { borderColor: colors.text }]} + style={[styles.contextTrigger, { borderColor: colors.border }]}apps/expo-stylesheet/components/ui/avatar.tsx (2)
5-5: Use a type-only import for ICustomTheme.Avoids unintended value import under verbatimModuleSyntax.
-import { ICustomTheme } from '~/lib/constants'; +import { type ICustomTheme } from '~/lib/constants';
56-57: Make rounded-full resilient to size overrides.Use a very large radius to guarantee a circle for any size.
- borderRadius: 50, // rounded-full + borderRadius: 9999, // rounded-full- borderRadius: 50, + borderRadius: 9999,Also applies to: 69-70
apps/expo-stylesheet/components/ui/toggle-group.tsx (1)
67-71: Text variants ignore group-level variant/size.Use the same context fallback you apply to container styles to keep text/icon styles consistent.
- value={[ - toggleTextVariants({ variant, size, colors }), - { color: isSelected ? colors.accentText : colors.text }, - ]} + value={[ + toggleTextVariants({ + variant: context.variant || variant, + size: context.size || size, + colors, + }), + { color: isSelected ? colors.accentText : colors.text }, + ]}apps/expo-stylesheet/components/ui/radio-group.tsx (1)
44-57: Consider slight usability polish.Optional: add hitSlop to the Item or wrap with a label Pressable to enlarge tap target, especially on mobile.
- return ( - <RadioGroupPrimitive.Item ref={ref} style={flattenRadioItemStyle} {...props}> + return ( + <RadioGroupPrimitive.Item + ref={ref} + style={flattenRadioItemStyle} + hitSlop={{ top: 8, bottom: 8, left: 8, right: 8 }} + {...props} + >apps/expo-stylesheet/app/(components)/menubar.tsx (4)
25-30: Use safe-area for horizontal insets as wellProtect against notches/rounded corners on iOS by applying left/right safe-area insets.
- const contentInsets = { - top: insets.top, - bottom: insets.bottom, - left: 12, - right: 12, - }; + const contentInsets = { + top: insets.top, + bottom: insets.bottom, + left: Math.max(insets.left, 12), + right: Math.max(insets.right, 12), + };
38-44: Avoid stale closure in navigation listenerInclude dependencies or use useFocusEffect to ensure the latest onValueChange is used.
- React.useEffect(() => { - const sub = navigation.addListener('blur', () => { - onValueChange(undefined); - }); - return sub; - }, []); + React.useEffect(() => { + const sub = navigation.addListener('blur', () => onValueChange(undefined)); + return sub; + }, [navigation, onValueChange]);Or switch to useFocusEffect from @react-navigation/native for clarity.
51-58: Close submenus when switching top-level menuKeeps submenu state predictable when hopping between menus.
function onValueChange(val: string | undefined) { if (typeof val === 'string') { - setValue(val); + if (val !== value) closeSubs(); + setValue(val); return; } closeSubs(); setValue(undefined); }
62-69: Add basic a11y to the dismissal overlayMake the overlay discoverable to screen readers.
- <Pressable + <Pressable onPress={() => { onValueChange(undefined); }} + accessibilityRole="button" + accessibilityLabel="Dismiss menu" style={StyleSheet.absoluteFillObject} />apps/expo-stylesheet/components/ui/tabs.tsx (1)
33-45: Expose selection state to assistive techAdvertise active tab via accessibilityState.
<TabsPrimitive.Trigger ref={ref} + accessibilityState={{ selected: isActive }} style={StyleSheet.flatten([apps/expo-stylesheet/components/ui/button.tsx (1)
11-16: Duplicate disabled prop in ButtonPropsPressable already defines disabled; keeping it here can cause confusion.
Consider removing the redundant disabled?: boolean from ButtonProps and rely on Pressable’s prop.
apps/expo-stylesheet/components/ui/accordion.tsx (2)
151-152: Remove text styles from a ViewfontSize on View has no effect; text styling is already provided via contentProvider.
content: { overflow: 'hidden', - fontSize: 14, },
53-81: Web trigger element choiceUsing View for web works but loses Pressable semantics. Consider always rendering Pressable for consistency.
-const Trigger = Platform.OS === 'web' ? View : Pressable; +const Trigger = Pressable;apps/expo-stylesheet/components/ui/dialog.tsx (1)
20-28: Also flatten Overlay styles for web.Minor consistency tweak to prevent web style-array edge cases.
->(({ style, ...props }, ref) => { - return ( - <DialogPrimitive.Overlay - ref={ref} - style={[styles.overlayWeb, style as StyleProp<ViewStyle>]} - {...props} - /> - ); -}); +>(({ style, ...props }, ref) => { + const flattenStyle = StyleSheet.flatten([styles.overlayWeb, style as StyleProp<ViewStyle>]); + return <DialogPrimitive.Overlay ref={ref} style={flattenStyle} {...props} />; +});apps/expo-stylesheet/components/ui/context-menu.tsx (1)
52-68: Trailing chevron layout: avoid inline style object recreation.Minor perf/readability: lift the icon style to StyleSheet to avoid recreating objects.
- <Icon size={18} color={colors.text} style={{ marginLeft: 'auto' }} /> + <Icon size={18} color={colors.text} style={styles.trailingIcon} />Add:
indicatorBox: { position: 'absolute', left: 8, height: 14, width: 14, alignItems: 'center', justifyContent: 'center', }, + trailingIcon: { + marginLeft: 'auto', + },apps/expo-stylesheet/components/ui/alert-dialog.tsx (1)
118-126: Unify Pressable style handling and typings.You’re passing a function for native and a flattened style for web. Consider standardizing to Pressable’s style callback signature everywhere for consistency and to avoid surprises if primitives switch implementation.
Example:
-const actionBtnStyle = - Platform.OS === 'web' ? buttonVariants({ colors, style }) : nativeStylesFunc; +const actionBtnStyle = ({ pressed }: { pressed: boolean }) => + StyleSheet.flatten([ + buttonVariants({ colors, style }), + pressed && { backgroundColor: colors.borderMedium }, + ]);Repeat for Cancel with the outline variant.
Also applies to: 140-150
apps/expo-stylesheet/components/ui/menubar.tsx (2)
352-397: Optional: add Android elevation for shadows.shadow* only affects iOS/web. Add elevation for parity.
content: { zIndex: 50, minWidth: 128, overflow: 'hidden', borderRadius: 6, borderWidth: 1, padding: 4, shadowOpacity: 0.1, shadowRadius: 4, + ...(Platform.OS === 'android' ? { elevation: 4 } : null), },
1-8: Consistent theme typing for extended colors.Some components cast useTheme() to ICustomTheme while others don’t. Standardize to ensure access to extended palette (mutedText, accent, etc.) is type-safe.
Example:
- const { colors } = useTheme(); + const { colors } = useTheme() as ICustomTheme;Also applies to: 23-37, 143-159, 168-206
| <Pressable | ||
| style={styles.topRightButton} | ||
| onPress={() => { | ||
| // Only for Native platforms: open menu programmatically | ||
| triggerRef.current?.open(); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Limit programmatic open to native and hide the invisible button on web.
Prevents no-op warnings on web and avoids an invisible tap target overlaying UI.
- <Pressable
- style={styles.topRightButton}
- onPress={() => {
- // Only for Native platforms: open menu programmatically
- triggerRef.current?.open();
- }}
- />
+ {Platform.OS !== 'web' ? (
+ <Pressable
+ style={styles.topRightButton}
+ onPress={() => {
+ triggerRef.current?.open();
+ }}
+ accessibilityLabel="Open context menu"
+ />
+ ) : null}📝 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.
| <Pressable | |
| style={styles.topRightButton} | |
| onPress={() => { | |
| // Only for Native platforms: open menu programmatically | |
| triggerRef.current?.open(); | |
| }} | |
| /> | |
| {Platform.OS !== 'web' ? ( | |
| <Pressable | |
| style={styles.topRightButton} | |
| onPress={() => { | |
| triggerRef.current?.open(); | |
| }} | |
| accessibilityLabel="Open context menu" | |
| /> | |
| ) : null} |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/app/(components)/context-menu.tsx around lines 40 to 46,
avoid rendering or calling the invisible Pressable on web: import Platform from
'react-native' if needed and conditionally render the Pressable only when
Platform.OS !== 'web' (or wrap the onPress/trigger call with a Platform check),
so the button is not present nor triggers a no-op warning on web; do not rely on
styling to hide it—omit the element entirely on web to prevent overlaying the
UI.
| height: 150, | ||
| width: '100%', | ||
| maxWidth: 300, | ||
| marginHorizontal: 'auto', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| borderRadius: 6, | ||
| borderWidth: 1, | ||
| borderStyle: 'dashed', | ||
| }, |
There was a problem hiding this comment.
Fix invalid style on native: marginHorizontal: 'auto' is web-only.
Use alignSelf: 'center' (works on all platforms) and drop the 'auto' margin.
contextTrigger: {
height: 150,
- width: '100%',
- maxWidth: 300,
- marginHorizontal: 'auto',
+ width: '100%',
+ maxWidth: 300,
+ alignSelf: 'center',
alignItems: 'center',📝 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.
| height: 150, | |
| width: '100%', | |
| maxWidth: 300, | |
| marginHorizontal: 'auto', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| borderRadius: 6, | |
| borderWidth: 1, | |
| borderStyle: 'dashed', | |
| }, | |
| contextTrigger: { | |
| height: 150, | |
| width: '100%', | |
| maxWidth: 300, | |
| alignSelf: 'center', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| borderRadius: 6, | |
| borderWidth: 1, | |
| borderStyle: 'dashed', | |
| }, |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/app/(components)/context-menu.tsx around lines 141 to
150, the style uses marginHorizontal: 'auto' which is web-only and invalid on
native; replace that with alignSelf: 'center' and remove the marginHorizontal
property so the container centers across platforms; keep the rest of the style
properties unchanged.
| export { Toggle, ToggleIcon, toggleTextVariants, toggleVariants, ToggleProps }; | ||
|
|
There was a problem hiding this comment.
Type export bug: ToggleProps exported as a value
This is a compile error in TS: “only refers to a type, but is being used as a value.”
-export { Toggle, ToggleIcon, toggleTextVariants, toggleVariants, ToggleProps };
+export { Toggle, ToggleIcon, toggleTextVariants, toggleVariants };
+export type { ToggleProps };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export { Toggle, ToggleIcon, toggleTextVariants, toggleVariants, ToggleProps }; | |
| export { Toggle, ToggleIcon, toggleTextVariants, toggleVariants }; | |
| export type { ToggleProps }; |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/toggle.tsx around lines 124-125, the file
currently exports ToggleProps as a value which causes the TS error "only refers
to a type, but is being used as a value"; change the export so the type-only
symbol is exported with the TypeScript type modifier (for example split the
export into a value export for Toggle, ToggleIcon, toggleTextVariants,
toggleVariants and a separate type export like "export type { ToggleProps }"),
ensuring ToggleProps is declared as an interface/type and removing it from the
value export list.
| <TooltipPrimitive.Portal hostName={portalHost}> | ||
| <TooltipPrimitive.Overlay style={Platform.OS !== 'web' ? StyleSheet.absoluteFill : undefined}> | ||
| <Animated.View |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Portal prop mismatch across platforms (hostName vs container).
Ensure portalHost works on web and native.
- <TooltipPrimitive.Portal hostName={portalHost}>
- <TooltipPrimitive.Overlay style={Platform.OS !== 'web' ? StyleSheet.absoluteFill : undefined}>
+ {/*
+ Map portalHost -> hostName (native) or container (web).
+ */}
+ {(() => {
+ const portalProps = Platform.select({
+ web: { container: portalHost },
+ default: { hostName: portalHost },
+ }) as any;
+ return (
+ <TooltipPrimitive.Portal {...portalProps}>
+ <TooltipPrimitive.Overlay
+ style={Platform.OS !== 'web' ? StyleSheet.absoluteFill : undefined}
+ pointerEvents="box-none"
+ >Follow-through (closing tags unchanged).
🏁 Script executed:
#!/bin/bash
# Locate the tooltip.tsx file within the expo-stylesheet app
file=$(fd --type f "tooltip.tsx" apps/expo-stylesheet/components/ui | head -n1)
echo "Found file: $file"
# Display relevant lines around the Portal usage (lines 1-100)
sed -n '1,100p' "$file"Length of output: 2352
Map portalHost to the correct Portal prop per platform
In apps/expo-stylesheet/components/ui/tooltip.tsx (around lines 29–31), replace
- <TooltipPrimitive.Portal hostName={portalHost}>
+ <TooltipPrimitive.Portal
+ {...Platform.select({
+ web: { container: portalHost },
+ default: { hostName: portalHost },
+ })}
+ >and add pointerEvents="box-none" to <TooltipPrimitive.Overlay> to allow touch passthrough. (rnprimitives.com)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/tooltip.tsx around lines 29–31, map the
portalHost prop to the platform-specific Portal prop and add touch passthrough:
when Platform.OS === 'web' pass portalHost as hostName, otherwise pass
portalHost as host (use a conditional prop object or spread), and add
pointerEvents="box-none" to the <TooltipPrimitive.Overlay> element while keeping
the existing StyleSheet.absoluteFill logic.
| export const mergeBaseStyleWithUserStyle = <TemplateStyle extends ViewStyle | TextStyle>( | ||
| baseStyle: (TemplateStyle | false | undefined | null)[], | ||
| userStyle: any | ||
| ): (TemplateStyle | false | undefined | null)[] => { | ||
| if (isViewStyleArray(userStyle)) { | ||
| // unfortunately, we can't use '...style' directly. | ||
| // See isViewStyleArray() function for more details. | ||
| return [...baseStyle, ...Object.values(userStyle as object)]; | ||
| } | ||
|
|
||
| return [...baseStyle, userStyle]; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type the API with StyleProp and handle arrays directly
Using StyleProp improves interop with RN registered styles. Also flatten true arrays without Object.values.
-import { ViewStyle, TextStyle } from 'react-native';
+import { ViewStyle, TextStyle, StyleProp } from 'react-native';
@@
-export const mergeBaseStyleWithUserStyle = <TemplateStyle extends ViewStyle | TextStyle>(
- baseStyle: (TemplateStyle | false | undefined | null)[],
- userStyle: any
+export const mergeBaseStyleWithUserStyle = <TemplateStyle extends ViewStyle | TextStyle>(
+ baseStyle: (TemplateStyle | false | undefined | null)[],
+ userStyle: StyleProp<TemplateStyle>
): (TemplateStyle | false | undefined | null)[] => {
- if (isViewStyleArray(userStyle)) {
+ if (Array.isArray(userStyle)) {
+ return [...baseStyle, ...userStyle];
+ }
+ if (isViewStyleArray(userStyle)) {
// unfortunately, we can't use '...style' directly.
// See isViewStyleArray() function for more details.
return [...baseStyle, ...Object.values(userStyle as object)];
}
return [...baseStyle, userStyle];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const mergeBaseStyleWithUserStyle = <TemplateStyle extends ViewStyle | TextStyle>( | |
| baseStyle: (TemplateStyle | false | undefined | null)[], | |
| userStyle: any | |
| ): (TemplateStyle | false | undefined | null)[] => { | |
| if (isViewStyleArray(userStyle)) { | |
| // unfortunately, we can't use '...style' directly. | |
| // See isViewStyleArray() function for more details. | |
| return [...baseStyle, ...Object.values(userStyle as object)]; | |
| } | |
| return [...baseStyle, userStyle]; | |
| }; | |
| import { ViewStyle, TextStyle, StyleProp } from 'react-native'; | |
| export const mergeBaseStyleWithUserStyle = <TemplateStyle extends ViewStyle | TextStyle>( | |
| baseStyle: (TemplateStyle | false | undefined | null)[], | |
| userStyle: StyleProp<TemplateStyle> | |
| ): (TemplateStyle | false | undefined | null)[] => { | |
| if (Array.isArray(userStyle)) { | |
| return [...baseStyle, ...userStyle]; | |
| } | |
| if (isViewStyleArray(userStyle)) { | |
| // unfortunately, we can't use '...style' directly. | |
| // See isViewStyleArray() function for more details. | |
| return [...baseStyle, ...Object.values(userStyle as object)]; | |
| } | |
| return [...baseStyle, userStyle]; | |
| }; |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/lib/utils.ts around lines 3 to 14, the function
currently types userStyle as any and flattens RN style arrays via Object.values
which breaks interop with registered RN styles; change the signature to accept
userStyle as StyleProp<TemplateStyle> (import StyleProp, ViewStyle, TextStyle
from react-native) and when userStyle is an array (Array.isArray(userStyle))
handle it by concatenating/flattening the array elements directly rather than
using Object.values; keep existing branch for non-array StyleProp values and
return a new array combining baseStyle with the flattened userStyle, ensuring
types align with (TemplateStyle | false | undefined | null)[].
| <AlertDialogPortal hostName={portalHost}> | ||
| <AlertDialogOverlay> | ||
| <AlertDialogPrimitive.Content ref={ref} style={flattenContentStyles} {...props} /> | ||
| </AlertDialogOverlay> | ||
| </AlertDialogPortal> | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Portal prop name (hostName vs container).
The primitives’ web Portal typically accepts container rather than hostName. Passing an unknown prop can break TS types and be ignored at runtime.
Run a quick grep to confirm accepted props and adjust accordingly:
If web expects container, gate by platform:
- <AlertDialogPortal hostName={portalHost}>
+ <AlertDialogPortal {...(Platform.OS === 'web' ? {} : { hostName: portalHost })}>🏁 Script executed:
#!/bin/bash
rg -nP -C2 'interface\\s+.*PortalProps|type\\s+.*PortalProps|function\\s+Portal\\s*\\(' packages | sed -n '1,200p'
rg -n 'hostName|container' packagesLength of output: 7042
🏁 Script executed:
#!/bin/bash
# Print the first 200 lines of the app’s alert-dialog component to verify import and usage of AlertDialogPortal
FILE="apps/expo-stylesheet/components/ui/alert-dialog.tsx"
if [ -f "$FILE" ]; then
sed -n '1,200p' "$FILE"
else
echo "File not found: $FILE"
fiLength of output: 6960
Gate Portal props by platform
On web AlertDialogPrimitive.Portal expects container, not hostName; update to:
- <AlertDialogPortal hostName={portalHost}>
+ <AlertDialogPortal {...(Platform.OS === 'web' ? { container: portalHost } : { hostName: portalHost })}>📝 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.
| <AlertDialogPortal hostName={portalHost}> | |
| <AlertDialogOverlay> | |
| <AlertDialogPrimitive.Content ref={ref} style={flattenContentStyles} {...props} /> | |
| </AlertDialogOverlay> | |
| </AlertDialogPortal> | |
| ); | |
| <AlertDialogPortal {...(Platform.OS === 'web' ? { container: portalHost } : { hostName: portalHost })}> | |
| <AlertDialogOverlay> | |
| <AlertDialogPrimitive.Content ref={ref} style={flattenContentStyles} {...props} /> | |
| </AlertDialogOverlay> | |
| </AlertDialogPortal> | |
| ); |
| <DialogPrimitive.Content | ||
| ref={ref} | ||
| style={[ | ||
| styles.content, | ||
| { | ||
| borderColor: colors.border, | ||
| backgroundColor: colors.card, | ||
| shadowColor: colors.text, | ||
| }, | ||
| style, | ||
| ]} | ||
| {...props} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Flatten Content styles on web to avoid style-array issues.
You noted needing StyleSheet.flatten for web; apply the same here to be safe and consistent.
- <DialogPrimitive.Content
- ref={ref}
- style={[
- styles.content,
- {
- borderColor: colors.border,
- backgroundColor: colors.card,
- shadowColor: colors.text,
- },
- style,
- ]}
- {...props}
- >
+ {/*
+ Flatten to a single object to avoid web/Radix style array pitfalls
+ */}
+ {(() => {
+ const flattenContentStyle = StyleSheet.flatten([
+ styles.content,
+ {
+ borderColor: colors.border,
+ backgroundColor: colors.card,
+ shadowColor: colors.text,
+ },
+ style,
+ ]);
+ return (
+ <DialogPrimitive.Content ref={ref} style={flattenContentStyle} {...props}>
+ {children}
+ <DialogPrimitive.Close style={styles.closeButton}>
+ <X size={Platform.OS === 'web' ? 16 : 18} color={open ? colors.primary : colors.text} />
+ </DialogPrimitive.Close>
+ </DialogPrimitive.Content>
+ );
+ })()}
- {children}
- <DialogPrimitive.Close style={styles.closeButton}>
- <X size={Platform.OS === 'web' ? 16 : 18} color={open ? colors.primary : colors.text} />
- </DialogPrimitive.Close>
- </DialogPrimitive.Content>📝 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.
| <DialogPrimitive.Content | |
| ref={ref} | |
| style={[ | |
| styles.content, | |
| { | |
| borderColor: colors.border, | |
| backgroundColor: colors.card, | |
| shadowColor: colors.text, | |
| }, | |
| style, | |
| ]} | |
| {...props} | |
| {/* | |
| Flatten to a single object to avoid web/Radix style array pitfalls | |
| */} | |
| {(() => { | |
| const flattenContentStyle = StyleSheet.flatten([ | |
| styles.content, | |
| { | |
| borderColor: colors.border, | |
| backgroundColor: colors.card, | |
| shadowColor: colors.text, | |
| }, | |
| style, | |
| ]); | |
| return ( | |
| <DialogPrimitive.Content ref={ref} style={flattenContentStyle} {...props}> | |
| {children} | |
| <DialogPrimitive.Close style={styles.closeButton}> | |
| <X | |
| size={Platform.OS === 'web' ? 16 : 18} | |
| color={open ? colors.primary : colors.text} | |
| /> | |
| </DialogPrimitive.Close> | |
| </DialogPrimitive.Content> | |
| ); | |
| })()} |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/dialog.tsx around lines 68 to 79, the
Content component currently receives a style array which can cause style-array
issues on web; replace the array with a flattened style object by calling
StyleSheet.flatten on the combined styles (styles.content, the inline
border/background/shadow object, and the incoming style) and pass that single
flattened object as the style prop so web receives a plain style object.
| <Animated.View style={chevronStyle}> | ||
| <ChevronDown size={12} color={colors.text} aria-hidden={true} /> | ||
| </Animated.View> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Drop non-native prop on icon.
aria-hidden isn’t a valid prop for lucide/react-native-svg on native; avoid potential warnings.
- <ChevronDown size={12} color={colors.text} aria-hidden={true} />
+ <ChevronDown size={12} color={colors.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.
| <Animated.View style={chevronStyle}> | |
| <ChevronDown size={12} color={colors.text} aria-hidden={true} /> | |
| </Animated.View> | |
| <Animated.View style={chevronStyle}> | |
| <ChevronDown size={12} color={colors.text} /> | |
| </Animated.View> |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/navigation-menu.tsx around lines 95 to 97,
the ChevronDown icon is being passed the web-only prop aria-hidden which is not
valid for lucide/react-native-svg on native; remove the aria-hidden prop from
the ChevronDown component and, if the intent is to hide the icon from
accessibility tools, replace it with the appropriate native prop (e.g.,
accessible={false} on the icon or
accessibilityElementsHidden/importantForAccessibility on the parent) so no
invalid props are passed to the native SVG component.
| const H1 = React.forwardRef<TextRef, SlottableTextProps>( | ||
| ({ style, asChild = false, ...props }, ref) => { | ||
| const { colors } = useTheme(); | ||
| const Component = asChild ? Slot.Text : RNText; | ||
| const themedStyle = [styles.h1, { color: colors.text }, style]; | ||
|
|
||
| return <Component role='heading' aria-level='1' style={themedStyle} ref={ref} {...props} />; | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add native accessibilityRole for headings (keep web roles).
Ensure screen readers identify headings on iOS/Android.
- return <Component role='heading' aria-level='1' style={themedStyle} ref={ref} {...props} />;
+ return (
+ <Component
+ role='heading'
+ aria-level='1'
+ accessibilityRole="header"
+ style={themedStyle}
+ ref={ref}
+ {...props}
+ />
+ );Apply similarly to H2/H3/H4 returns (aria-level 2/3/4 respectively) by adding accessibilityRole="header".
Also applies to: 20-35, 39-47, 51-59
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/typography.tsx around lines 8-16 (and
similarly for H2/H3/H4 at 20-35, 39-47, 51-59), the components set web roles
(role and aria-level) but lack native accessibilityRole for iOS/Android; update
each heading return to include accessibilityRole="header" in addition to the
existing role and aria-level (use aria-level="2"/"3"/"4" for H2/H3/H4
respectively) so screen readers on native platforms detect headings while
preserving web attributes.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
apps/expo-stylesheet/app/_layout.tsx (1)
73-73: Resolved: fontWeight is now a string.Thanks for converting 600 → '600'; this aligns with RN typings and avoids runtime inconsistencies.
apps/expo-stylesheet/components/ui/alert-dialog.tsx (2)
67-73: Gate Portal prop by platform (avoid unknown prop on web).On web,
Portaltypically doesn't accepthostName; passing it may be ignored and can break TS inference. Gate by platform and only passhostNameon native. If web supportscontainer, wire it conditionally once confirmed.- <AlertDialogPortal hostName={portalHost}> + <AlertDialogPortal {...(Platform.OS === 'web' ? {} : { hostName: portalHost })}>Run to verify accepted Portal props before switching to
container:#!/bin/bash rg -nP -C2 '(interface|type)\s+.*PortalProps|Portal\s*\(' -g '!**/node_modules/**' rg -n 'hostName|container' -g '!**/node_modules/**'
213-217: Resolved: fontWeight is a string (correct for RN).Nice fix; previous numeric value would have been invalid.
apps/expo-stylesheet/components/ui/dialog.tsx (3)
82-85: Add accessibility label and larger touch target for the Close button.Improve SR discoverability and usability on touch devices.
- <DialogPrimitive.Close style={styles.closeButton}> + <DialogPrimitive.Close + accessibilityRole="button" + accessibilityLabel="Close dialog" + hitSlop={{ top: 8, right: 8, bottom: 8, left: 8 }} + style={styles.closeButton} + > <X size={Platform.OS === 'web' ? 16 : 18} color={open ? colors.primary : colors.text} /> </DialogPrimitive.Close>
183-187: Resolved: fontWeight uses string type.Prior concern about numeric fontWeight is addressed.
68-79: Flatten Content style to avoid web/Radix style-array crashes.On web, passing a style array can trigger "Failed to set an indexed property [0] on 'CSSStyleDeclaration'". Flatten to a single object before passing to Content.
const { open } = DialogPrimitive.useRootContext(); const { colors } = useTheme(); + const flattenContentStyle = StyleSheet.flatten([ + styles.content, + { + borderColor: colors.border, + backgroundColor: colors.card, + shadowColor: colors.text, + }, + style, + ]); + return ( <DialogPortal hostName={portalHost}> <DialogOverlay> <DialogPrimitive.Content ref={ref} - style={[ - styles.content, - { - borderColor: colors.border, - backgroundColor: colors.card, - shadowColor: colors.text, - }, - style, - ]} + style={flattenContentStyle} {...props} >apps/expo-stylesheet/components/ui/dropdown-menu.tsx (1)
233-241: Resolved:fontWeightis now a string. Good catch.Previous review flagged numeric
fontWeight; you corrected it to'600'.apps/expo-stylesheet/components/ui/context-menu.tsx (1)
369-373: LGTM: fontWeight fixed to string.The previous invalid numeric fontWeight is now correctly a string.
🧹 Nitpick comments (15)
apps/expo-stylesheet/app/_layout.tsx (1)
70-78: Make header title robust and theme-consistent.
- Guard against non-string children to avoid runtime errors.
- Use NAV_THEME text color instead of hard-coded black/white for consistency.
Apply within this block:
- headerTitle(props) { - return ( - <Text - style={{ - fontSize: Platform.OS === 'web' ? 20 : 18, - fontWeight: '600', - color: isDarkColorScheme ? 'white' : 'black', - }} - > - {toOptions(props.children)} - </Text> - ); - }, + headerTitle(props) { + const titleText = + typeof props.children === 'string' ? toOptions(props.children) : String(props.children ?? ''); + return ( + <Text + style={{ + fontSize: Platform.OS === 'web' ? 20 : 18, + fontWeight: '600', + color: isDarkColorScheme ? NAV_THEME.dark.text : NAV_THEME.light.text, + }} + > + {titleText} + </Text> + ); + },Optionally, make toOptions resilient:
function toOptions(name: unknown) { const str = String(name ?? ''); const title = str .replace('(components)/', '') .split('-') .map((s) => s.replace(/\b\w/g, (c) => c.toUpperCase())) .join(' '); return title; }apps/expo-stylesheet/components/ui/alert-dialog.tsx (5)
113-126: Allow Pressable style callbacks for Action (preserves consumer flexibility).Support
styleas a Pressable callback on native and merge it with your pressed-state styles.const AlertDialogAction = React.forwardRef< React.ElementRef<typeof AlertDialogPrimitive.Action>, - React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Action> & { - style?: StyleProp<ViewStyle>; - } + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Action> & { + style?: + | StyleProp<ViewStyle> + | ((state: { pressed: boolean; hovered?: boolean; focused?: boolean }) => StyleProp<ViewStyle>); + } >(({ style, ...props }, ref) => { const { colors } = useTheme() as ICustomTheme; - const nativeStylesFunc = ({ pressed }: { pressed: boolean }) => [ - buttonVariants({ colors, style }), - pressed && { backgroundColor: colors.borderMedium }, - ]; - const actionBtnStyle = - Platform.OS === 'web' ? buttonVariants({ colors, style }) : nativeStylesFunc; + const resolve = (pressed: boolean) => + StyleSheet.flatten([ + buttonVariants({ colors }), + pressed && { backgroundColor: colors.borderMedium }, + typeof style === 'function' + ? style({ pressed, hovered: false, focused: false }) + : style, + ]); + const actionBtnStyle = + Platform.OS === 'web' + ? buttonVariants({ colors, style: typeof style === 'function' ? undefined : style }) + : ({ pressed }: { pressed: boolean }) => resolve(pressed);
135-150: Mirror Pressable style callback support for Cancel.Same ergonomics as Action; accept callback and merge.
const AlertDialogCancel = React.forwardRef< React.ElementRef<typeof AlertDialogPrimitive.Cancel>, React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Cancel> & { - style?: StyleProp<ViewStyle>; + style?: + | StyleProp<ViewStyle> + | ((state: { pressed: boolean; hovered?: boolean; focused?: boolean }) => StyleProp<ViewStyle>); } >(({ style, ...props }, ref) => { const { colors } = useTheme() as ICustomTheme; - const nativeStylesFunc = ({ pressed }: { pressed: boolean }) => [ - buttonVariants({ variant: 'outline', colors, style }), - pressed && { backgroundColor: colors.accent }, - ]; - const cancelBtnStyle = - Platform.OS === 'web' - ? buttonVariants({ variant: 'outline', colors, style }) - : nativeStylesFunc; + const resolve = (pressed: boolean) => + StyleSheet.flatten([ + buttonVariants({ variant: 'outline', colors }), + pressed && { backgroundColor: colors.accent }, + typeof style === 'function' + ? style({ pressed, hovered: false, focused: false }) + : style, + ]); + const cancelBtnStyle = + Platform.OS === 'web' + ? buttonVariants({ + variant: 'outline', + colors, + style: typeof style === 'function' ? undefined : style, + }) + : ({ pressed }: { pressed: boolean }) => resolve(pressed);
2-2: Add PressableStateCallbackType as a type-only import (supports above changes).-import { Platform, StyleSheet, View, ViewStyle, StyleProp } from 'react-native'; +import { Platform, StyleSheet, View, ViewStyle, StyleProp, type PressableStateCallbackType } from 'react-native';
193-203: Ensure zIndex takes effect on content.On web and RN, zIndex is most reliable with a non-static position. Add
position: 'relative'.content: { zIndex: 50, maxWidth: 512, gap: 16, borderWidth: 1, padding: 24, borderRadius: 8, shadowOpacity: 0.1, shadowOffset: { width: 0, height: 2 }, shadowRadius: 6, + position: 'relative', },
20-21: Remove commented-out code.The commented
useRootContextlines add noise.- // const { open } = AlertDialogPrimitive.useRootContext(); + /* no-op */Or delete outright.
Also applies to: 55-56
apps/expo-stylesheet/components/ui/dialog.tsx (4)
21-28: Optionally flatten OverlayWeb style for consistency.Not strictly required, but keeps web safe from any future style-array regressions and mirrors Content handling.
return ( <DialogPrimitive.Overlay ref={ref} - style={[styles.overlayWeb, style as StyleProp<ViewStyle>]} + style={StyleSheet.flatten([styles.overlayWeb, style as StyleProp<ViewStyle>])} {...props} /> );
36-45: Nit: flatten OverlayNative style to a single object.Minor readability + parity with web.
- <DialogPrimitive.Overlay - ref={ref} - style={[StyleSheet.absoluteFill, styles.overlayNative, style as StyleProp<ViewStyle>]} - {...props} - > + <DialogPrimitive.Overlay + ref={ref} + style={StyleSheet.flatten([StyleSheet.absoluteFill, styles.overlayNative, style as StyleProp<ViewStyle>])} + {...props} + >
156-165: Add Android elevation to match iOS shadows.Current shadow* props don’t render on Android; add elevation for visual parity.
content: { maxWidth: 512, gap: 16, borderWidth: 1, padding: 24, borderRadius: 8, shadowOpacity: 0.2, shadowRadius: 8, shadowOffset: { width: 0, height: 4 }, + elevation: 8, },
114-123: Nit: unify theme typing usage.You cast to ICustomTheme in Description but not in Title; consider consistent typing for maintainability.
- const { colors } = useTheme(); + const { colors } = useTheme() as ICustomTheme;apps/expo-stylesheet/components/ui/dropdown-menu.tsx (3)
68-68: Type theme consistently.Cast
useTheme()toICustomThemewhere you access custom tokens for consistency and better IntelliSense.- const { colors } = useTheme(); + const { colors } = useTheme() as ICustomTheme;Also applies to: 95-95
76-80: Use a stable shadow color and add Android elevation.
shadowColor: colors.textcan be too light in dark mode; Android also needselevation.{ borderColor: colors.border, backgroundColor: colors.card, - shadowColor: colors.text, + shadowColor: '#000', + elevation: 4, },Apply in both SubContent and Content style blocks.
Also applies to: 99-103
268-285: Consider using your themed Text component for shortcuts.To inherit typography from
TextClassContextand keep styles consistent across platforms, render~/components/ui/text’sTextinstead of RNText.Minimal change:
-import { Platform, StyleProp, StyleSheet, Text, TextStyle, View, ViewStyle } from 'react-native'; +import { Platform, StyleProp, StyleSheet, TextStyle, View, ViewStyle } from 'react-native'; +import { Text } from '~/components/ui/text';apps/expo-stylesheet/components/ui/context-menu.tsx (2)
320-332: Add Android elevation for consistent shadow.Current shadows are iOS-only; add elevation for Android parity.
content: { zIndex: 50, minWidth: 128, overflow: 'hidden', borderRadius: 6, borderWidth: 1, marginTop: 4, padding: 4, shadowColor: '#000', shadowOpacity: 0.05, shadowOffset: { width: 0, height: 1 }, - shadowRadius: 3, + shadowRadius: 3, + elevation: 6, },
79-80: Unify theme typing for IntelliSense and safety.Cast
useTheme()toICustomThemeconsistently socolors.accent/mutedTextare typed everywhere.- const { colors } = useTheme(); + const { colors } = useTheme() as ICustomTheme;Also applies to: 270-271
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
apps/expo-stylesheet/app/(components)/collapsible.tsx(1 hunks)apps/expo-stylesheet/app/(components)/hover-card.tsx(1 hunks)apps/expo-stylesheet/app/(components)/navigation-menu.tsx(1 hunks)apps/expo-stylesheet/app/(components)/popover.tsx(1 hunks)apps/expo-stylesheet/app/(components)/separator.tsx(1 hunks)apps/expo-stylesheet/app/(components)/table.tsx(1 hunks)apps/expo-stylesheet/app/_layout.tsx(4 hunks)apps/expo-stylesheet/components/ui/accordion.tsx(1 hunks)apps/expo-stylesheet/components/ui/alert-dialog.tsx(1 hunks)apps/expo-stylesheet/components/ui/button.tsx(1 hunks)apps/expo-stylesheet/components/ui/card.tsx(1 hunks)apps/expo-stylesheet/components/ui/context-menu.tsx(1 hunks)apps/expo-stylesheet/components/ui/dialog.tsx(1 hunks)apps/expo-stylesheet/components/ui/dropdown-menu.tsx(1 hunks)apps/expo-stylesheet/components/ui/label.tsx(1 hunks)apps/expo-stylesheet/components/ui/menubar.tsx(1 hunks)apps/expo-stylesheet/components/ui/select.tsx(3 hunks)apps/expo-stylesheet/components/ui/table.tsx(1 hunks)apps/expo-stylesheet/components/ui/tabs.tsx(1 hunks)apps/expo-stylesheet/components/ui/toggle.tsx(1 hunks)apps/expo-stylesheet/components/ui/typography.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- apps/expo-stylesheet/app/(components)/table.tsx
- apps/expo-stylesheet/components/ui/accordion.tsx
- apps/expo-stylesheet/components/ui/table.tsx
- apps/expo-stylesheet/components/ui/toggle.tsx
- apps/expo-stylesheet/app/(components)/separator.tsx
- apps/expo-stylesheet/app/(components)/navigation-menu.tsx
- apps/expo-stylesheet/components/ui/tabs.tsx
- apps/expo-stylesheet/components/ui/label.tsx
- apps/expo-stylesheet/app/(components)/collapsible.tsx
- apps/expo-stylesheet/components/ui/typography.tsx
- apps/expo-stylesheet/components/ui/button.tsx
- apps/expo-stylesheet/components/ui/card.tsx
- apps/expo-stylesheet/app/(components)/popover.tsx
- apps/expo-stylesheet/app/(components)/hover-card.tsx
- apps/expo-stylesheet/components/ui/menubar.tsx
- apps/expo-stylesheet/components/ui/select.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
apps/expo-stylesheet/components/ui/dialog.tsx (2)
apps/expo-stylesheet/lib/constants.ts (1)
ICustomTheme(100-100)packages/dialog/src/dialog.web.tsx (3)
props(175-182)props(93-100)props(106-132)
apps/expo-stylesheet/components/ui/context-menu.tsx (2)
apps/expo-stylesheet/lib/constants.ts (1)
ICustomTheme(100-100)packages/context-menu/src/context-menu.web.tsx (6)
props(482-489)props(69-107)props(494-503)align(130-189)props(240-247)props(431-438)
apps/expo-stylesheet/components/ui/dropdown-menu.tsx (3)
apps/expo-stylesheet/lib/constants.ts (1)
ICustomTheme(100-100)apps/expo-stylesheet/components/ui/text.tsx (2)
TextClassContext(31-31)Text(31-31)apps/docs/src/components/ui/dropdown-menu.tsx (7)
props(24-37)props(58-70)props(117-133)props(78-88)props(141-147)props(94-111)props(153-159)
apps/expo-stylesheet/app/_layout.tsx (2)
apps/expo-stylesheet/lib/useColorScheme.tsx (1)
useColorScheme(4-14)apps/expo-stylesheet/components/ThemeToggle.tsx (1)
ThemeToggle(8-33)
apps/expo-stylesheet/components/ui/alert-dialog.tsx (2)
apps/expo-stylesheet/lib/constants.ts (1)
ICustomTheme(100-100)apps/expo-stylesheet/components/ui/button.tsx (2)
buttonVariants(92-144)buttonTextVariants(146-194)
🔇 Additional comments (11)
apps/expo-stylesheet/app/_layout.tsx (3)
8-10: LGTM: Theme control wiring is clean.Importing ThemeToggle and the shared useColorScheme hook keeps theme state centralized and consistent across the app.
30-33: LGTM: Startup theme state derivation is sound.Deriving isDarkColorScheme from the store and gating initial render elsewhere prevents flash-of-incorrect-theme.
84-88: LGTM: Header action for theme toggling.Placing ThemeToggle in headerRight is a good UX touch and keeps theme switching discoverable.
apps/expo-stylesheet/components/ui/dialog.tsx (4)
103-112: LGTM: Title uses flattened themed style.Good use of precomputed flattened style and theming.
51-54: LGTM: Platform-specific overlay selection is clear and correct.
125-136: LGTM: Export surface is tidy and explicit.
158-159: Drop gap fallback – RN ≥ 0.71 supports numericgapand this project targets RN 0.79.2. (reactnative.dev)apps/expo-stylesheet/components/ui/dropdown-menu.tsx (3)
253-262: No change required:colors.borderMediumis valid
ICustomThemeColor(defined in lib/constants.ts) includes aborderMediumfield, so usingcolors.borderMediumis safe at runtime.
306-325: No action needed: Flexboxgapis supported since React Native 0.71 and this project uses RN 0.79.2 via Expo SDK 53. (reactnative.dev, expo.dev)
126-133: No changes needed: Pressable-style function supported by Item, CheckboxItem, and RadioItem on both React Native (iOS/Android) and React Native Web.apps/expo-stylesheet/components/ui/context-menu.tsx (1)
333-341: Remove version check: CSSgapis supported in RN 0.79.2 (Expo SDK 53). Thegapprop for pixel values was introduced in React Native 0.71 and Expo SDK 53 includes RN 0.79.2 with the New Architecture by default, sogap: 8works as expected (reactnative.dev, expo.dev)
| <>{children}</> | ||
| <Icon size={18} color={colors.text} style={{ marginLeft: 'auto' }} /> | ||
| </ContextMenuPrimitive.SubTrigger> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix cross-platform alignment: avoid marginLeft: 'auto' (not supported on native).
On iOS/Android, marginLeft does not accept 'auto'. Wrap children with a flex spacer and drop the auto margin to keep the chevron right-aligned everywhere.
- <>{children}</>
- <Icon size={18} color={colors.text} style={{ marginLeft: 'auto' }} />
+ <View style={{ flex: 1, minWidth: 0 }}>{children}</View>
+ <Icon size={18} color={colors.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.
| <>{children}</> | |
| <Icon size={18} color={colors.text} style={{ marginLeft: 'auto' }} /> | |
| </ContextMenuPrimitive.SubTrigger> | |
| <ContextMenuPrimitive.SubTrigger> | |
| <View style={{ flex: 1, minWidth: 0 }}>{children}</View> | |
| <Icon size={18} color={colors.text} /> | |
| </ContextMenuPrimitive.SubTrigger> |
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/context-menu.tsx around lines 65 to 67,
the Icon is right-aligned using style={{ marginLeft: 'auto' }}, which isn't
supported on iOS/Android; remove the marginLeft on the Icon and instead insert a
flexible spacer between the children and the Icon (e.g., wrap children and then
add a View/Box with style={{ flex: 1 }} or equivalent) so the chevron stays
right-aligned cross-platform; adjust imports if needed and preserve existing
styling on children and Icon.
| style={[ | ||
| { | ||
| marginLeft: 'auto', | ||
| fontSize: 12, | ||
| lineHeight: 16, | ||
| letterSpacing: 1, | ||
| color: colors.mutedText, | ||
| }, | ||
| style, | ||
| ]} | ||
| {...props} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make Shortcut trailing alignment native-safe.
Same issue: marginLeft: 'auto' isn’t valid on native. Prefer a parent flex spacer (see SubTrigger fix) or gate the auto margin to web only.
- marginLeft: 'auto',
+ ...(Platform.OS === 'web' ? { marginLeft: 'auto' } : null),
+ // expects parent to place a flex:1 spacer before Shortcut on nativeCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/context-menu.tsx around lines 286 to 296,
the Shortcut component uses marginLeft: 'auto' which is invalid on React Native;
replace this with a native-safe layout by either (a) moving alignment into the
parent using a flex spacer element (an empty View with flex: 1) to push the
Shortcut to the end, or (b) gate the 'marginLeft: "auto"' style behind a
web-only check (Platform.OS === 'web') so native platforms receive a compatible
style (e.g., no marginLeft and rely on parent flex alignment). Ensure props and
style composition still merge correctly after the change.
| const nativeStyle = ({ pressed }: { pressed: boolean }) => [ | ||
| styles.item, | ||
| { backgroundColor: open ? colors.accent : 'transparent' }, | ||
| inset && { paddingLeft: 32 }, | ||
| pressed && { backgroundColor: colors.accent }, | ||
| style, | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix cross-platform alignment; avoid marginLeft: 'auto' (invalid on native).
Use container layout instead of a string margin value.
@@
const nativeStyle = ({ pressed }: { pressed: boolean }) => [
styles.item,
{ backgroundColor: open ? colors.accent : 'transparent' },
+ { justifyContent: 'space-between' },
inset && { paddingLeft: 32 },
pressed && { backgroundColor: colors.accent },
style,
];
@@
const webStyle = StyleSheet.flatten([
styles.item,
{ backgroundColor: open ? colors.accent : 'transparent' },
+ { justifyContent: 'space-between' },
inset && { paddingLeft: 32 },
style,
]);
@@
- <Icon size={18} color={colors.text} style={{ marginLeft: 'auto' }} />
+ <Icon size={18} color={colors.text} />Also applies to: 38-43, 55-55
🤖 Prompt for AI Agents
In apps/expo-stylesheet/components/ui/dropdown-menu.tsx around lines 31-37 (also
applies to 38-43 and 55), the code uses marginLeft: 'auto' which is invalid on
React Native; remove string-based margin and implement proper flex layout
instead: replace marginLeft: 'auto' with layout-driven properties on the
parent/container (e.g., use flexDirection and justifyContent or set alignSelf:
'flex-end' or give the sibling a flex: 1 so the element aligns to the right).
Update the affected styles and any container markup so alignment is controlled
by flex properties rather than a string margin value.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/expo-stylesheet/lib/utils.ts (2)
1-6: Type userStyle with StyleProp and handle true arrays directlyImproves type-safety and preserves registered RN styles; avoid Object.values() for real arrays.
Apply:
-import { ViewStyle, TextStyle } from 'react-native'; +import { ViewStyle, TextStyle, StyleProp } from 'react-native'; export const mergeBaseStyleWithUserStyle = <TemplateStyle extends ViewStyle | TextStyle>( - baseStyle: (TemplateStyle | false | undefined | null)[], - userStyle: any -): (TemplateStyle | false | undefined | null)[] => { - if (isViewStyleArray(userStyle)) { + baseStyle: (TemplateStyle | false | undefined | null)[], + userStyle: StyleProp<TemplateStyle> +): (TemplateStyle | false | undefined | null)[] => { + if (Array.isArray(userStyle)) { + return [...baseStyle, ...userStyle]; + } + if (isViewStyleArray(userStyle)) { // unfortunately, we can't use '...style' directly. // See isViewStyleArray() function for more details. return [...baseStyle, ...Object.values(userStyle as object)]; } - return [...baseStyle, userStyle]; + return [...baseStyle, userStyle as any]; }
17-19: Good guardrails for null and true arraysNull-safe check and early Array.isArray() return fix the earlier crash and branching.
apps/expo-stylesheet/app/(components)/table.tsx (2)
178-186: fontWeight strings look goodThanks for switching to string weights; fixes TS/runtime issues.
216-219: Text alignment fix LGTMUsing textAlign instead of alignItems on Text is correct.
🧹 Nitpick comments (4)
apps/expo-stylesheet/lib/utils.ts (1)
21-36: Use hasOwnProperty to avoid scanning keysMinor: faster and clearer than keysArray.includes('0').
- const keysArray = Object.keys(obj); + const keysArray = Object.keys(obj); @@ - if (keysArray.includes('0')) return true; + if (Object.prototype.hasOwnProperty.call(obj, '0')) return true;apps/expo-stylesheet/app/(components)/table.tsx (3)
92-101: Provide FlatList keyExtractor and drop per-row keyLets FlatList manage stable keys; avoids default index keys.
- <FlatList + <FlatList data={INVOICES} contentContainerStyle={{ paddingBottom: insets.bottom }} showsVerticalScrollIndicator={false} + keyExtractor={(item) => item.invoice} renderItem={({ item: invoice, index }) => { return ( <TableRow - key={invoice.invoice} style={[index % 2 !== 0 && { backgroundColor: colors.accentMild }]} >
132-151: Derive total from data instead of hard-coding $2,500.00Prevents drift when INVOICES changes.
export default function TableScreen() { const { width } = useWindowDimensions(); @@ const { colors } = useTheme() as ICustomTheme; + const totalAmount = React.useMemo(() => { + return INVOICES.reduce((sum, i) => { + const n = parseFloat(String(i.totalAmount).replace(/[^\d.-]/g, '')) || 0; + return sum + n; + }, 0); + }, []); + @@ > - <Text>$2,500.00</Text> + <Text> + {`$${totalAmount.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 })}`} + </Text> </Button>Also applies to: 222-319
133-151: Footer alignment: match column widths for consistent gridCurrent flex-based footer cells may misalign with header widths.
- <TableCell style={styles.tableFooterCell}> + <TableCell style={[styles.tableFooterCell, { width: columnWidths[0] }]}> <Text>Total</Text> </TableCell> - <TableCell style={styles.tableFooterCellAmount}> + <TableCell style={[styles.tableFooterCellAmount, { width: columnWidths[3] }]}> <ButtontableFooterCell: { - flex: 1, justifyContent: 'center', },Also applies to: 204-211
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/expo-stylesheet/app/(components)/dialog.tsx(1 hunks)apps/expo-stylesheet/app/(components)/table.tsx(1 hunks)apps/expo-stylesheet/components/ui/button.tsx(1 hunks)apps/expo-stylesheet/components/ui/checkbox.tsx(1 hunks)apps/expo-stylesheet/components/ui/dropdown-menu.tsx(1 hunks)apps/expo-stylesheet/components/ui/input.tsx(1 hunks)apps/expo-stylesheet/components/ui/navigation-menu.tsx(1 hunks)apps/expo-stylesheet/lib/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/expo-stylesheet/components/ui/navigation-menu.tsx
- apps/expo-stylesheet/components/ui/dropdown-menu.tsx
- apps/expo-stylesheet/components/ui/checkbox.tsx
- apps/expo-stylesheet/components/ui/input.tsx
- apps/expo-stylesheet/components/ui/button.tsx
- apps/expo-stylesheet/app/(components)/dialog.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/expo-stylesheet/app/(components)/table.tsx (1)
apps/expo-stylesheet/lib/constants.ts (1)
ICustomTheme(100-100)
🔇 Additional comments (1)
apps/expo-stylesheet/app/(components)/table.tsx (1)
55-58: Retaingap– supported since RN 0.71, project uses RN 0.79.2
gapwas added in RN 0.71 (per v0.71.0 changelog) and is documented in current Layout Props; Expo SDK 53 uses RN 0.79.2, so it’s safe to keepgaprather than adding manual margins. (github.com, reactnative.university)Likely an incorrect or invalid review comment.
An almost one to one copy of the example app
"expo-nativewind"but without using any tailwind/nativewind. Solely relying on css.Motivation:
I was looking for RN components library which is very customizable and I can just copy/paste the code from docs. But didn't use any additional packages like nativewind or tailwind. This library caught my eye. After going through this repo. I realized example app
"expo-stylesheet"was not fully completed. It only has one component that was"Select". So, I decided to add rest of the components. While taking reference from example app"expo-nativewind"Changes made:
"expo-nativewind".Limitations:
:hover,:active,:focus.StyleSheet.flatten()at various places to make components web compatible. As@radix-uidoesn't support passing styles as an array. And I was getting errorFailed to set an indexed property [0] on 'CSSStyleDeclaration': Indexed property setter is not supported.Start the Expo Stylesheet Example app
Summary by CodeRabbit
New Features
Refactor
Chores