Skip to content

Comments

fix(inputnumber): 适配RN和鸿蒙,加减按钮icon在RN和鸿蒙暂不展示#2353

Merged
xiaoyatong merged 15 commits intojdf2e:dev-harmonyfrom
irisSong:dev-harmony-inputnumber
Jun 18, 2024
Merged

fix(inputnumber): 适配RN和鸿蒙,加减按钮icon在RN和鸿蒙暂不展示#2353
xiaoyatong merged 15 commits intojdf2e:dev-harmonyfrom
irisSong:dev-harmony-inputnumber

Conversation

@irisSong
Copy link
Collaborator

@irisSong irisSong commented Jun 14, 2024

🤔 这个变动的性质是?

  • 新特性提交
  • 日常 bug 修复
  • 站点、文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • TypeScript 定义更新
  • 包体积优化
  • 性能优化
  • 功能增强
  • 国际化改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他改动(是关于什么的改动?)

🔗 相关 Issue

💡 需求背景和解决方案

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • fork仓库代码是否为最新避免文件冲突
  • Files changed 没有 package.json lock 等无关文件

Summary by CodeRabbit

  • 新功能

    • 更新了“InputNumber”组件的版本至3.0.0,提升组件性能和功能。
  • 样式

    • 将输入数字元素的默认宽度从40px增加到60px,以优化用户交互体验。
  • 重构

    • 多个演示文件中,将InputNumber组件包裹在Cell组件内,改善布局结构。

@coderabbitai
Copy link

coderabbitai bot commented Jun 14, 2024

Walkthrough

这次更新主要涉及多个文件中的InputNumber组件,其中最显著的变化是将版本从 2.0.0 升级到 3.0.0。对多个示例文件进行了调整,将 InputNumber 组件嵌套在 Cell 组件中。此外,还对样式变量和平台适配脚本进行了修改,以增强跨平台的兼容性和组件的布局。

Changes

文件 变更总结
src/config.json InputNumber 组件版本从 2.0.0 更新为 3.0.0
src/packages/inputnumber/demos/taro/demo1.tsxdemo9.tsx InputNumber 组件嵌套在 Cell 组件中,并更新相关的导入语句。
src/packages/inputnumber/inputnumber.taro.tsx 更新平台相关函数的导入,调整 InputNumberPropstype 属性,修改类名以适应不同平台,通过平台兼容性标志进行条件渲染。
src/styles/variables.scss 将输入数字元素的默认宽度从40px增加到60px。
packages/nutui-taro-demo-rn/scripts/taro/adapted.js inputnumber 添加到导出的模块列表中。

Poem

在代码的世界里我们升级,
从2到3,你看那线条悄然延续。
Cell与InputNumber的一场舞蹈,
各大平台兼容,毫不枯燥。
新的宽度,布局更为完美,
更新的版本,未来更加明媚。 🌟


Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 51eb6f9 and a0335b1.

Files selected for processing (2)
  • src/packages/inputnumber/demos/taro/demo6.tsx (1 hunks)
  • src/packages/inputnumber/inputnumber.taro.tsx (7 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/packages/inputnumber/demos/taro/demo6.tsx
Additional context used
Biome
src/packages/inputnumber/inputnumber.taro.tsx

[error] 219-219: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 222-223: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

Additional comments not posted (2)
src/packages/inputnumber/inputnumber.taro.tsx (2)

230-254: 在 React Native 和 HarmonyOS 平台上,减号和加号按钮使用了文本代替图标。请确认这种设计符合 UI 规范。

Also applies to: 285-308


90-93: 请确保 harmonyAndRnrn 这些平台检测函数的实现逻辑是正确的,以便正确处理不同平台的特定行为。

Verification successful

harmonyAndRnrn 函数的定义已经在 src/utils/platform-taro.ts 文件中找到了。可以确认这些函数的实现逻辑存在,并且导入语句也正确无误。

  • harmonyAndRn 函数定义在 src/utils/platform-taro.ts 文件中。
  • rn 函数定义在 src/utils/platform-taro.ts 文件中。

这些函数在多个文件中被正确导入,包括 src/packages/inputnumber/inputnumber.taro.tsx

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# 描述:验证平台检测函数 `harmonyAndRn` 和 `rn` 的实现是否正确。

# 测试:搜索这些函数的实现。预期:找到这些函数的定义和逻辑实现。
ast-grep --lang javascript --pattern $'function harmonyAndRn() {
  $$$
}'
ast-grep --lang javascript --pattern $'function rn() {
  $$$
}'

