refactor: 将 Button 和 Switch 从 AntD 迁移到 ReactNativeElementUI - #292
refactor: 将 Button 和 Switch 从 AntD 迁移到 ReactNativeElementUI#292lqx-nancy wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
Walkthrough本次更新将本地 Button 和 Switch 组件切换为 RNEUI 实现,显式定义按钮与开关属性类型,并更新课程表、通知及设置模块的组件引用和加载、操作逻辑。 ChangesUI 控件迁移
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/button/index.tsx (1)
25-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win加载指示器颜色硬编码为白色,未随主题适配。
loadingProps={{ color: '#fff' }}固定为白色,如果currentStyle?.button_style在某些主题下设置了浅色/白色背景,加载指示器可能与背景对比度不足而难以辨认。建议让加载指示器颜色跟随currentStyle(例如从主题中派生,或允许通过 prop 覆盖),而不是固定写死。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/button/index.tsx` around lines 25 - 26, Update the loadingProps color in the button component to derive from currentStyle or the active theme instead of hard-coding '`#fff`', while preserving support for an explicit prop override if the component already exposes one. Ensure the loading indicator remains sufficiently contrasted across button themes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/button/index.tsx`:
- Around line 19-24: Update the RNEButton props in the component to merge style
into buttonStyle, preserving the existing currentStyle?.button_style and
borderRadius defaults while allowing caller-provided visual properties such as
borderRadius to take effect; keep style in containerStyle only if container
styling is still required.
---
Nitpick comments:
In `@src/components/button/index.tsx`:
- Around line 25-26: Update the loadingProps color in the button component to
derive from currentStyle or the active theme instead of hard-coding '`#fff`',
while preserving support for an explicit prop override if the component already
exposes one. Ensure the loading indicator remains sufficiently contrasted across
button themes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e5ac576b-055b-44c3-b5d0-3886af4153b3
📒 Files selected for processing (9)
src/components/button/index.tsxsrc/components/button/type.tssrc/components/switch/index.tsxsrc/components/switch/type.tssrc/modules/courseTable/components/CourseDataForm.tsxsrc/modules/courseTable/components/courseTable/ModalContent.tsxsrc/modules/notification/component/NotiPicker.tsxsrc/modules/setting/components/pushSubscriptionItem.tsxsrc/modules/setting/components/selectStyle.tsx
| <RNEButton | ||
| title={children} | ||
| onPress={onPress} | ||
| disabled={disabled || isLoading} | ||
| buttonStyle={[currentStyle?.button_style, { borderRadius: 6 }]} | ||
| containerStyle={[{ width: 200, height: 40 }, style]} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
@rneui/themed Button buttonStyle vs containerStyle which controls visible background and borderRadius
💡 Result:
In the @rneui/themed Button component, the buttonStyle prop is the correct choice for controlling the visible background color and borderRadius of the button itself [1][2]. The two props serve distinct roles: buttonStyle: This prop is used to apply styles directly to the inner button component [1][3]. Consequently, it is where you should define properties like backgroundColor and borderRadius to affect the appearance of the button [2]. containerStyle: This prop is used to apply styles to the outer wrapper (container) that encapsulates the button [1][3]. It is primarily used for positioning the button, setting margins, or managing how the button interacts with surrounding layout elements [3]. Summary Table Prop | Purpose | Primary Use Case --- | --- | --- buttonStyle | Styles the inner touchable button [1][3] | Background color, borderRadius, border width, padding [2] containerStyle | Styles the outer component container [1][3] | Margins, layout positioning, alignment [3] When customizing the appearance of the button, you should use buttonStyle. If you find that the borderRadius is not appearing correctly, ensure you are applying it to the buttonStyle prop [2]. If you are using a container for layout purposes, keep those styles in containerStyle to avoid layout conflicts [3].
Citations:
- 1: https://reactnativeelements.com/docs/components/button
- 2: https://stackoverflow.com/questions/61385753/problem-with-border-radius-button-react-native-elements
- 3: https://reactnativeelements.com/docs/next/customizing
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/components/button/index.tsx ==\n'
cat -n src/components/button/index.tsx
printf '\n== selectStyle.tsx matches ==\n'
rg -n "style=.*borderRadius|<Button|buttonStyle|containerStyle" src -g 'selectStyle.tsx' -A 4 -B 4Repository: Muxi-X/ccnubox_rn
Length of output: 3504
style 应同时合并到 buttonStyle
containerStyle 只影响外层容器;当前 style 传进来后不会作用到实际按钮。像 selectStyle.tsx 里传入的 borderRadius: 10 会被静默丢掉,按钮仍然固定是 { borderRadius: 6 }。把 style 也并入 buttonStyle 才能让调用方的视觉配置生效。
建议修改
- buttonStyle={[currentStyle?.button_style, { borderRadius: 6 }]}
+ buttonStyle={[currentStyle?.button_style, { borderRadius: 6 }, style]}
containerStyle={[{ width: 200, height: 40 }, style]}📝 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.
| <RNEButton | |
| title={children} | |
| onPress={onPress} | |
| disabled={disabled || isLoading} | |
| buttonStyle={[currentStyle?.button_style, { borderRadius: 6 }]} | |
| containerStyle={[{ width: 200, height: 40 }, style]} | |
| <RNEButton | |
| title={children} | |
| onPress={onPress} | |
| disabled={disabled || isLoading} | |
| buttonStyle={[currentStyle?.button_style, { borderRadius: 6 }, style]} | |
| containerStyle={[{ width: 200, height: 40 }, style]} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/button/index.tsx` around lines 19 - 24, Update the RNEButton
props in the component to merge style into buttonStyle, preserving the existing
currentStyle?.button_style and borderRadius defaults while allowing
caller-provided visual properties such as borderRadius to take effect; keep
style in containerStyle only if container styling is still required.
There was a problem hiding this comment.
Pull request overview
本次 PR 将项目中部分 UI 组件从 AntD 迁移到 ReactNativeElementUI(@rneui/themed),并新增统一的 Switch 封装层,以减少对 AntD 的依赖并统一交互与样式入口。
Changes:
- 用
@rneui/themed重写Button封装层,并更新课程表提交按钮的 loading 传参方式 - 新增
Switch封装层,并将通知/推送订阅/样式设置页面的开关统一迁移到该封装 - 将课程表弹窗底部的 AntD ghost Button 替换为
TouchableOpacity
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/setting/components/selectStyle.tsx | 将开关组件切换到新的 Switch 封装层 |
| src/modules/setting/components/pushSubscriptionItem.tsx | 将推送订阅开关切换到新的 Switch 封装层 |
| src/modules/notification/component/NotiPicker.tsx | 将通知选择器中的开关切换到新的 Switch 封装层 |
| src/modules/courseTable/components/courseTable/ModalContent.tsx | 将弹窗 footer 的 AntD ghost Button 替换为 TouchableOpacity |
| src/modules/courseTable/components/CourseDataForm.tsx | 提交按钮迁移到新的 Button 封装,并改用 isLoading |
| src/components/switch/type.ts | 新增 SwitchProps 类型定义 |
| src/components/switch/index.tsx | 新增 Switch 封装实现(基于 @rneui/themed) |
| src/components/button/type.ts | 调整 ButtonProps 以适配 RNEUI Button(并扩展样式能力) |
| src/components/button/index.tsx | Button 封装实现从 RectButton 迁移到 @rneui/themed Button |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
-
- 自定义 Button 的 style 现在只传给 RNEUI containerStyle,borderRadius/backgroundColor 这类视觉样式不会作用到真实按钮本体,现有 selectStyle 等调用点会出现样式回归。建议显式区分 buttonStyle/containerStyle,或至少把需要影响按钮本体的 style 合并进 buttonStyle。
-
- loadingProps 里 spinner color 写死 #fff,浅色按钮主题下可能不可见。建议从主题/按钮背景推导,或暴露 loadingProps/loadingColor 覆盖入口。
-
- ModalContent 里把 AntD ghost Button 换成裸 TouchableOpacity 后没有复用 styles.touchableBtn,删除/编辑按钮点击区域会变小;建议补 style 和 accessibilityRole。
-
- ButtonProps 直接从 RectButtonProps 缩窄成手写接口,后续 testID/accessibility/nativeID 等常见按钮属性会丢失。若迁移到 RNEUI,建议基于 RNE Button 的 props 做 Omit/扩展,而不是只保留当前几个字段。Switch 的命名/注释问题是小修,但也可以顺手整理。
-Button 封装层从RectButton改为@rneui/themed Button
-ModalContent.tsx中AntD ghost 替换为 TouchableOpacity
-CourseDataForm.tsx改用新button
-新增switch封装层
-pushSubscriptionItem.tsx、NotiPicker.tsx、selectStyle.tsx 改为使用新 Switch
Summary by CodeRabbit