feat: add all rn-primitives components to native-ui package#89
feat: add all rn-primitives components to native-ui package#89adelrodriguez wants to merge 1 commit intomainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| className={cn( | ||
| "overflow-hidden rounded-md border border-border bg-popover p-1 shadow-lg shadow-black/5", | ||
| Platform.select({ | ||
| web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-context-menu-content-transform-origin) z-50 min-w-[8rem]", |
There was a problem hiding this comment.
Wrong CSS variable for dropdown menu
DropdownMenuSubContent references --radix-context-menu-content-transform-origin, which is the CSS variable for context menus, not dropdown menus. On web, this means the transform origin will not be set (the variable won't be defined by Radix's dropdown menu), so the animation will fall back to the default origin. The correct variable is --radix-dropdown-menu-content-transform-origin.
The same issue appears in DropdownMenuContent on line 128, which also references --radix-context-menu-content-available-height and --radix-context-menu-content-transform-origin.
| web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-context-menu-content-transform-origin) z-50 min-w-[8rem]", | |
| web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-dropdown-menu-content-transform-origin) z-50 min-w-[8rem]", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/native-ui/src/components/dropdown-menu.tsx
Line: 82
Comment:
**Wrong CSS variable for dropdown menu**
`DropdownMenuSubContent` references `--radix-context-menu-content-transform-origin`, which is the CSS variable for context menus, not dropdown menus. On web, this means the transform origin will not be set (the variable won't be defined by Radix's dropdown menu), so the animation will fall back to the default origin. The correct variable is `--radix-dropdown-menu-content-transform-origin`.
The same issue appears in `DropdownMenuContent` on line 128, which also references `--radix-context-menu-content-available-height` and `--radix-context-menu-content-transform-origin`.
```suggestion
web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-dropdown-menu-content-transform-origin) z-50 min-w-[8rem]",
```
How can I resolve this? If you propose a fix, please make it concise.| "min-w-[8rem] overflow-hidden rounded-md border border-border bg-popover p-1 shadow-lg shadow-black/5", | ||
| Platform.select({ | ||
| web: cn( | ||
| "z-50 max-h-(--radix-context-menu-content-available-height) origin-(--radix-context-menu-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95", |
There was a problem hiding this comment.
Wrong CSS variables (copy-paste from context-menu)
Both --radix-context-menu-content-available-height and --radix-context-menu-content-transform-origin are incorrect here. These should be the dropdown menu equivalents. These variables won't be defined by Radix's DropdownMenu component on web, causing max-height to be unset (potentially overflowing the viewport) and the transform origin to default.
| "z-50 max-h-(--radix-context-menu-content-available-height) origin-(--radix-context-menu-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95", | |
| "z-50 max-h-(--radix-dropdown-menu-content-available-height) origin-(--radix-dropdown-menu-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/native-ui/src/components/dropdown-menu.tsx
Line: 128
Comment:
**Wrong CSS variables (copy-paste from context-menu)**
Both `--radix-context-menu-content-available-height` and `--radix-context-menu-content-transform-origin` are incorrect here. These should be the dropdown menu equivalents. These variables won't be defined by Radix's `DropdownMenu` component on web, causing `max-height` to be unset (potentially overflowing the viewport) and the transform origin to default.
```suggestion
"z-50 max-h-(--radix-dropdown-menu-content-available-height) origin-(--radix-dropdown-menu-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95",
```
How can I resolve this? If you propose a fix, please make it concise.| className={cn( | ||
| "overflow-hidden rounded-md border border-border bg-popover p-1 shadow-lg shadow-black/5", | ||
| Platform.select({ | ||
| web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-context-menu-content-transform-origin) z-50 min-w-[8rem]", |
There was a problem hiding this comment.
Wrong CSS variable for menubar sub-content
Same copy-paste issue as in dropdown-menu.tsx — MenubarSubContent references --radix-context-menu-content-transform-origin instead of --radix-menubar-content-transform-origin. This means the transform-origin animation won't work correctly on web.
| web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-context-menu-content-transform-origin) z-50 min-w-[8rem]", | |
| web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-menubar-content-transform-origin) z-50 min-w-[8rem]", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/native-ui/src/components/menubar.tsx
Line: 155
Comment:
**Wrong CSS variable for menubar sub-content**
Same copy-paste issue as in `dropdown-menu.tsx` — `MenubarSubContent` references `--radix-context-menu-content-transform-origin` instead of `--radix-menubar-content-transform-origin`. This means the transform-origin animation won't work correctly on web.
```suggestion
web: "animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 fade-in-0 data-[state=closed]:zoom-out-95 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--radix-menubar-content-transform-origin) z-50 min-w-[8rem]",
```
How can I resolve this? If you propose a fix, please make it concise.| "min-w-[12rem] overflow-hidden rounded-md border border-border bg-popover p-1 shadow-lg shadow-black/5", | ||
| Platform.select({ | ||
| web: cn( | ||
| "z-50 max-h-(--radix-context-menu-content-available-height) origin-(--radix-context-menu-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95", |
There was a problem hiding this comment.
Wrong CSS variables for menubar content
MenubarContent uses --radix-context-menu-content-available-height and --radix-context-menu-content-transform-origin, which are the context menu CSS variables. These should be --radix-menubar-content-available-height and --radix-menubar-content-transform-origin respectively.
| "z-50 max-h-(--radix-context-menu-content-available-height) origin-(--radix-context-menu-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95", | |
| "z-50 max-h-(--radix-menubar-content-available-height) origin-(--radix-menubar-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/native-ui/src/components/menubar.tsx
Line: 194
Comment:
**Wrong CSS variables for menubar content**
`MenubarContent` uses `--radix-context-menu-content-available-height` and `--radix-context-menu-content-transform-origin`, which are the context menu CSS variables. These should be `--radix-menubar-content-available-height` and `--radix-menubar-content-transform-origin` respectively.
```suggestion
"z-50 max-h-(--radix-menubar-content-available-height) origin-(--radix-menubar-content-transform-origin) animate-in cursor-default fade-in-0 zoom-in-95",
```
How can I resolve this? If you propose a fix, please make it concise.| if (Platform.OS === "web") { | ||
| return props.children as React.ReactNode | ||
| } |
There was a problem hiding this comment.
Web branch drops all props except children
When Platform.OS === "web", this returns props.children directly, silently discarding any other props (className, style, pointerEvents, etc.). This is used in MenubarContent with style={StyleSheet.absoluteFill} and pointerEvents="box-none", which would be lost on web.
Consider wrapping children in a View on web when layout-affecting props are present, or at least documenting that callers must not rely on non-animation props being applied on web.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/native-ui/src/components/native-only-animated-view.tsx
Line: 16-18
Comment:
**Web branch drops all props except `children`**
When `Platform.OS === "web"`, this returns `props.children` directly, silently discarding any other props (`className`, `style`, `pointerEvents`, etc.). This is used in `MenubarContent` with `style={StyleSheet.absoluteFill}` and `pointerEvents="box-none"`, which would be lost on web.
Consider wrapping children in a `View` on web when layout-affecting props are present, or at least documenting that callers must not rely on non-animation props being applied on web.
How can I resolve this? If you propose a fix, please make it concise.
adelrodriguez
force-pushed
the
02-14-feat_add_all_rn-primitives_components_to_native-ui_package
branch
2 times, most recently
from
c870d89 to
9949e29
Compare
adelrodriguez
force-pushed
the
02-14-feat_add_all_rn-primitives_components_to_native-ui_package
branch
from
9949e29 to
98b7ee3
Compare
1 participant
Greptile Summary
This PR consolidates all rn-primitives components into the
native-uipackage, adding 20+ new UI components (accordion, alert-dialog, avatar, badge, card, checkbox, context-menu, dialog, dropdown-menu, hover-card, input, label, menubar, popover, progress, radio-group, select, separator, skeleton, switch, tabs, textarea, toggle, toggle-group, tooltip). The implementation removes duplicate components from the mobile app and centralizes them in the shared package.Key Changes
@rn-primitivescomponents topackage.jsonapps/mobile/src/app/index.tsxshowcasing all componentslarge-title-headercomplex implementation (simplified to basic export)alert,avatar,toggle)Issues Found
dropdown-menu.tsxandmenubar.tsxuse wrong Radix CSS variables (--radix-context-menu-*instead of component-specific variables), breaking web animationsNativeOnlyAnimatedViewreturns only children on web, discarding layout props likestyleandpointerEventsConfidence Score: 3/5
dropdown-menu.tsxandmenubar.tsxwhere context-menu variables are used instead of the correct component-specific variables. These will cause animation issues on web. Additionally,NativeOnlyAnimatedViewsilently drops props on web, which could lead to unexpected layout issues. Most other components are well-implemented.packages/native-ui/src/components/dropdown-menu.tsxandpackages/native-ui/src/components/menubar.tsxneed CSS variable fixes before mergeImportant Files Changed
@rn-primitivescomponents (accordion, alert-dialog, avatar, etc.) and moved some deps to devDependencies/peerDependenciesFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[native-ui Package] --> B[Primitive Wrappers] A --> C[Custom Components] B --> D[rn-primitives accordion] B --> E[rn-primitives alert-dialog] B --> F[rn-primitives avatar] B --> G[rn-primitives checkbox] B --> H[rn-primitives context-menu] B --> I[rn-primitives dialog] B --> J[rn-primitives dropdown-menu] B --> K[rn-primitives menubar] B --> L[rn-primitives select] B --> M[15+ more primitives] C --> N[Alert] C --> O[Badge] C --> P[Card] C --> Q[Skeleton] R[Mobile App] --> |imports| A S[Web App] --> |imports| A A --> T[Platform-Specific Handling] T --> U[NativeOnlyAnimatedView] T --> V[FullWindowOverlay iOS] T --> W[Platform.select CSS] style J fill:#ff9999 style K fill:#ff9999 style U fill:#ffcc99Last reviewed commit: 98b7ee3