Length of output: 139



Script:

#!/bin/bash
# 描述:验证平台检测函数 `harmonyAndRn` 和 `rn` 的实现是否正确。

# 测试:搜索这些函数的定义和逻辑实现,考虑更多的定义方式。
# 搜索函数定义,包括普通函数和箭头函数。
rg "function harmonyAndRn" -t js -t ts
rg "const harmonyAndRn" -t js -t ts
rg "function rn" -t js -t ts
rg "const rn" -t js -t ts

# 搜索可能的导入语句,以确定这些函数是否从其他模块导入。
rg "import.*harmonyAndRn" -t js -t ts
rg "import.*rn" -t js -t ts

Length of output: 3398


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Outside diff range and nitpick comments (3)
src/packages/inputnumber/demos/taro/demo9.tsx (1)

61-68: 注意,Toast 组件已被注释掉。如果这是临时的改动,请添加 TODO 或 FIXME 标记以便未来跟踪。

src/packages/inputnumber/demo.taro.tsx (1)

67-108: 代码整体看起来很整洁,但建议在每个示例之间添加一些注释,以提高代码的可读性和可维护性。

src/packages/inputnumber/inputnumber.taro.tsx (1)

224-263: 在鸿蒙和React Native平台上隐藏加减按钮是符合PR目标的。但是,建议添加相关注释说明为什么在这些平台上需要隐藏这些按钮,以便其他开发者理解背景。

@@ -1 +1 @@
exports = module.exports = ["cell","cellgroup","overlay","button","switch","countdown","empty","price","tag","layout","row","col","space","grid","griditem","divider","image","resultpage","notify","pagination","swiper","textarea","indicator","hoverbutton","safearea","hoverbuttonitem"]; No newline at end of file
exports = module.exports = ["cell","cellgroup","overlay","button","switch","countdown","empty","price","tag","layout","row","col","space","grid","griditem","divider","image","resultpage","notify","pagination","swiper","textarea","indicator","hoverbutton","safearea","hoverbuttonitem", 'inputnumber'];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

避免直接对全局变量进行赋值,这可能会覆盖重要的功能或导致意外行为。

- exports = module.exports = ["cell","cellgroup","overlay","button","switch","countdown","empty","price","tag","layout","row","col","space","grid","griditem","divider","image","resultpage","notify","pagination","swiper","textarea","indicator","hoverbutton","safearea","hoverbuttonitem", 'inputnumber'];
+ module.exports = ["cell","cellgroup","overlay","button","switch","countdown","empty","price","tag","layout","row","col","space","grid","griditem","divider","image","resultpage","notify","pagination","swiper","textarea","indicator","hoverbutton","safearea","hoverbuttonitem", 'inputnumber'];
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
exports = module.exports = ["cell","cellgroup","overlay","button","switch","countdown","empty","price","tag","layout","row","col","space","grid","griditem","divider","image","resultpage","notify","pagination","swiper","textarea","indicator","hoverbutton","safearea","hoverbuttonitem", 'inputnumber'];
module.exports = ["cell","cellgroup","overlay","button","switch","countdown","empty","price","tag","layout","row","col","space","grid","griditem","divider","image","resultpage","notify","pagination","swiper","textarea","indicator","hoverbutton","safearea","hoverbuttonitem", 'inputnumber'];
Tools
Biome

[error] 1-1: A global variable should not be reassigned. (lint/suspicious/noGlobalAssign)

Assigning to a global variable can override essential functionality.

