feat(popup): RN #2614
feat(popup): RN #2614
Conversation
Walkthrough本次更改涉及多个文件的更新，以支持 React Native (RN) 的功能。主要修改包括在 Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 14
🧹 Outside diff range and nitpick comments (12)
scripts/rn/copy-file.js (2)
10-10: 控制台日志改进得当，建议进一步优化这个改动通过添加脚本名称增加了日志输出的上下文，这是一个很好的改进。它有助于在多个脚本运行时更容易识别日志来源。
为了进一步提高可读性和一致性，建议使用模板字符串：
- console.log('rn copy-file.js args',args) + console.log(`rn copy-file.js args: ${JSON.stringify(args)}`)这样可以确保参数被正确格式化，特别是当
args包含复杂对象时。
Line range hint
1-85: 建议增强错误处理和日志记录脚本的整体结构良好，功能模块化，这有助于维护和理解。然而，有几个地方可以进行改进：
错误处理：虽然在一些函数中有错误处理，但不是所有的异步操作都有适当的错误处理。建议在所有的异步操作中添加 try-catch 块。
日志记录：考虑使用更结构化的日志记录方式，例如使用日志级别（info、warn、error）来区分不同类型的消息。
配置管理：考虑将硬编码的配置（如
childAdaptedArray）移到配置文件中，以提高可维护性。以下是一个改进错误处理和日志记录的示例：
const logger = { info: (message) => console.log(`[INFO] ${message}`), warn: (message) => console.warn(`[WARN] ${message}`), error: (message, error) => console.error(`[ERROR] ${message}`, error) }; const copyFile = async (from, to, success, isSingle = false) => { try { await fse.copy(from, to); logger.info(`${success}!> ${to}`); // ... rest of the function } catch (err) { logger.error('复制目录时发生错误', err); } };这些改进将使脚本更加健壮和易于调试。
src/packages/popup/demo.taro.tsx (1)
82-84: className 逻辑的改进很好，但可以考虑提高可读性新的 className 逻辑通过使用
harmonyAndRn()函数增加了对 Harmony OS 和 React Native 平台的支持，这是一个很好的改进。然而，嵌套的三元运算符可能会降低代码的可读性。建议：考虑将这个逻辑提取到一个单独的函数中，以提高可读性。例如：
const getScrollViewClassName = () => { if (harmonyAndRn()) return ''; return `demo ${Taro.getEnv() === 'WEB' ? 'web' : ''}`; }; // 然后在 JSX 中使用： <ScrollView className={getScrollViewClassName()}>这样可以使代码更易于理解和维护。
gulpfile.js (2)
Line range hint
23-49: 代码更新符合现代 JavaScript 实践，并增加了对 React Native 的支持这些更改很好地modernize了代码并扩展了功能：
- 使用箭头函数语法提高了一致性。
- 新增的 React Native 文件监视任务表明项目正在扩展跨平台支持。
建议：考虑将重复的
interval配置提取为一个常量，以提高可维护性。可以考虑应用以下更改：
+ const watchOptions = { interval: 500 }; gulp.watch( `src/packages/${argv}/demos/taro/*`, - interval, + watchOptions, gulp.series(`${argv}copyDemo`) ) // 对其他 watch 调用进行类似的更改这样可以在一个地方管理所有 watch 任务的间隔设置。
Line range hint
75-100: 语法更新一致，但存在未使用的注释代码这个更改总体上是好的：
- 将函数语法更新为箭头函数，与文件中的其他更改保持一致。
- 保留了任务的原有功能，确保了现有行为的稳定性。
建议：
考虑清理未使用的注释代码。如果这些注释对未来的开发有价值，可以将其移至文档中。否则，删除它们可以提高代码的可读性。
可以考虑删除或移动以下注释代码：
- // 导入全局变量文件，不好用 - // includePaths: [ - // 'src/styles/theme-default.scss', - // 'src/styles/mixins/text-ellipsis.scss', - // 'src/styles/variables.scss', - // ],如果这些信息很重要，可以将其添加到项目文档中。
packages/nutui-taro-demo/config/index.js (1)
88-92: 考虑 React Native 支持的整体影响这些更改表明项目正在增加对 React Native 的支持。这是一个重要的架构变更，可能会对整个项目产生广泛影响。
建议：
- 确保有完整的迁移计划，包括测试策略和回滚方案。
- 更新项目文档，清楚地说明如何在 Taro 和 React Native 环境中使用这些组件。
- 考虑是否需要为开发团队提供关于 React Native 开发的培训或指导。
- 审查并更新与这些变更相关的所有单元测试和集成测试。
- 考虑是否需要更新 CI/CD 流程以适应新的 React Native 构建和测试要求。
packages/nutui-taro-demo/scripts/taro/generate-nutui-taro.js (1)
18-18: 优化版本判断逻辑当前直接判断
version !== '3.0.0'，可能会在版本升级后影响代码维护。建议使用更灵活的版本控制，例如从配置文件或环境变量中获取版本信息。可以考虑以下修改：
- if (version !== '3.0.0') return + const currentVersion = require('../../nutui-react/package.json').version + if (version !== currentVersion) return或者使用版本范围判断：
- if (version !== '3.0.0') return + if (semver.lt(version, '3.0.0')) return（注意：需要确保已安装
semver库）src/packages/popup/popup.scss (3)
33-36: 检查嵌套的类名是否合理在
&-title内部再次嵌套&-title，生成的类名为.nut-popup-title-title。这可能会导致命名混淆，建议检查命名是否符合 BEM 规范，或者考虑重命名以提高代码可读性。
241-241: 修正注释格式以确保一致性在第 241 行，注释
/* #endif*/中缺少空格，建议改为/* #endif */。为确保预处理器正确解析，建议统一注释格式。
404-406: 修正 RTL 模式下的注释格式在第 406 行，注释
/* #endif*/缺少空格，建议改为/* #endif */。同时，确保 RTL 模式下的transform变换与预期一致。src/packages/popup/popup.harmony.css (2)
55-69: 统一动画类名的命名规范
.nut-overlay-slide-enter-active、.nut-overlay-slide-appear-active、.nut-overlay-slide-exit-active等类名较为冗长且相似，建议统一命名规范，或者使用更具描述性的名称，以提高代码的可读性和可维护性。
Line range hint
88-105: 简化标题区域的样式结构
.nut-popup-title及其子元素使用了多重 flex 布局，可能导致样式过于复杂。建议简化层级或合并一些样式规则，以提升渲染性能和代码简洁性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
🔇 Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
- gulpfile.js (5 hunks)
- package.json (2 hunks)
- packages/nutui-taro-demo/config/index.js (1 hunks)
- packages/nutui-taro-demo/scripts/taro/generate-nutui-taro.js (3 hunks)
- scripts/rn/copy-file.js (1 hunks)
- scripts/taro/generate-nutui-taro.js (3 hunks)
- src/config.json (1 hunks)
- src/packages/popup/demo.taro.tsx (3 hunks)
- src/packages/popup/index.rn.ts (1 hunks)
- src/packages/popup/popup.harmony.css (11 hunks)
- src/packages/popup/popup.rn.tsx (1 hunks)
- src/packages/popup/popup.scss (15 hunks)
🧰 Additional context used
Biome
src/packages/popup/popup.rn.tsx
[error] 44-44: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 149-152: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 160-162: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 167-168: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 173-176: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 177-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 184-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 190-193: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments not posted (21)
src/packages/popup/index.rn.ts (3)
4-4: 默认导出看起来正确。将 Popup 组件作为默认导出符合常见的 React/React Native 实践，这样可以在其他文件中方便地导入 Popup 组件。
3-3: 请验证 PopupProps 类型的兼容性。从 '.taro' 文件导出类型并在 '.rn' 文件中使用可能会导致不一致。请确保 './popup.taro' 中的 PopupProps 类型与 React Native 实现兼容。
建议运行以下脚本来比较两个文件中的 PopupProps 定义：
#!/bin/bash # 描述：比较 Taro 和 RN 文件中的 PopupProps 定义 # 测试：提取并比较 PopupProps 定义 echo "Taro PopupProps:" rg -t typescript "export.*interface.*PopupProps" -A 20 "src/packages/popup/popup.taro.(ts|tsx)" echo "\nRN PopupProps (if exists):" rg -t typescript "export.*interface.*PopupProps" -A 20 "src/packages/popup/popup.rn.(ts|tsx)"
1-1: 导入语句看起来正确。导入语句遵循了 React Native 特定导入的惯例。
请运行以下脚本来验证 './popup.rn' 文件的存在：
src/packages/popup/demo.taro.tsx (3)
6-6: 新导入看起来不错！
harmonyAndRn的导入似乎是为了处理特定平台的功能。这个添加看起来是合理的，因为它可能用于确定当前平台是 Harmony OS 还是 React Native。
104-104: 请解释为什么注释掉 Demo7注意到 Demo7 组件被注释掉了。虽然这可能是一个临时的更改，但没有解释说明原因可能会导致其他开发人员感到困惑。
建议：
- 如果 Demo7 不再需要，考虑完全删除它而不是注释掉。
- 如果这是一个临时更改，请添加一个注释解释原因，例如：
// TODO: Demo7 暂时被禁用，因为 [原因]。预计在 [日期/版本] 重新启用。 // <Demo7 />这将帮助其他开发人员理解这个更改的原因和预期持续时间。
Line range hint
1-110: 总体评价：代码改进良好，有小的优化空间这次更改主要涉及了对不同平台（包括 Harmony OS 和 React Native）的支持，这是一个很好的改进。主要的修改包括：
- 添加了
harmonyAndRn的导入，用于平台特定功能。- 修改了 ScrollView 的 className 逻辑，以适应更多的平台场景。
- 注释掉了 Demo7 组件。
总的来说，这些更改提高了代码的灵活性和适用性。然而，有一些小的改进建议：
- 考虑重构 ScrollView 的 className 逻辑，以提高可读性。
- 为被注释掉的 Demo7 组件添加解释或考虑完全移除它。
这些小的改进将进一步提高代码的可维护性和可读性。
gulpfile.js (3)
68-73: 语法更新保持一致性这个更改很好：
- 将函数语法更新为箭头函数，与文件中的其他更改保持一致。
- 保留了任务的原有功能，确保了现有行为的稳定性。
Line range hint
102-123: 多个任务的语法更新保持一致性这些更改很好：
- 将 'copyCss'、'copyTaroDemo' 和 'copyTaro' 任务的函数语法都更新为箭头函数，与文件中的其他更改保持一致。
- 保留了所有任务的原有功能，确保了现有行为的稳定性。
这种一致的更新方式有助于提高代码的可读性和可维护性。
124-129: 新增 React Native 支持任务，但需验证 CSS 导入兼容性新增的 'copyRN' 任务是一个很好的补充：
- 遵循了现有的任务结构和命名约定，保持了代码的一致性。
- 专门设计用于支持 React Native 开发，扩展了项目的跨平台能力。
然而，有一点需要注意：
请验证预置的 CSS 导入语句是否与 React Native 兼容。React Native 通常使用不同于 web 的样式系统。
如果验证显示没有使用 React Native 特定的样式方法，可能需要调整样式导入和使用方式。
packages/nutui-taro-demo/config/index.js (1)
92-92: 更新了 @nutui/icons-react-taro 的别名路径这个更改将 @nutui/icons-react-taro 的路径也更改为 React Native 实现。
请确保以下几点：
- React Native 版本的图标实现已经完成并经过测试。
- 所有使用 @nutui/icons-react-taro 的组件都与新的 RN 实现兼容。
- 项目文档已更新，反映了这一变化。
#!/bin/bash # 检查是否有任何文件仍在使用旧的 Taro 特定图标实现 rg --type js --type ts --type tsx "from\s+['\"]\@nutui/icons-react-taro" -g '!config/*'package.json (2)
197-197: 新增 React Native 依赖添加 React Native 依赖是一个重要的改动，表明项目正在扩展以支持 React Native 平台。这与 PR 的目标相符。
为确保顺利集成 React Native，请考虑以下建议：
- 确保项目中已经正确配置了 React Native 的开发环境。
- 检查是否需要添加其他 React Native 相关的依赖或开发工具。
- 考虑更新项目文档，说明如何在 React Native 环境中使用和测试组件。
129-133: Taro.js 包版本更新至稳定版本将 Taro.js 相关包从 alpha 版本更新到稳定版本 4.0.2 是一个很好的改进。这有助于提高项目的稳定性和可靠性。
请确保查看 Taro.js 4.0.2 版本的更新日志，以了解是否有任何破坏性变更或新特性需要在代码库中进行相应的调整。可以运行以下命令来检查项目中使用 Taro.js API 的地方：
✅ Verification successful
请使用以下修正后的命令来搜索项目中使用 Taro.js API 的文件：
批准 Taro.js 包版本更新
将 Taro.js 相关包从 alpha 版本升级到稳定版本 4.0.2 是一个积极的改进，能够提升项目的稳定性和可靠性。
请确保查阅 Taro.js 4.0.2 版本的更新日志，以确认是否存在任何破坏性变更或新功能需要在项目中进行相应的调整。考虑到项目中有多个文件使用了 Taro.js 的 API，建议重点检查以下方面：
- API 的使用是否发生了变化。
- 新增的功能是否需要在现有代码中进行集成或调整。
- 任何弃用的功能是否已经被替代或移除。
确保所有依赖 Taro.js 的模块在新版本下正常运行，并根据需要进行必要的代码修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索项目中使用 Taro.js API 的文件 rg --type typescript --type javascript "import.*from.*@tarojs"Length of output: 99
Script:
#!/bin/bash # 搜索项目中使用 Taro.js API 的文件，使用适当的文件扩展名 rg --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' "import.*from.*@tarojs"Length of output: 38354
src/config.json (1)
831-838: Popup 组件更新：版本号变更和 React Native 支持
版本号从 2.0.0 更新到 3.0.0，这表明有重大更新。请确保：
- 更新日志中详细记录了所有破坏性变更。
- 使用该组件的地方都已经适配新版本。
新增
"rn": true属性，表明 Popup 组件现在支持 React Native。
- 请确保 React Native 版本的实现已经过充分测试。
- 考虑更新文档，说明如何在 React Native 环境中使用该组件。
这些更改符合 PR 的目标，增强了组件的跨平台能力。
运行以下脚本来验证 Popup 组件的使用是否已更新：
scripts/taro/generate-nutui-taro.js (1)
85-86: 确认构建版本是否需要引入样式文件在第85-86行，
fileRNStr中包含了importScssStr，而在fileRNStrBuild（第73行）中并未包含。如果构建版本同样需要样式文件，建议在fileRNStrBuild中加入importScssStr，以确保样式在构建后正常应用。建议的修改：
let fileRNStrBuild = `${importRNStr} +${importScssStr} export { ${packages.join(',')} };`请确认是否需要在构建版本中引入样式文件。
src/packages/popup/popup.scss (2)
4-9: 请确认条件编译指令能正确解析您在样式中使用了
/* #ifdef rn */和/* #ifndef rn */等条件编译指令。请确认在构建过程中，有预处理器能够正确解析并处理这些指令，否则这些条件样式可能不会按预期生效。
223-225: 验证条件编译指令的作用在动画关键帧中，使用了
/* #ifndef rn */条件注释来包裹transform属性。请确认在 RN 环境中是否确实需要忽略这些transform，以及预处理器是否能够正确处理这些条件指令。Also applies to: 256-258, 289-291, 322-324
src/packages/popup/popup.harmony.css (5)
3-18: 优化 position 属性的条件编译在
.nut-overlay类中，根据不同环境使用了position: absolute;和position: fixed;。请确保在编译过程中，/* #ifdef rn */和/* #ifndef rn */条件能正确解析，以避免位置属性在不同平台上出现问题。
33-54: 验证动画关键帧在 React Native 的兼容性
@keyframes nut-fade-in和@keyframes nut-fade-out定义了渐入和渐出动画。请确认这些 CSS 动画在 React Native 环境中是否受支持。如有必要，针对 React Native 平台提供相应的替代方案。
72-82: 检查 .nut-popup 类的条件编译逻辑在
.nut-popup类中，同样使用了/* #ifdef rn */和/* #ifndef rn */进行条件编译，请确保这些条件在构建过程中能够正确识别，并且不同平台上的position属性符合预期。
262-271: 动画 transform 属性的条件编译在
@keyframes popup-slide-top-enter和popup-slide-top-exit中，transform属性被排除在 React Native 环境中。请确认在 React Native 中，需要替换为合适的动画实现，以保持用户体验一致。
297-306: 确保侧边弹出动画在各平台一致对于
@keyframes popup-slide-right-enter和popup-slide-right-exit，同样对transform属性进行了条件编译。请确认在 React Native 平台，侧边弹出动画能够正常工作，或提供替代方案。
| @@ -85,11 +85,11 @@ const config = { | |||
| '@/utils': path.resolve(__dirname, '../nutui-react/utils'), | |||
| '@nutui/nutui-react-taro': path.resolve( | |||
| __dirname, | |||
| '../nutui-react/packages/nutui.react.taro.ts' | |||
| '../nutui-react/packages/nutui.react.rn.ts' | |||
There was a problem hiding this comment.
💡 Codebase verification
发现仍在使用旧的 Taro 实现 @nutui/nutui-react-taro
以下文件仍在引入 @nutui/nutui-react-taro，请确保这些引用已更新为新的 React Native 实现，以保持项目的一致性和正确性：
- src/packages/form/demos/taro/demo7.tsx
- src/packages/form/demos/taro/demo4.tsx
- src/packages/form/demos/taro/demo6.tsx
- src/packages/form/demos/taro/demo5.tsx
- src/packages/form/demos/taro/demo3.tsx
- （以及其他相关文件）
🔗 Analysis chain
更新了 @nutui/nutui-react-taro 的别名路径
这个更改将 @nutui/nutui-react-taro 的路径从 Taro 特定实现更改为 React Native 实现。
请确保以下几点：
- React Native 版本的实现已经完成并经过测试。
- 所有使用 @nutui/nutui-react-taro 的组件都与新的 RN 实现兼容。
- 项目文档已更新，反映了这一变化。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有任何文件仍在使用旧的 Taro 特定实现
rg --type js --type ts --type tsx "from\s+['\"]\@nutui/nutui-react-taro" -g '!config/*'
Length of output: 204
Script:
#!/bin/bash
# 检查是否有任何文件仍在使用旧的 Taro 特定实现 @nutui/nutui-react-taro
rg --type js --type ts --type tsx "from\s+['\"]@nutui/nutui-react-taro['\"]" -g '!config/*'
Length of output: 125
Script:
#!/bin/bash
# 检查是否有任何文件仍在使用旧的 Taro 特定实现 @nutui/nutui-react-taro
rg "from\s+['\"]@nutui/nutui-react-taro['\"]" -g '*.js' -g '*.ts' -g '*.tsx' -g '!config/*'
Length of output: 58615
Script:
#!/bin/bash
# 检查是否有任何文件仍在使用旧的 Taro 特定实现 @nutui/nutui-react-taro
rg "from\s+['\"]@nutui/nutui-react-taro['\"]" -g '*.js' -g '*.ts' -g '*.tsx' -g '!config/*'
Length of output: 58615
| importRNStr += `import ${name} from '@/packages/${name.toLowerCase()}/index.${rn?'rn':'taro'}'\n` | ||
| importRNStr += `export * from '@/packages/${name.toLowerCase()}/index.${rn?'rn':'taro'}'\n` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
提取重复代码，增强可维护性
importStr 和 importRNStr 的生成逻辑存在重复，建议提取成函数，减少代码冗余，提高代码的可读性和可维护性。
可以重构为：
function generateImportStrings(name, type) {
return `import ${name} from '@/packages/${name.toLowerCase()}/index.${type}'\nexport * from '@/packages/${name.toLowerCase()}/index.${type}'\n`;
}
importStr += generateImportStrings(name, 'taro');
importRNStr += generateImportStrings(name, rn ? 'rn' : 'taro');| let fileRNStr = `${importRNStr} | ||
| ${importScssStr} | ||
| export { ${packages.join(',')} };` | ||
| fs.outputFile( | ||
| path.resolve(__dirname, '../../nutui-react/packages/nutui.react.rn.ts'), | ||
| fileRNStr, | ||
| 'utf8', | ||
| (error) => { | ||
| if (error) throw error | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
封装文件输出逻辑，减少代码重复
多个文件的输出逻辑相似，建议封装成通用函数，避免代码重复，提高代码可维护性。
可以考虑如下重构：
function writeOutputFile(filePath, content) {
fs.outputFile(
path.resolve(__dirname, filePath),
content,
'utf8',
(error) => {
if (error) throw error;
}
);
}
writeOutputFile('../../nutui-react/packages/nutui.react.taro.ts', fileStr);
writeOutputFile('../../nutui-react/packages/nutui.react.rn.ts', fileRNStr);| @@ -12,14 +13,17 @@ const raws = [] | |||
|
|
|||
| config.nav.map((item) => { | |||
| item.packages.forEach((element) => { | |||
| let { name, show, type, taro, exportEmpty, exclude, version } = element | |||
| let { name, show, type, taro, exportEmpty, exclude, version, rn } = element | |||
There was a problem hiding this comment.
建议为解构赋值添加默认值
在对 element 进行解构赋值时，rn 属性可能不存在，导致 rn 为 undefined。建议为 rn 设置默认值，确保后续使用安全。
可以应用以下修改：
- let { name, show, type, taro, exportEmpty, exclude, version, rn } = element
+ let { name, show, type, taro, exportEmpty, exclude, version, rn = false } = element📝 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.
| let { name, show, type, taro, exportEmpty, exclude, version, rn } = element | |
| let { name, show, type, taro, exportEmpty, exclude, version, rn = false } = element |
|
|
||
| let fileRNStrBuild = `${importRNStr} | ||
| export { ${packages.join(',')} };` | ||
|
|
||
| fs.outputFile( | ||
| path.resolve(__dirname, '../../src/packages/nutui.react.build.rn.ts'), | ||
| fileRNStrBuild, | ||
| 'utf8', | ||
| (error) => { | ||
| if (error) throw error | ||
| } | ||
| ) | ||
|
|
||
| let fileRNStr = `${importRNStr} | ||
| ${importScssStr} | ||
| export { ${packages.join(',')} };` | ||
| fs.outputFile( | ||
| path.resolve(__dirname, '../../src/packages/nutui.react.rn.ts'), | ||
| fileRNStr, | ||
| 'utf8', | ||
| (error) => { | ||
| if (error) throw error | ||
| } | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议提取重复的文件生成逻辑
在第71-94行，针对 RN 的 fileRNStrBuild 和 fileRNStr 的文件生成逻辑与之前 Taro 平台的文件生成部分非常相似，存在代码重复。为了提高代码的可维护性和减少冗余，建议将文件生成的逻辑提取到一个通用的函数中，通过参数来控制生成的内容和文件路径。
示例代码：
function outputFile(filePath, content) {
fs.outputFile(
path.resolve(__dirname, filePath),
content,
'utf8',
(error) => {
if (error) throw error;
}
);
}
// 生成 RN 平台的构建文件
outputFile('../../src/packages/nutui.react.build.rn.ts', fileRNStrBuild);
// 生成 RN 平台的开发文件
outputFile('../../src/packages/nutui.react.rn.ts', fileRNStr);
// 如果需要生成 Taro 平台的文件，也可以调用同一个函数
outputFile('../../src/packages/nutui.react.build.taro.ts', fileStrBuild);
outputFile('../../src/packages/nutui.react.taro.ts', fileStr);| /* #ifdef rn */ | ||
| height: 300px; | ||
| /* #endif */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
调整弹出层高度以适配不同设备
在 .nut-popup-bottom 类中，针对 React Native 平台设置了固定高度 height: 300px;。建议根据不同设备的屏幕尺寸进行自适应，避免在小屏幕设备上显示异常。
| /* #ifndef rn */ | ||
| transform: translate(50%, -50%); | ||
| /* #endif*/ |
There was a problem hiding this comment.
修正 RTL 模式下的居中定位
在 RTL 方向下，.nut-popup-center 的 transform 属性仅在非 React Native 环境下生效。请确保在 React Native 的 RTL 环境中，弹出层也能正确居中显示。
| onOpen && onOpen() | ||
| } | ||
|
|
||
| const close = () => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议使用可选链操作符简化函数调用
在多处地方，可以使用可选链操作符 ?. 来简化代码，提高可读性。例如，将 onOpen && onOpen() 替换为 onOpen?.()。
建议如下修改：
- onOpen && onOpen()
+ onOpen?.()
- onClose && onClose()
+ onClose?.()
- const closed = onOverlayClick && onOverlayClick(e)
+ const closed = onOverlayClick?.(e)
- onClick && onClick(e)
+ onClick?.(e)
- const closed = onCloseIconClick && onCloseIconClick(e)
+ const closed = onCloseIconClick?.(e)
- afterShow && afterShow()
+ afterShow?.()
- afterClose && afterClose()
+ afterClose?.()Also applies to: 160-162, 167-168, 173-176, 177-178, 184-187, 190-193
🧰 Tools
Biome
[error] 149-152: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| onOverlayClick: (e: ITouchEvent) => boolean | void | ||
| onCloseIconClick: (e: ITouchEvent) => boolean | void |
There was a problem hiding this comment.
建议将返回类型中的 'void' 替换为 'undefined'
在 TypeScript 中，在联合类型中使用 void 可能会引起混淆。将返回类型从 boolean | void 更改为 boolean | undefined，可以提高代码的可读性和明确性。
建议如下修改：
- onOverlayClick: (e: ITouchEvent) => boolean | void
+ onOverlayClick: (e: ITouchEvent) => boolean | undefined
- onCloseIconClick: (e: ITouchEvent) => boolean | void
+ onCloseIconClick: (e: ITouchEvent) => boolean | undefined📝 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.
| onOverlayClick: (e: ITouchEvent) => boolean | void | |
| onCloseIconClick: (e: ITouchEvent) => boolean | void | |
| onOverlayClick: (e: ITouchEvent) => boolean | undefined | |
| onCloseIconClick: (e: ITouchEvent) => boolean | undefined |
🧰 Tools
Biome
[error] 44-44: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 45-45: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/packages/popup/popup.rn.tsx
Outdated
| const renderPopByRN = () => { | ||
| return ( | ||
| <Modal | ||
| // animationType="slide" // 使用 slide 动画 | ||
| transparent // 使背景透明 | ||
| visible={innerVisible} | ||
| onRequestClose={onHandleClickOverlay} // 处理 Android 设备的返回按钮 | ||
| > | ||
| <View | ||
| style={{ | ||
| flex: 1, | ||
| justifyContent: 'flex-end', | ||
| backgroundColor: 'rgba(0, 0, 0, 0.5)', | ||
| }} | ||
| > | ||
| <View | ||
| style={{ | ||
| backgroundColor: 'white', | ||
| padding: 20, | ||
| borderTopLeftRadius: 20, | ||
| borderTopRightRadius: 20, | ||
| alignItems: 'center', | ||
| }} | ||
| > | ||
| <Text | ||
| style={{ | ||
| marginBottom: 20, | ||
| fontSize: 18, | ||
| }} | ||
| > | ||
| 这是一个从底部弹出的弹框！------ | ||
| </Text> | ||
| <TouchableOpacity onPress={onHandleClickOverlay}> | ||
| <Text>关闭</Text> | ||
| </TouchableOpacity> | ||
| </View> | ||
| </View> | ||
| </Modal> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议将内联样式提取到 StyleSheet
在 renderPopByRN 函数中，存在大量的内联样式。为了提高性能和代码的可维护性，建议使用 StyleSheet.create 将样式提取出来。
可以按以下方式修改：
+ import { StyleSheet } from 'react-native';
+ const styles = StyleSheet.create({
+ modalBackground: {
+ flex: 1,
+ justifyContent: 'flex-end',
+ backgroundColor: 'rgba(0, 0, 0, 0.5)',
+ },
+ modalContent: {
+ backgroundColor: 'white',
+ padding: 20,
+ borderTopLeftRadius: 20,
+ borderTopRightRadius: 20,
+ alignItems: 'center',
+ },
+ modalText: {
+ marginBottom: 20,
+ fontSize: 18,
+ },
+ closeText: {
+ color: '#007AFF',
+ fontSize: 16,
+ },
+ });
- <View
- style={{
- flex: 1,
- justifyContent: 'flex-end',
- backgroundColor: 'rgba(0, 0, 0, 0.5)',
- }}
- >
+ <View style={styles.modalBackground}>
- <View
- style={{
- backgroundColor: 'white',
- padding: 20,
- borderTopLeftRadius: 20,
- borderTopRightRadius: 20,
- alignItems: 'center',
- }}
- >
+ <View style={styles.modalContent}>
- <Text
- style={{
- marginBottom: 20,
- fontSize: 18,
- }}
- >
+ <Text style={styles.modalText}>将内联样式替换为样式表，可以使代码更加整洁，并有助于性能优化。
📝 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.
| const renderPopByRN = () => { | |
| return ( | |
| <Modal | |
| // animationType="slide" // 使用 slide 动画 | |
| transparent // 使背景透明 | |
| visible={innerVisible} | |
| onRequestClose={onHandleClickOverlay} // 处理 Android 设备的返回按钮 | |
| > | |
| <View | |
| style={{ | |
| flex: 1, | |
| justifyContent: 'flex-end', | |
| backgroundColor: 'rgba(0, 0, 0, 0.5)', | |
| }} | |
| > | |
| <View | |
| style={{ | |
| backgroundColor: 'white', | |
| padding: 20, | |
| borderTopLeftRadius: 20, | |
| borderTopRightRadius: 20, | |
| alignItems: 'center', | |
| }} | |
| > | |
| <Text | |
| style={{ | |
| marginBottom: 20, | |
| fontSize: 18, | |
| }} | |
| > | |
| 这是一个从底部弹出的弹框！------ | |
| </Text> | |
| <TouchableOpacity onPress={onHandleClickOverlay}> | |
| <Text>关闭</Text> | |
| </TouchableOpacity> | |
| </View> | |
| </View> | |
| </Modal> | |
| ) | |
| } | |
| import { StyleSheet } from 'react-native'; | |
| const styles = StyleSheet.create({ | |
| modalBackground: { | |
| flex: 1, | |
| justifyContent: 'flex-end', | |
| backgroundColor: 'rgba(0, 0, 0, 0.5)', | |
| }, | |
| modalContent: { | |
| backgroundColor: 'white', | |
| padding: 20, | |
| borderTopLeftRadius: 20, | |
| borderTopRightRadius: 20, | |
| alignItems: 'center', | |
| }, | |
| modalText: { | |
| marginBottom: 20, | |
| fontSize: 18, | |
| }, | |
| closeText: { | |
| color: '#007AFF', | |
| fontSize: 16, | |
| }, | |
| }); | |
| const renderPopByRN = () => { | |
| return ( | |
| <Modal | |
| // animationType="slide" // 使用 slide 动画 | |
| transparent // 使背景透明 | |
| visible={innerVisible} | |
| onRequestClose={onHandleClickOverlay} // 处理 Android 设备的返回按钮 | |
| > | |
| <View style={styles.modalBackground}> | |
| <View style={styles.modalContent}> | |
| <Text style={styles.modalText}> | |
| 这是一个从底部弹出的弹框！------ | |
| </Text> | |
| <TouchableOpacity onPress={onHandleClickOverlay}> | |
| <Text style={styles.closeText}>关闭</Text> | |
| </TouchableOpacity> | |
| </View> | |
| </View> | |
| </Modal> | |
| ) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
src/packages/popup/demos/taro/demo1.tsx (1)
27-27: 使用 Text 组件是个好改进，但内容可能需要更丰富将
View替换为Text来显示纯文本内容是一个很好的改进，这样更符合语义，可能会有更好的性能表现。不过，仅显示"正文"可能不足以充分演示组件的功能。考虑添加更多有意义的内容来更好地展示 Popup 组件的用途。
考虑使用以下方式来丰富内容：
- <Text>正文</Text> + <Text>这是一个弹出层示例，可以包含更多内容。例如：{`项目 ${i + 1}`}</Text>Also applies to: 30-30
src/packages/popup/demos/taro/demo3.tsx (1)
26-35: 条件渲染逻辑正确，但可以考虑改进用户体验条件渲染的实现正确，可以有效地在 Harmony OS 或 React Native 平台上隐藏"自定义图标"功能。这符合增强 React Native 支持的 PR 目标。
然而，完全移除某些平台上的功能可能会影响用户体验。建议考虑以下改进：
- 为不支持的平台提供替代功能。
- 如果无法提供替代功能，可以显示一条消息解释为什么该功能在当前平台不可用。
这样可以确保所有用户都能得到良好的体验，同时了解功能差异的原因。
考虑将条件渲染改为：
{harmonyAndRn() ? ( <Cell title="平台不支持自定义图标" /> ) : ( <Cell title="自定义图标" onClick={() => { setShowIconDefine(true) }} /> )}这样可以在不支持的平台上提供信息，而不是完全隐藏该选项。
src/packages/popup/popup.rn.tsx (1)
298-329: 建议清理或解释注释掉的属性在
renderNode函数中，TouchableOpacity组件有一些被注释掉的属性。这些注释可能会引起混淆，建议要么删除这些注释，要么添加解释说明为什么保留这些注释。请检查以下被注释的属性，决定是否需要它们，如果不需要，可以删除：
// visible={innerVisible} // closeOnOverlayClick={closeOnOverlayClick} // lockScroll={lockScroll} // duration={duration}如果这些属性可能在将来使用，请添加一个注释解释保留它们的原因。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/packages/popup/demo.taro.tsx (3 hunks)
- src/packages/popup/demos/taro/demo1.tsx (2 hunks)
- src/packages/popup/demos/taro/demo3.tsx (2 hunks)
- src/packages/popup/popup.rn.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/popup/demo.taro.tsx
🧰 Additional context used
Biome
src/packages/popup/popup.rn.tsx
[error] 43-43: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 44-44: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 150-153: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 161-163: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 168-169: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 174-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 178-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 185-188: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 191-194: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments not posted (7)
src/packages/popup/demos/taro/demo1.tsx (1)
3-3: 导入更改看起来不错！从
@tarojs/components导入ScrollView和Text组件是一个好的改进。这表明我们正在使用更具体和适合的 Taro 组件，这可能会提高性能和用户体验。src/packages/popup/demos/taro/demo3.tsx (1)
4-4: 导入语句添加正确新增的
harmonyAndRn函数导入看起来是为了后续的条件渲染逻辑。这是一个好的做法，可以根据平台来调整组件的渲染。src/packages/popup/popup.rn.tsx (5)
1-23: 导入语句看起来适合跨平台组件导入语句包含了 React Native 和 Taro 的组件，这表明该组件可能设计用于跨平台使用。这种方法可以提高代码的可重用性，但也可能增加维护的复杂性。请确保在不同平台上充分测试组件的行为。
47-65: 默认属性设置合理
defaultProps对象为各种属性设置了默认值，并通过扩展ComponentDefaults和defaultOverlayProps来复用代码。这种方法有助于保持代码的一致性和可维护性。
331-334: useEffect 钩子逻辑正确这个
useEffect钩子正确地处理了基于visible属性的弹出窗口的打开和关闭逻辑。这种方法确保了组件的状态与其 props 保持同步。
336-338: 过渡效果设置逻辑正确这个
useEffect钩子正确地根据position和transition属性设置过渡效果名称。这种方法可以确保应用正确的过渡效果，提高用户体验。
340-343: 组件返回语句使用了 Portal 渲染组件的返回语句使用
renderToContainer函数来渲染内容，这是处理模态框的好方法。使用 Portal 可以确保模态框正确地渲染在 DOM 树的顶层，避免 z-index 和样式继承的问题。
| <ScrollView style={{ height: '200px', overflowY: 'scroll' }}> | ||
| {Array.from({ length: 1 }) |
There was a problem hiding this comment.
💡 Codebase verification
建议保持数组长度一致以确保演示效果
仅 demo1.tsx 中的数组长度被设为 1，可能会影响该演示滚动功能的展示效果。建议将其恢复到与其他 demo 文件（如 demo2.tsx 和 demo8.tsx）一致的长度（200），以确保所有演示的一致性和有效性。
- 文件: src/packages/popup/demos/taro/demo1.tsx
- 位置: Array.from({ length: 1 })
🔗 Analysis chain
使用 ScrollView 是个好主意，但是数组长度的变化需要考虑
将 View 替换为 ScrollView 并添加滚动样式是一个很好的改进，这样可以在弹出层中实现可滚动的内容，提高了用户体验。
然而，将数组长度从 20 减少到 1 可能会影响演示的效果。考虑是否应该保持一定数量的项目来更好地展示滚动功能。
请运行以下脚本来验证其他 demo 文件中的数组长度：
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查其他 demo 文件中的数组长度
# 测试：搜索其他 demo 文件中的数组长度。预期：找到使用较长数组的其他 demo。
rg --type typescript 'Array\.from\({ length: \d+ }\)' src/packages/popup/demos
Length of output: 208
Script:
#!/bin/bash
# 描述：检查其他 demo 文件中的数组长度
# 修正后的正则表达式，用于匹配 Array.from({ length: 数字 })
rg --type typescript 'Array\.from\(\{\s*length:\s*\d+\s*\}\)' src/packages/popup/demos
Length of output: 123
Script:
#!/bin/bash
# 描述：检查其他 demo 文件中的数组长度
# 使用文件扩展名匹配 TypeScript 文件，避免依赖 --type
rg 'Array\.from\(\{\s*length:\s*\d+\s*\}\)' src/packages/popup/demos --glob '*.ts' --glob '*.tsx'
Length of output: 334
| const renderTitle = () => { | ||
| if (left || title || description) { | ||
| return ( | ||
| <View className={`${classPrefix}-title`}> | ||
| {position === 'bottom' && ( | ||
| <> | ||
| {left && ( | ||
| <View className={`${classPrefix}-title-left`}>{left}</View> | ||
| )} | ||
| {(title || description) && ( | ||
| <View className={`${classPrefix}-title-title`}> | ||
| <View>{title}</View> | ||
| {description && ( | ||
| <View className={`${classPrefix}-title-description`}> | ||
| {description} | ||
| </View> | ||
| )} | ||
| </View> | ||
| )} | ||
| </> | ||
| )} | ||
| {closeable && ( | ||
| <View className={closeClasses} onClick={onHandleClickCloseIcon}> | ||
| {React.isValidElement(closeIcon) ? ( | ||
| closeIcon | ||
| ) : harmonyAndRn() ? ( | ||
| <Text>T</Text> | ||
| ) : ( | ||
| <Close /> | ||
| )} | ||
| </View> | ||
| )} | ||
| </View> | ||
| ) | ||
| } | ||
| if (closeable) { | ||
| return ( | ||
| <> | ||
| {closeable && ( | ||
| <View className={closeClasses} onClick={onHandleClickCloseIcon}> | ||
| {React.isValidElement(closeIcon) ? ( | ||
| closeIcon | ||
| ) : harmonyAndRn() ? ( | ||
| <Text>I</Text> | ||
| ) : ( | ||
| <Close /> | ||
| )} | ||
| </View> | ||
| )} | ||
| </> | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议提取关闭图标渲染逻辑
renderTitle 函数中有重复的关闭图标渲染代码。为了提高代码的可维护性和减少重复，建议将关闭图标的渲染逻辑提取到一个单独的函数中。
建议创建一个新的函数来处理关闭图标的渲染，例如：
const renderCloseIcon = () => (
<View className={closeClasses} onClick={onHandleClickCloseIcon}>
{React.isValidElement(closeIcon) ? (
closeIcon
) : harmonyAndRn() ? (
<Text>{position === 'bottom' ? 'T' : 'I'}</Text>
) : (
<Close />
)}
</View>
);然后在 renderTitle 函数中使用这个新函数：
{closeable && renderCloseIcon()}这样可以减少代码重复，并使 renderTitle 函数更加简洁。
| const renderPop = () => { | ||
| return ( | ||
| <View | ||
| ref={refObject} | ||
| style={{ | ||
| position: 'absolute', | ||
| overflow: 'hidden', | ||
| ...popStyles, | ||
| }} | ||
| className={popClassName} | ||
| onClick={onHandleClick} | ||
| > | ||
| {renderTitle()} | ||
| {showChildren ? children : ''} | ||
| </View> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议将样式提取到 StyleSheet
在 renderPop 函数中，存在内联样式。为了提高性能和代码的可维护性，建议使用 StyleSheet.create 将样式提取出来。
可以按以下方式修改：
import { StyleSheet } from 'react-native';
const styles = StyleSheet.create({
popContainer: {
position: 'absolute',
overflow: 'hidden',
},
});
const renderPop = () => {
return (
<View
ref={refObject}
style={[styles.popContainer, popStyles]}
className={popClassName}
onClick={onHandleClick}
>
{renderTitle()}
{showChildren ? children : ''}
</View>
);
};这样可以提高样式的可重用性和性能。
src/packages/popup/popup.rn.tsx
Outdated
| className, | ||
| destroyOnClose, | ||
| portal, | ||
| onOpen, | ||
| onClose, | ||
| onOverlayClick, | ||
| onCloseIconClick, | ||
| afterShow, | ||
| afterClose, | ||
| onClick, | ||
| } = { ...defaultProps, ...props } | ||
| let innerIndex = zIndex || _zIndex | ||
| const [index, setIndex] = useState(innerIndex) | ||
| const [innerVisible, setInnerVisible] = useState(visible) | ||
| const [showChildren, setShowChildren] = useState(true) | ||
| const [transitionName, setTransitionName] = useState('') | ||
| const refObject = useLockScrollTaro(innerVisible && lockScroll) | ||
| const classPrefix = 'nut-popup' | ||
| const baseStyle = { | ||
| zIndex: index, | ||
| } | ||
|
|
||
| const overlayStyles = { | ||
| ...overlayStyle, | ||
| '--nutui-overlay-zIndex': index, | ||
| width: '100%', | ||
| height: '100%', | ||
| } | ||
|
|
||
| const popStyles = { | ||
| ...style, | ||
| ...baseStyle, | ||
| } | ||
|
|
||
| const popClassName = classNames( | ||
| { | ||
| [`${classPrefix}`]: true, | ||
| [`${classPrefix}-round`]: round || position === 'bottom', | ||
| [`${classPrefix}-${position}`]: true, | ||
| }, | ||
| className | ||
| ) | ||
|
|
||
| const closeClasses = classNames({ | ||
| [`${classPrefix}-title-right`]: true, | ||
| [`${classPrefix}-title-right-${closeIconPosition}`]: true, | ||
| }) | ||
|
|
||
| const open = () => { | ||
| if (!innerVisible) { | ||
| setInnerVisible(true) | ||
| setIndex(++innerIndex) | ||
| } | ||
| if (destroyOnClose) { | ||
| setShowChildren(true) | ||
| } | ||
| onOpen && onOpen() | ||
| } | ||
|
|
||
| const close = () => { | ||
| if (innerVisible) { | ||
| setInnerVisible(false) | ||
| if (destroyOnClose) { | ||
| setTimeout(() => { | ||
| setShowChildren(false) | ||
| }, Number(duration)) | ||
| } | ||
| onClose && onClose() | ||
| } | ||
| } | ||
|
|
||
| const onHandleClickOverlay = (e: ITouchEvent) => { | ||
| e.stopPropagation() | ||
| if (closeOnOverlayClick) { | ||
| const closed = onOverlayClick && onOverlayClick(e) | ||
| closed && close() | ||
| } | ||
| } | ||
|
|
||
| const onHandleClick = (e: ITouchEvent) => { | ||
| onClick && onClick(e) | ||
| } | ||
|
|
||
| const onHandleClickCloseIcon = (e: ITouchEvent) => { | ||
| const closed = onCloseIconClick && onCloseIconClick(e) | ||
| closed && close() | ||
| } | ||
|
|
||
| const onHandleOpened: EnterHandler<HTMLElement | undefined> | undefined = ( | ||
| e: HTMLElement | ||
| ) => { | ||
| afterShow && afterShow() | ||
| } | ||
|
|
||
| const onHandleClosed: ExitHandler<HTMLElement | undefined> | undefined = ( | ||
| e: HTMLElement | ||
| ) => { | ||
| afterClose && afterClose() | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议使用可选链操作符简化函数调用
在组件中的多处地方，可以使用可选链操作符 ?. 来简化代码，提高可读性。例如，将 onOpen && onOpen() 替换为 onOpen?.()。
建议如下修改：
- onOpen && onOpen()
+ onOpen?.()
- onClose && onClose()
+ onClose?.()
- const closed = onOverlayClick && onOverlayClick(e)
+ const closed = onOverlayClick?.(e)
- onClick && onClick(e)
+ onClick?.(e)
- const closed = onCloseIconClick && onCloseIconClick(e)
+ const closed = onCloseIconClick?.(e)
- afterShow && afterShow()
+ afterShow?.()
- afterClose && afterClose()
+ afterClose?.()📝 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 Popup: FunctionComponent< | |
| Partial<PopupProps> & | |
| Omit<React.HTMLAttributes<HTMLDivElement>, 'onClick' | 'title'> | |
| > = (props) => { | |
| const { | |
| children, | |
| visible, | |
| overlay, | |
| closeOnOverlayClick, | |
| overlayStyle, | |
| overlayClassName, | |
| zIndex, | |
| lockScroll, | |
| duration, | |
| closeable, | |
| closeIconPosition, | |
| closeIcon, | |
| left, | |
| title, | |
| description, | |
| style, | |
| transition, | |
| round, | |
| position, | |
| className, | |
| destroyOnClose, | |
| portal, | |
| onOpen, | |
| onClose, | |
| onOverlayClick, | |
| onCloseIconClick, | |
| afterShow, | |
| afterClose, | |
| onClick, | |
| } = { ...defaultProps, ...props } | |
| let innerIndex = zIndex || _zIndex | |
| const [index, setIndex] = useState(innerIndex) | |
| const [innerVisible, setInnerVisible] = useState(visible) | |
| const [showChildren, setShowChildren] = useState(true) | |
| const [transitionName, setTransitionName] = useState('') | |
| const refObject = useLockScrollTaro(innerVisible && lockScroll) | |
| const classPrefix = 'nut-popup' | |
| const baseStyle = { | |
| zIndex: index, | |
| } | |
| const overlayStyles = { | |
| ...overlayStyle, | |
| '--nutui-overlay-zIndex': index, | |
| width: '100%', | |
| height: '100%', | |
| } | |
| const popStyles = { | |
| ...style, | |
| ...baseStyle, | |
| } | |
| const popClassName = classNames( | |
| { | |
| [`${classPrefix}`]: true, | |
| [`${classPrefix}-round`]: round || position === 'bottom', | |
| [`${classPrefix}-${position}`]: true, | |
| }, | |
| className | |
| ) | |
| const closeClasses = classNames({ | |
| [`${classPrefix}-title-right`]: true, | |
| [`${classPrefix}-title-right-${closeIconPosition}`]: true, | |
| }) | |
| const open = () => { | |
| if (!innerVisible) { | |
| setInnerVisible(true) | |
| setIndex(++innerIndex) | |
| } | |
| if (destroyOnClose) { | |
| setShowChildren(true) | |
| } | |
| onOpen && onOpen() | |
| } | |
| const close = () => { | |
| if (innerVisible) { | |
| setInnerVisible(false) | |
| if (destroyOnClose) { | |
| setTimeout(() => { | |
| setShowChildren(false) | |
| }, Number(duration)) | |
| } | |
| onClose && onClose() | |
| } | |
| } | |
| const onHandleClickOverlay = (e: ITouchEvent) => { | |
| e.stopPropagation() | |
| if (closeOnOverlayClick) { | |
| const closed = onOverlayClick && onOverlayClick(e) | |
| closed && close() | |
| } | |
| } | |
| const onHandleClick = (e: ITouchEvent) => { | |
| onClick && onClick(e) | |
| } | |
| const onHandleClickCloseIcon = (e: ITouchEvent) => { | |
| const closed = onCloseIconClick && onCloseIconClick(e) | |
| closed && close() | |
| } | |
| const onHandleOpened: EnterHandler<HTMLElement | undefined> | undefined = ( | |
| e: HTMLElement | |
| ) => { | |
| afterShow && afterShow() | |
| } | |
| const onHandleClosed: ExitHandler<HTMLElement | undefined> | undefined = ( | |
| e: HTMLElement | |
| ) => { | |
| afterClose && afterClose() | |
| } | |
| export const Popup: FunctionComponent< | |
| Partial<PopupProps> & | |
| Omit<React.HTMLAttributes<HTMLDivElement>, 'onClick' | 'title'> | |
| > = (props) => { | |
| const { | |
| children, | |
| visible, | |
| overlay, | |
| closeOnOverlayClick, | |
| overlayStyle, | |
| overlayClassName, | |
| zIndex, | |
| lockScroll, | |
| duration, | |
| closeable, | |
| closeIconPosition, | |
| closeIcon, | |
| left, | |
| title, | |
| description, | |
| style, | |
| transition, | |
| round, | |
| position, | |
| className, | |
| destroyOnClose, | |
| portal, | |
| onOpen, | |
| onClose, | |
| onOverlayClick, | |
| onCloseIconClick, | |
| afterShow, | |
| afterClose, | |
| onClick, | |
| } = { ...defaultProps, ...props } | |
| let innerIndex = zIndex || _zIndex | |
| const [index, setIndex] = useState(innerIndex) | |
| const [innerVisible, setInnerVisible] = useState(visible) | |
| const [showChildren, setShowChildren] = useState(true) | |
| const [transitionName, setTransitionName] = useState('') | |
| const refObject = useLockScrollTaro(innerVisible && lockScroll) | |
| const classPrefix = 'nut-popup' | |
| const baseStyle = { | |
| zIndex: index, | |
| } | |
| const overlayStyles = { | |
| ...overlayStyle, | |
| '--nutui-overlay-zIndex': index, | |
| width: '100%', | |
| height: '100%', | |
| } | |
| const popStyles = { | |
| ...style, | |
| ...baseStyle, | |
| } | |
| const popClassName = classNames( | |
| { | |
| [`${classPrefix}`]: true, | |
| [`${classPrefix}-round`]: round || position === 'bottom', | |
| [`${classPrefix}-${position}`]: true, | |
| }, | |
| className | |
| ) | |
| const closeClasses = classNames({ | |
| [`${classPrefix}-title-right`]: true, | |
| [`${classPrefix}-title-right-${closeIconPosition}`]: true, | |
| }) | |
| const open = () => { | |
| if (!innerVisible) { | |
| setInnerVisible(true) | |
| setIndex(++innerIndex) | |
| } | |
| if (destroyOnClose) { | |
| setShowChildren(true) | |
| } | |
| onOpen?.() | |
| } | |
| const close = () => { | |
| if (innerVisible) { | |
| setInnerVisible(false) | |
| if (destroyOnClose) { | |
| setTimeout(() => { | |
| setShowChildren(false) | |
| }, Number(duration)) | |
| } | |
| onClose?.() | |
| } | |
| } | |
| const onHandleClickOverlay = (e: ITouchEvent) => { | |
| e.stopPropagation() | |
| if (closeOnOverlayClick) { | |
| const closed = onOverlayClick?.(e) | |
| closed && close() | |
| } | |
| } | |
| const onHandleClick = (e: ITouchEvent) => { | |
| onClick?.(e) | |
| } | |
| const onHandleClickCloseIcon = (e: ITouchEvent) => { | |
| const closed = onCloseIconClick?.(e) | |
| closed && close() | |
| } | |
| const onHandleOpened: EnterHandler<HTMLElement | undefined> | undefined = ( | |
| e: HTMLElement | |
| ) => { | |
| afterShow?.() | |
| } | |
| const onHandleClosed: ExitHandler<HTMLElement | undefined> | undefined = ( | |
| e: HTMLElement | |
| ) => { | |
| afterClose?.() | |
| } |
🧰 Tools
Biome
[error] 150-153: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 161-163: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 168-169: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 174-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 178-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 185-188: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
src/packages/popup/demos/taro/demo7.tsx (3)
27-33: 使用 Text 组件替换 span 是正确的将 span 替换为 Text 组件是正确的做法，这符合 Taro 的组件使用规范。onClick 事件处理器也被正确保留，保证了功能的完整性。
建议：考虑为 Text 组件添加适当的样式，以确保在不同平台上的一致外观。
43-49: Text 组件的使用是正确的，建议保持一致性将 span 替换为 Text 组件的做法是正确的，与之前的修改保持一致，并符合 Taro 的规范。onClick 事件处理器也被正确保留。
建议：为了保持代码的一致性，考虑将 "close" 文本提取为一个常量或使用国际化文本。这样可以提高代码的可维护性和可扩展性。
Line range hint
1-89: 总体评估：改动合理，有进一步优化空间这些修改正确地将 span 元素替换为 Taro 的 Text 组件，提高了组件与 Taro 项目的兼容性。改动保持了一致性，没有改变组件的功能。
建议未来的改进：
- 考虑将重复的 Popup 逻辑提取为一个可复用的组件。
- 使用常量或国际化文本来管理文本内容。
- 添加适当的类型注解以增强代码的可读性和可维护性。
- 考虑使用自定义 Hook 来管理 Popup 和 Dialog 的状态，以简化主组件的逻辑。
src/packages/popup/popup.rn.tsx (2)
70-313: 组件结构良好，但可以优化回调函数的调用Popup 组件的结构遵循了 React 的最佳实践，使用 useState 和 useEffect 钩子来管理组件状态和副作用。然而，在调用回调函数时，可以使用可选链操作符来简化代码。
建议进行以下修改：
- onOpen && onOpen() + onOpen?.() - onClose && onClose() + onClose?.() - const closed = onOverlayClick && onOverlayClick(e) + const closed = onOverlayClick?.(e) - onClick && onClick(e) + onClick?.(e) - const closed = onCloseIconClick && onCloseIconClick(e) + const closed = onCloseIconClick?.(e) - afterShow && afterShow() + afterShow?.() - afterClose && afterClose() + afterClose?.()这些修改可以使代码更加简洁和易读，同时保持相同的功能。
🧰 Tools
🪛 Biome
[error] 151-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 162-164: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 169-170: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 175-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 179-180: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 186-189: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 192-195: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
209-304: 渲染方法结构良好，但可以进一步优化
renderTitle、renderPop和renderNode方法很好地处理了弹出窗口的不同方面，包括标题、内容和遮罩层。使用 React Native 的 Modal 组件来创建弹出窗口是恰当的。然而，
renderTitle方法中存在一些重复的关闭图标渲染逻辑。建议将关闭图标的渲染逻辑提取到一个单独的函数中，以减少代码重复并提高可维护性。例如：const renderCloseIcon = () => ( <View className={closeClasses} onClick={onHandleClickCloseIcon}> {React.isValidElement(closeIcon) ? ( closeIcon ) : harmonyAndRn() ? ( <Text>{position === 'bottom' ? 'T' : 'I'}</Text> ) : ( <Close /> )} </View> );然后在
renderTitle函数中使用这个新函数：{closeable && renderCloseIcon()}这样可以减少代码重复，并使
renderTitle函数更加简洁。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/packages/popup/demos/taro/demo7.tsx (3 hunks)
- src/packages/popup/popup.rn.tsx (1 hunks)
🧰 Additional context used
🪛 Biome
src/packages/popup/popup.rn.tsx
[error] 43-43: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 44-44: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 151-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 162-164: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 169-170: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 175-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 179-180: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 186-189: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 192-195: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
src/packages/popup/demos/taro/demo7.tsx (1)
3-3: 导入 Text 组件是正确的做法从 @tarojs/components 导入 Text 组件是正确的。这个改动与后续在组件中使用 Text 替换 span 的修改保持一致。
src/packages/popup/popup.rn.tsx (3)
1-23: 导入语句看起来合适且全面导入语句涵盖了 React、React Native 和 Taro 的必要组件和类型，以及自定义的工具函数。这表明该组件旨在跨平台使用，包括 React Native 和可能的鸿蒙 OS 支持。
47-65: 默认属性设置合理且促进代码复用
defaultProps对象为 Popup 组件的各种属性提供了合理的默认值。通过使用展开运算符引入ComponentDefaults和defaultOverlayProps，代码实现了良好的复用性。这种方法使得默认值的管理更加集中和一致。
1-318: 总体评价：结构良好的跨平台 Popup 组件，有少量优化空间这个 Popup 组件整体实现得很好，遵循了 React 的最佳实践，并且设计用于跨平台使用（React Native 和 Taro）。组件使用 React 钩子有效地管理状态和副作用，提供了丰富的自定义选项。
主要的改进建议包括：
- 在
PopupProps接口中，将回调函数的返回类型从boolean | void更改为boolean | undefined。- 使用可选链操作符来简化回调函数的调用。
- 提取重复的关闭图标渲染逻辑到单独的函数中。
- 使用
StyleSheet.create来定义样式，以提高性能和可维护性。实施这些建议将进一步提高代码的质量、可读性和性能。总的来说，这是一个设计良好的组件，能够在不同平台上提供一致的 Popup 功能。
🧰 Tools
🪛 Biome
[error] 43-43: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 44-44: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 151-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 162-164: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 169-170: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 175-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 179-180: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 186-189: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 192-195: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| const baseStyle = { | ||
| zIndex: index, | ||
| } | ||
|
|
||
| const overlayStyles = { | ||
| ...overlayStyle, | ||
| '--nutui-overlay-zIndex': index, | ||
| width: '100%', | ||
| height: '100%', | ||
| backgroundColor: 'rgba(0, 0, 0, 0.7)', | ||
| } | ||
|
|
||
| const popStyles = { | ||
| ...style, | ||
| ...baseStyle, | ||
| } | ||
|
|
||
| const popClassName = classNames( | ||
| { | ||
| [`${classPrefix}`]: true, | ||
| [`${classPrefix}-round`]: round || position === 'bottom', | ||
| [`${classPrefix}-${position}`]: true, | ||
| }, | ||
| className | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议使用 StyleSheet.create 来优化样式定义
当前的样式定义方式使用了内联样式，这虽然提供了灵活性，但可能会使渲染方法变得难以阅读。对于 React Native 应用，使用 StyleSheet.create 可以提供更好的性能和可维护性。
建议将内联样式重构为使用 StyleSheet.create。例如：
import { StyleSheet } from 'react-native';
const styles = StyleSheet.create({
baseStyle: {
zIndex: index,
},
overlayStyle: {
width: '100%',
height: '100%',
backgroundColor: 'rgba(0, 0, 0, 0.7)',
},
popContainer: {
position: 'absolute',
overflow: 'hidden',
},
});
// 在 renderPop 中使用
<View
ref={refObject}
style={[styles.popContainer, popStyles]}
className={popClassName}
onClick={onHandleClick}
>
{/* ... */}
</View>这种方法可以提高样式的可重用性和性能，同时使组件代码更加清晰。
Also applies to: 263-279
| export interface PopupProps extends OverlayProps { | ||
| position: string | ||
| transition: string | ||
| overlayStyle: React.CSSProperties | ||
| overlayClassName: string | ||
| closeable: boolean | ||
| closeIconPosition: string | ||
| closeIcon: ReactNode | ||
| left: ReactNode | ||
| title: ReactNode | ||
| description: ReactNode | ||
| destroyOnClose: boolean | ||
| portal: Teleport | ||
| overlay: boolean | ||
| round: boolean | ||
| onOpen: () => void | ||
| onClose: () => void | ||
| onOverlayClick: (e: ITouchEvent) => boolean | void | ||
| onCloseIconClick: (e: ITouchEvent) => boolean | void | ||
| } |
There was a problem hiding this comment.
建议优化回调函数的返回类型
PopupProps 接口提供了全面的选项来自定义 Popup 组件。然而，在 onOverlayClick 和 onCloseIconClick 的返回类型中使用 void 可能会引起混淆。
建议将这些函数的返回类型从 boolean | void 更改为 boolean | undefined，以提高代码的可读性和类型安全性。
- onOverlayClick: (e: ITouchEvent) => boolean | void
+ onOverlayClick: (e: ITouchEvent) => boolean | undefined
- onCloseIconClick: (e: ITouchEvent) => boolean | void
+ onCloseIconClick: (e: ITouchEvent) => boolean | undefined这样的修改可以使类型更加明确，并与 TypeScript 的最佳实践保持一致。
📝 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 interface PopupProps extends OverlayProps { | |
| position: string | |
| transition: string | |
| overlayStyle: React.CSSProperties | |
| overlayClassName: string | |
| closeable: boolean | |
| closeIconPosition: string | |
| closeIcon: ReactNode | |
| left: ReactNode | |
| title: ReactNode | |
| description: ReactNode | |
| destroyOnClose: boolean | |
| portal: Teleport | |
| overlay: boolean | |
| round: boolean | |
| onOpen: () => void | |
| onClose: () => void | |
| onOverlayClick: (e: ITouchEvent) => boolean | void | |
| onCloseIconClick: (e: ITouchEvent) => boolean | void | |
| } | |
| export interface PopupProps extends OverlayProps { | |
| position: string | |
| transition: string | |
| overlayStyle: React.CSSProperties | |
| overlayClassName: string | |
| closeable: boolean | |
| closeIconPosition: string | |
| closeIcon: ReactNode | |
| left: ReactNode | |
| title: ReactNode | |
| description: ReactNode | |
| destroyOnClose: boolean | |
| portal: Teleport | |
| overlay: boolean | |
| round: boolean | |
| onOpen: () => void | |
| onClose: () => void | |
| onOverlayClick: (e: ITouchEvent) => boolean | undefined | |
| onCloseIconClick: (e: ITouchEvent) => boolean | undefined | |
| } |
🧰 Tools
🪛 Biome
[error] 43-43: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 44-44: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新特性
Popup组件，提供可定制的模态界面。index.rn.ts和popup.rn.tsx，以支持 React Native 环境。样式
.nut-overlay类和多个动画关键帧。依赖更新
react-native作为新依赖。