Comment on lines +217 to +219
<div className={`${classPrefix}-minus`} onClick={handleReduce}>
<Minus
className={classNames('nut-inputnumber-icon icon-minus', {
[`${classPrefix}-icon-disabled`]: shadowValue === min || disabled,
})}
className={classNames(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对于鼠标事件触发的操作,应该添加对应的键盘事件处理,以支持键盘导航,提高无障碍访问性。

+ onKeyUp={handleReduce}
+ onKeyUp={handlePlus}

Also applies to: 242-244

Tools
Biome

[error] 217-219: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.

Comment on lines 13 to 17
background-color: gray;
/* #endif */
/* #ifndef rn harmony */
background-color: transparent;
/* #endif */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 HarmonyOS 和 React Native 平台上,为 .nut-inputnumber-minus 类设置灰色背景。请确认这种颜色选择符合设计规范。

Comment on lines 27 to 31
background-color: gray;
/* #endif */
/* #ifndef rn harmony */
background-color: transparent;
/* #endif */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

与上面的评论类似,这里也为 HarmonyOS 和 React Native 平台上的 .nut-inputnumber-add 类设置了灰色背景。同样,请验证这是否符合设计要求。

Comment on lines 13 to 18
/* #ifdef rn harmony */
background-color: gray;
/* #endif */
/* #ifndef rn harmony */
background-color: $inputnumber-button-background-color;
/* #endif */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 HarmonyOS 和 React Native 平台上,为按钮设置灰色背景。这是一个重要的视觉调整,请确保它与设计规范一致。

Copy link
Collaborator

@xiaoyatong xiaoyatong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+- 符号 可以使用 Text 标签,做兜底逻辑~~

min: number | string
max: number | string
type?: Extract<InputProps['type'], 'number' | 'digit'>
type?: Extract<InputProps['type'], 'text'>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个为什么要调整呢~

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已更改

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (1)
packages/nutui-taro-demo-rn/scripts/taro/adapted.js (1)

Line range hint 1-1: 请注意,这里直接对全局变量进行赋值可能会覆盖重要功能或导致不预期的行为。建议使用更安全的方法来定义模块导出。

- exports = module.exports = [...]
+ module.exports = [...]

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (1)
src/packages/inputnumber/inputnumber.taro.tsx (1)

Line range hint 218-218: 静态分析工具提示:建议使用可选链。这可以进一步简化代码。

-    onChange?.(parseFloat(valueStr || '0').toFixed(digits) as any, e)
+    onChange?.(parseFloat(valueStr || '0').toFixed(digits) as any, e)

Comment on lines +214 to 215
const handleBlur = (e: any) => {
setFocused(false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

静态分析工具提示:建议使用可选链。这可以提高代码的安全性和可读性。

-    onBlur && onBlur(e)
+    onBlur?.(e)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleBlur = (e: any) => {
setFocused(false)
const handleBlur = (e: any) => {
setFocused(false)
onBlur?.(e)
Tools
Biome

[error] 214-215: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

nutuiInputnumberInputMargin: '0 2px',
}

const isRnAndHarmony = harmonyAndRn()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

添加了 isRnAndHarmony 变量用于存储环境检测结果,但在此文件中似乎未使用此变量。建议移除或者利用此变量进行条件渲染以适应不同环境。

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

}
}
const handleFocus = (e: React.FocusEvent<HTMLInputElement>) => {
const handleFocus = (e: any) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议使用可选链来优化代码的可读性和安全性。

-    onFocus && onFocus(e)
+    onFocus?.(e)
-    onBlur && onBlur(e)
+    onBlur?.(e)

Also applies to: 219-219

Comment on lines +192 to +193
const handleInputChange = (e: any) => {
if (!focused && isHarmony) return
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

处理 HarmonyOS 平台时,输入变化的逻辑似乎被过早地终止了。这可能会导致在 HarmonyOS 上无法正确处理输入变化。

-    if (!focused && isHarmony) return
+    if (!focused && isHarmony) {
+        // 可以在这里添加特定的逻辑处理
+    }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleInputChange = (e: any) => {
if (!focused && isHarmony) return
const handleInputChange = (e: any) => {
if (!focused && isHarmony) {
// 可以在这里添加特定的逻辑处理
}

@xiaoyatong xiaoyatong merged commit c92319a into jdf2e:dev-harmony Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants