Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(radio): v14 #2786

Closed
wants to merge 5 commits into from
Closed

feat(radio): v14 #2786

wants to merge 5 commits into from

Conversation

oasis-cloud
Copy link
Collaborator

@oasis-cloud oasis-cloud commented Nov 22, 2024

🤔 这个变动的性质是?

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

🔗 相关 Issue

💡 需求背景和解决方案

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

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

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

Summary by CodeRabbit

  • 新功能
    • 引入了新的 Icon 组件,简化了图标渲染逻辑。
  • 变更
    • 移除了与“自定义尺寸”相关的演示和翻译。
    • 更新了 Radio 组件以使用新的 Icon 组件,改进了图标渲染。
  • 样式
    • 新增 .nut-radio-self 类,增强了单选按钮的视觉表现。

Copy link

coderabbitai bot commented Nov 22, 2024

Walkthrough

本次更改涉及多个文件,主要是移除与Demo7组件及其相关翻译的所有引用。具体来说,src/packages/radio/demo.taro.tsxsrc/packages/radio/demo.tsx中删除了对Demo7的导入和渲染逻辑,同时也移除了与“自定义尺寸”相关的翻译条目。此外,新添加了一个Icon组件,并在src/packages/radio/radio.tsxsrc/packages/radio/radio.taro.tsx中更新了图标的渲染逻辑,以使用新的Icon组件。

Changes

文件路径 更改摘要
src/packages/radio/demo.taro.tsx, src/packages/radio/demo.tsx 移除对Demo7的导入及其渲染逻辑,删除与“自定义尺寸”相关的翻译条目。
src/packages/radio/demos/h5/demo7.tsx, src/packages/radio/demos/taro/demo7.tsx 删除Demo7组件文件。
src/packages/radio/icon.tsx 新增Icon组件,定义IconProps接口,设置默认属性并导出该组件。
src/packages/radio/radio.scss 新增.nut-radio-self类,定义不同状态下的样式。
src/packages/radio/radio.tsx, src/packages/radio/radio.taro.tsx 更新图标渲染逻辑,使用新的Icon组件,修改标签渲染以增强灵活性,添加新的导入。

Possibly related PRs

Suggested reviewers

  • irisSong

兔子欢快地跳跃,
组件更新真是妙,
图标渲染新风貌,
自定义尺寸已去掉,
代码清晰如春晓。 🐰✨

Warning

Rate limit exceeded

@oasis-cloud has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 47 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between b93ff79 and 75fdcc7.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b93ff79 and 75fdcc7.

📒 Files selected for processing (1)
  • src/packages/radio/__test__/radio.spec.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/packages/radio/test/radio.spec.tsx

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:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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.

@github-actions github-actions bot added the 3.x label Nov 22, 2024
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: 3

🧹 Outside diff range and nitpick comments (9)
src/packages/radio/icon.tsx (2)

10-22: 建议使用 React.memo 优化组件性能

由于该组件是纯展示组件,建议使用 React.memo 包装以避免不必要的重渲染。

建议修改如下:

-const Icon = (props: Partial<IconProps>) => {
+const Icon = React.memo((props: Partial<IconProps>) => {
  const { tag, classPrefix, name } = mergeProps(
    {
      tag: 'div',
      classPrefix: 'nut-radio-self',
      name: 'normal',
    },
    props
  )
  return createElement<{ className: string }>(tag, {
    className: `${classPrefix} ${classPrefix}-${name}`,
  })
-}
+})

1-24: 建议增强组件的可扩展性和文档

为了提高组件的可复用性和可维护性,建议:

  1. 添加 JSDoc 文档说明组件的用途和属性
  2. 考虑添加 styleclassName 属性以支持自定义样式
  3. 添加单元测试确保组件的可靠性

需要我帮助实现以上建议吗?

src/packages/radio/radio.scss (4)

3-8: 基础样式结构合理,建议添加注释

基础类设置了合适的尺寸和定位属性,但建议添加简要注释说明各个状态的视觉效果。

建议在类定义前添加如下注释:

+// 自定义单选按钮基础样式
+// normal: 普通状态
+// checked: 选中状态
+// disabled: 禁用状态
+// checked-disabled: 选中禁用状态
.nut-radio-self {

15-27: 伪元素属性复用合理,建议优化性能

共用的伪元素样式通过选择器组合来实现代码复用,这是个好的实践。但是多重变换可能影响性能。

建议优化变换方式:

-      transform: translate(-50%, -50%);
+      transform: translate(-50%, -50%) translateZ(0);

这样可以触发 GPU 加速,提高渲染性能。


29-42: 选中状态渐变效果美观,但阴影值需要使用变量

渐变背景和阴影效果的视觉呈现很好,但建议将阴影值抽取为 SCSS 变量以保持一致性。

建议添加变量定义:

+$radio-shadow-color: rgba(255, 15, 35, 0.2);
+$radio-shadow-config: 0px 2px 4px 0px;

.nut-radio-self {
  &-checked {
-    box-shadow: 0px 2px 4px 0px rgba(255, 15, 35, 0.2);
+    box-shadow: $radio-shadow-config $radio-shadow-color;

44-61: 禁用状态样式统一,建议添加过渡效果

禁用和选中禁用状态的样式定义清晰,但缺少状态切换的过渡效果。

建议添加过渡效果:

.nut-radio-self {
+  transition: background-color 0.3s ease;
   
   &-disabled {
     background: $color-text-disabled;
+    transition: background-color 0.3s ease;
   }
   
   &-checked-disabled {
     background: $color-primary-disabled-special;
+    transition: background-color 0.3s ease;
   }
}
src/packages/radio/radio.tsx (2)

87-93: 建议优化条件渲染函数的命名和实现

renderIconByDisabledProperty 函数名称过于冗长,且职责不够明确。建议重构以提高可读性。

建议如下修改:

-function renderIconByDisabledProperty() {
+function renderCheckedIcon() {
   return disabled ? (
     <Icon name="checked-disabled" />
   ) : (
     <Icon name="checked" />
   )
 }

122-122: 优化条件渲染表达式

当前的条件渲染方式可以使用更简洁的写法。

建议如下修改:

-{children ? <div className={labelcls}>{children}</div> : null}
+{children && <div className={labelcls}>{children}</div>}
src/packages/radio/radio.taro.tsx (1)

88-94: 建议优化 renderIconByDisabledProperty 函数的命名和实现

当前实现存在以下问题:

  1. 函数名称过于冗长,不够简洁
  2. 函数嵌套在 renderIcon 内部可能影响可读性

建议如下重构:

- function renderIconByDisabledProperty() {
+ function renderCheckedIcon() {
    return disabled ? (
      <Icon name="checked-disabled" tag={View} />
    ) : (
      <Icon name="checked" tag={View} />
    )
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 0648882 and c08869a.

📒 Files selected for processing (8)
  • src/packages/radio/demo.taro.tsx (0 hunks)
  • src/packages/radio/demo.tsx (0 hunks)
  • src/packages/radio/demos/h5/demo7.tsx (0 hunks)
  • src/packages/radio/demos/taro/demo7.tsx (0 hunks)
  • src/packages/radio/icon.tsx (1 hunks)
  • src/packages/radio/radio.scss (1 hunks)
  • src/packages/radio/radio.taro.tsx (3 hunks)
  • src/packages/radio/radio.tsx (3 hunks)
💤 Files with no reviewable changes (4)
  • src/packages/radio/demo.taro.tsx
  • src/packages/radio/demo.tsx
  • src/packages/radio/demos/h5/demo7.tsx
  • src/packages/radio/demos/taro/demo7.tsx
🔇 Additional comments (5)
src/packages/radio/icon.tsx (1)

24-24: 导出语句使用正确

使用默认导出对单个组件来说是合适的选择。

src/packages/radio/radio.scss (1)

10-13: 正常状态样式使用变量,符合规范

使用 $color-text-disabled$color-background-overlay 变量来定义边框和背景色,保持了样式的一致性和可维护性。

src/packages/radio/radio.tsx (1)

7-7: 新增图标组件导入

从专用的图标模块导入 Icon 组件替代之前的图标组件,这样的重构提高了代码的模块化程度。

✅ Verification successful

Icon组件导入和使用验证完成

从代码分析结果来看:

  • Icon组件在 @/packages/radio/icon.tsx 中正确定义和导出
  • Radio组件正确引入并使用了 Icon 组件来渲染不同状态的图标(normal、disabled、checked、checked-disabled)
  • 这种重构确实提高了代码的模块化程度,使图标渲染逻辑更加清晰
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 验证新的 Icon 组件是否正确导出和使用
ast-grep --pattern 'export $_ Icon'

Length of output: 37


Script:

#!/bin/bash
# 检查 Icon 组件的定义和导出
fd "icon.tsx" --type f --exec cat {}

# 检查 Radio 组件中 Icon 的使用
fd "radio.tsx" --type f --exec cat {}

Length of output: 6604

src/packages/radio/radio.taro.tsx (2)

8-8: 导入新的 Icon 组件替代原有图标组件

从代码重构的角度来看,使用统一的 Icon 组件是一个很好的改进,可以提高代码的可维护性和一致性。


123-123: Label 渲染逻辑优化得当

使用条件渲染来处理 children 是一个很好的改进,避免了不必要的 DOM 节点创建。

Comment on lines +4 to +8
interface IconProps {
tag: any
classPrefix: string
name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}
Copy link

@coderabbitai coderabbitai bot Nov 22, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议改进 IconProps 接口中 tag 属性的类型定义

目前 tag 属性使用 any 类型会降低类型安全性。建议将其限制为具体的 HTML 元素类型或 React 组件类型。

建议修改如下:

interface IconProps {
-  tag: any
+  tag: keyof JSX.IntrinsicElements | React.ComponentType
  classPrefix: string
  name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}
📝 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
interface IconProps {
tag: any
classPrefix: string
name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}
interface IconProps {
tag: keyof JSX.IntrinsicElements | React.ComponentType
classPrefix: string
name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}

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

Choose a reason for hiding this comment

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

@irisSong,可以使用这个类型定义。将 tag 的类型改为 keyof JSX.IntrinsicElements | React.ComponentType 可以提高类型安全性,确保 tag 只能是特定的 HTML 元素类型或 React 组件类型。修改后的代码如下:

interface IconProps {
  tag: keyof JSX.IntrinsicElements | React.ComponentType
  classPrefix: string
  name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}

这样修改后,代码会更加严谨,有助于捕获类型错误。

Comment on lines +94 to +112

if (disabled && !checkedStatement) {
return <CheckDisabled className={classNames(color())} />
return <Icon name="disabled" />
}
if (checkedStatement) {
return React.isValidElement(activeIcon) ? (
React.cloneElement<any>(activeIcon, {
...activeIcon.props,
className: classNames(color()),
})
) : (
<CheckChecked className={classNames(color())} />
)
return React.isValidElement(activeIcon)
? React.cloneElement<any>(activeIcon, {
...activeIcon.props,
className: classNames(color()),
})
: renderIconByDisabledProperty()
}
return React.isValidElement(icon) ? (
React.cloneElement<any>(icon, {
...icon.props,
className: classNames(color()),
})
) : (
<CheckNormal className={classNames(color())} />
<Icon name="normal" />
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议简化图标渲染逻辑

当前的图标渲染逻辑包含多层嵌套条件,可以通过提取通用逻辑来简化代码。

建议重构如下:

-if (disabled && !checkedStatement) {
-  return <Icon name="disabled" />
-}
-if (checkedStatement) {
-  return React.isValidElement(activeIcon)
-    ? React.cloneElement<any>(activeIcon, {
-        ...activeIcon.props,
-        className: classNames(color()),
-      })
-    : renderIconByDisabledProperty()
-}
-return React.isValidElement(icon) ? (
-  React.cloneElement<any>(icon, {
-    ...icon.props,
-    className: classNames(color()),
-  })
-) : (
-  <Icon name="normal" />
-)
+const iconName = getIconName(disabled, checkedStatement)
+const customIcon = checkedStatement ? activeIcon : icon
+
+return React.isValidElement(customIcon)
+  ? React.cloneElement<any>(customIcon, {
+      ...customIcon.props,
+      className: classNames(color()),
+    })
+  : <Icon name={iconName} />

需要在组件外部添加辅助函数:

function getIconName(disabled: boolean, checked: boolean): string {
  if (disabled && !checked) return 'disabled'
  if (checked) return disabled ? 'checked-disabled' : 'checked'
  return 'normal'
}

Comment on lines +95 to +113

if (disabled && !checkedStatement) {
return <CheckDisabled className={classNames(color())} />
return <Icon name="disabled" tag={View} />
}
if (checkedStatement) {
return React.isValidElement(activeIcon) ? (
React.cloneElement<any>(activeIcon, {
...activeIcon.props,
className: classNames(color()),
})
) : (
<CheckChecked className={classNames(color())} />
)
return React.isValidElement(activeIcon)
? React.cloneElement<any>(activeIcon, {
...activeIcon.props,
className: classNames(color()),
})
: renderIconByDisabledProperty()
}
return React.isValidElement(icon) ? (
React.cloneElement<any>(icon, {
...icon.props,
className: classNames(color()),
})
) : (
<CheckNormal className={classNames(color())} />
<Icon name="normal" tag={View} />
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

优化图标渲染逻辑的条件判断

当前的条件判断结构较为复杂,建议重构以提高可读性和可维护性。

建议采用以下方式重构:

- if (disabled && !checkedStatement) {
-   return <Icon name="disabled" tag={View} />
- }
- if (checkedStatement) {
-   return React.isValidElement(activeIcon)
-     ? React.cloneElement<any>(activeIcon, {
-         ...activeIcon.props,
-         className: classNames(color()),
-       })
-     : renderIconByDisabledProperty()
- }
- return React.isValidElement(icon) ? (
-   React.cloneElement<any>(icon, {
-     ...icon.props,
-     className: classNames(color()),
-   })
- ) : (
-   <Icon name="normal" tag={View} />
- )
+ const getIcon = () => {
+   if (disabled && !checkedStatement) return 'disabled'
+   if (checkedStatement) return disabled ? 'checked-disabled' : 'checked'
+   return 'normal'
+ }
+ 
+ const customIcon = checkedStatement ? activeIcon : icon
+ if (React.isValidElement(customIcon)) {
+   return React.cloneElement<any>(customIcon, {
+     ...customIcon.props,
+     className: classNames(color()),
+   })
+ }
+ 
+ return <Icon name={getIcon()} tag={View} />

Committable suggestion skipped: line range outside the PR's diff.

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 (12)
src/packages/radio/icon.tsx (3)

10-22: 建议优化组件性能和类型安全性

  1. 建议使用 React.memo 优化渲染性能
  2. 建议添加 props 类型验证
  3. 可以使用 classnames 库优化类名组合

建议修改如下:

+import classNames from 'classnames'
+import { memo } from 'react'

-const Icon = (props: Partial<IconProps>) => {
+const Icon = memo((props: Partial<IconProps>) => {
  const { tag, classPrefix, name } = mergeProps(
    {
      tag: 'div',
      classPrefix: 'nut-radio-self',
      name: 'normal',
    },
    props
  )
  return createElement<{ className: string }>(tag, {
-    className: `${classPrefix} ${classPrefix}-${name}`,
+    className: classNames(classPrefix, `${classPrefix}-${name}`),
  })
-}
+})

24-24: 建议添加组件文档注释

为了提高代码可维护性,建议添加 JSDoc 文档注释,说明组件的用途和属性说明。

建议添加如下文档:

+/**
+ * Radio 组件的图标展示组件
+ * @param props.tag - 要渲染的 HTML 元素或组件
+ * @param props.classPrefix - 类名前缀
+ * @param props.name - 图标状态
+ */
export default Icon

1-24: 建议进行架构改进

为了提高组件的质量和可用性,建议考虑以下几点:

  1. 添加 ARIA 属性以提高可访问性
  2. 添加单元测试用例
  3. 考虑添加自定义图标支持

需要我帮助实现以上任何改进吗?我可以:

  1. 生成包含 ARIA 属性的实现代码
  2. 提供单元测试用例
  3. 实现自定义图标功能
src/packages/radio/radio.scss (6)

3-8: 基础样式设置合理,建议添加过渡效果

基础样式设置合理,尺寸和边框圆角的定义符合规范。但建议添加状态切换的过渡效果,以提升用户体验。

 .nut-radio-self {
   width: 16px;
   height: 16px;
   position: relative;
   box-sizing: border-box;
   border-radius: 50%;
+  transition: all 0.2s ease-in-out;
 }

10-13: 正常状态样式规范,建议使用变量定义边框宽度

边框宽度建议使用变量定义,以保持一致性和可维护性。

+$radio-border-width: 0.5px;
+
 &-normal {
-  border: 0.5px solid $color-text-disabled;
+  border: $radio-border-width solid $color-text-disabled;
   background: $color-background-overlay;
 }

15-27: 共享样式结构合理,建议优化伪元素定位

当前使用绝对定位和transform实现居中,可以考虑使用更现代的布局方式。

 &-checked,
 &-disabled,
 &-checked-disabled {
   &:after {
     display: block;
     position: absolute;
-    top: 50%;
-    left: 50%;
-    transform: translate(-50%, -50%);
+    inset: 0;
+    margin: auto;
     content: ' ';
     background: $color-primary-text;
   }
 }

29-42: 选中状态样式美观,建议添加暗色主题支持

渐变和阴影效果美观,但建议考虑添加暗色主题支持。同时,建议将渐变角度改为变量。

+$radio-gradient-angle: 90deg;
+
 &-checked {
   background: linear-gradient(
-    90deg,
+    $radio-gradient-angle,
     $color-primary-stop-1 0%,
     $color-primary-stop-2 100%
   );
   box-shadow: 0px 2px 4px 0px rgba(255, 15, 35, 0.2);

+  @media (prefers-color-scheme: dark) {
+    box-shadow: 0px 2px 4px 0px rgba(255, 15, 35, 0.1);
+  }
 }

44-51: 禁用状态样式合理,建议增加高对比度模式支持

为提高可访问性,建议添加高对比度模式的样式支持。

 &-disabled {
   background: $color-text-disabled;

+  @media (forced-colors: active) {
+    background: GrayText;
+  }
 }

53-61: 选中禁用状态样式规范,建议统一尺寸变量

建议将重复使用的尺寸值(如8px)提取为变量。

+$radio-indicator-size: 8px;
+
 &-checked-disabled {
   background: $color-primary-disabled-special;

   &:after {
-    width: 8px;
-    height: 8px;
+    width: $radio-indicator-size;
+    height: $radio-indicator-size;
     border-radius: 50%;
   }
 }
src/packages/radio/radio.tsx (2)

86-93: 建议优化图标渲染逻辑的可维护性

renderIconByDisabledProperty 函数的实现可以进一步优化:

  1. 函数名称不够直观,建议改为更具描述性的名称
  2. 可以考虑将图标名称抽取为常量

建议按照以下方式重构:

-function renderIconByDisabledProperty() {
+const ICON_NAMES = {
+  CHECKED: 'checked',
+  CHECKED_DISABLED: 'checked-disabled'
+} as const
+
+function renderCheckedIcon() {
   return disabled ? (
-    <Icon name="checked-disabled" />
+    <Icon name={ICON_NAMES.CHECKED_DISABLED} />
   ) : (
-    <Icon name="checked" />
+    <Icon name={ICON_NAMES.CHECKED} />
   )
 }

122-122: 优化标签渲染的性能

当前实现在每次渲染时都会进行条件判断。建议使用 useMemo 来优化性能。

建议重构为:

+const labelContent = useMemo(() => {
+  if (!children) return null;
+  return <div className={labelcls}>{children}</div>;
+}, [children, labelcls]);

-{children ? <div className={labelcls}>{children}</div> : null}
+{labelContent}
src/packages/radio/radio.taro.tsx (1)

87-113: 建议优化图标渲染逻辑的条件判断

当前实现有以下优点:

  • 通过renderIconByDisabledProperty辅助函数提高了代码可读性
  • 保持了自定义图标的灵活性

建议改进:

可以通过使用查找表(lookup table)简化条件判断:

  const renderIcon = () => {
    const { icon, activeIcon } = props
+   const iconMap = {
+     disabled: {
+       checked: () => renderIconByDisabledProperty(),
+       unchecked: () => <Icon name="disabled" tag={View} />
+     },
+     enabled: {
+       checked: () => React.isValidElement(activeIcon) 
+         ? React.cloneElement<any>(activeIcon, {
+             ...activeIcon.props,
+             className: classNames(color()),
+           })
+         : renderIconByDisabledProperty(),
+       unchecked: () => React.isValidElement(icon)
+         ? React.cloneElement<any>(icon, {
+             ...icon.props,
+             className: classNames(color()),
+           })
+         : <Icon name="normal" tag={View} />
+     }
+   }
+   const state = disabled ? 'disabled' : 'enabled'
+   const checkState = checkedStatement ? 'checked' : 'unchecked'
+   return iconMap[state][checkState]()
-   // 移除原有的嵌套条件判断
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 0648882 and c08869a.

📒 Files selected for processing (8)
  • src/packages/radio/demo.taro.tsx (0 hunks)
  • src/packages/radio/demo.tsx (0 hunks)
  • src/packages/radio/demos/h5/demo7.tsx (0 hunks)
  • src/packages/radio/demos/taro/demo7.tsx (0 hunks)
  • src/packages/radio/icon.tsx (1 hunks)
  • src/packages/radio/radio.scss (1 hunks)
  • src/packages/radio/radio.taro.tsx (3 hunks)
  • src/packages/radio/radio.tsx (3 hunks)
💤 Files with no reviewable changes (4)
  • src/packages/radio/demo.taro.tsx
  • src/packages/radio/demo.tsx
  • src/packages/radio/demos/h5/demo7.tsx
  • src/packages/radio/demos/taro/demo7.tsx
🔇 Additional comments (3)
src/packages/radio/radio.tsx (1)

7-7: 新增 Icon 组件导入需要验证其实现

请确保新引入的 Icon 组件已经完整实现并经过测试。

✅ Verification successful

Icon 组件已正确实现,无需担心

Icon 组件已在 src/packages/radio/icon.tsx 中完整实现,包含以下特性:

  • 支持自定义标签渲染
  • 包含完整的类型定义
  • 提供了必要的样式类名处理
  • 使用 mergeProps 处理默认值
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 验证 Icon 组件的实现
ast-grep --pattern 'export default function Icon' src/packages/radio/icon.tsx

Length of output: 79


Script:

#!/bin/bash
# 检查 icon.tsx 文件是否存在
fd "icon.tsx" src/packages/radio

# 如果文件存在,查看其内容
fd "icon.tsx" src/packages/radio -x cat {}

# 搜索任何可能的 Icon 组件定义(包括其他可能的导出形式)
ast-grep --pattern 'const Icon = ' 
ast-grep --pattern 'function Icon'
ast-grep --pattern 'class Icon'

Length of output: 758

src/packages/radio/radio.taro.tsx (2)

8-8: 导入更改优化了图标管理!

将多个具体图标组件的导入整合为单个Icon组件的导入,这种改进:

  • 简化了导入语句
  • 集中了图标管理
  • 提高了代码的可维护性

123-123: 优化了标签渲染逻辑!

通过添加条件渲染检查,避免了在没有子元素时渲染空的View标签,这样的优化:

  • 提高了DOM渲染效率
  • 减少了不必要的空元素

Comment on lines +4 to +8
interface IconProps {
tag: any
classPrefix: string
name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议改进 IconProps 接口中 tag 属性的类型定义

目前 tag 属性使用 any 类型会降低类型安全性。建议使用更具体的类型定义。

建议修改如下:

interface IconProps {
-  tag: any
+  tag: keyof JSX.IntrinsicElements | React.ComponentType
  classPrefix: string
  name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}
📝 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
interface IconProps {
tag: any
classPrefix: string
name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}
interface IconProps {
tag: keyof JSX.IntrinsicElements | React.ComponentType
classPrefix: string
name: 'normal' | 'disabled' | 'checked' | 'checked-disabled'
}

if (disabled && !checkedStatement) {
return <CheckDisabled className={classNames(color())} />
return <Icon name="disabled" />
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议统一图标渲染的条件判断结构

当前的条件判断结构较为分散,建议统一处理逻辑以提高可读性。

建议重构为:

-if (disabled && !checkedStatement) {
-  return <Icon name="disabled" />
-}
-if (checkedStatement) {
-  return React.isValidElement(activeIcon)
-    ? React.cloneElement<any>(activeIcon, {
-        ...activeIcon.props,
-        className: classNames(color()),
-      })
-    : renderIconByDisabledProperty()
-}
-return React.isValidElement(icon) ? (
-  React.cloneElement<any>(icon, {
-    ...icon.props,
-    className: classNames(color()),
-  })
-) : (
-  <Icon name="normal" />
-)
+const renderIcon = () => {
+  if (disabled && !checkedStatement) {
+    return <Icon name="disabled" />
+  }
+  
+  const currentIcon = checkedStatement ? activeIcon : icon
+  if (React.isValidElement(currentIcon)) {
+    return React.cloneElement<any>(currentIcon, {
+      ...currentIcon.props,
+      className: classNames(color()),
+    })
+  }
+  
+  if (checkedStatement) {
+    return renderCheckedIcon()
+  }
+  
+  return <Icon name="normal" />
+}

Also applies to: 99-104, 112-112

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.01%. Comparing base (beb7be2) to head (75fdcc7).
Report is 63 commits behind head on feat_v3.x.

Files with missing lines Patch % Lines
src/packages/radio/radio.tsx 71.42% 4 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           feat_v3.x    #2786      +/-   ##
=============================================
+ Coverage      83.98%   84.01%   +0.02%     
=============================================
  Files            220      221       +1     
  Lines          17881    17906      +25     
  Branches        2676     2683       +7     
=============================================
+ Hits           15018    15044      +26     
+ Misses          2858     2857       -1     
  Partials           5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 2e3a079 and b93ff79.

📒 Files selected for processing (1)
  • src/packages/radio/__test__/radio.spec.tsx (1 hunks)
🔇 Additional comments (1)
src/packages/radio/__test__/radio.spec.tsx (1)

19-19: 请验证禁用状态的类名更改是否与组件实现一致

测试用例中将禁用状态的类名从 nut-radio-icon-disabled 更改为 nut-radio-self-disabled,这个改动需要确保与 Radio 组件的实现保持同步。

expect(container.querySelector('.nut-icon')).toHaveClass(
'nut-icon-CheckChecked'
)
expect(container.querySelector('.nut-radio-self-checked')).toBeTruthy()
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

测试用例中的类名与组件实现不一致

根据 Radio 组件的源码实现,选中状态使用的是 nut-radio-icon-checked 类名,而不是测试用例中使用的 nut-radio-self-checked。测试用例需要更新以匹配实际的组件实现。

  • src/packages/radio/__test__/radio.spec.tsx 中,将 .nut-radio-self-checked 更改为 .nut-radio-icon-checked
🔗 Analysis chain

请验证选中状态的类名更改是否与组件实现一致

测试用例中将选中状态的类名从 nut-icon-CheckChecked 更改为 nut-radio-self-checked,这个改动需要确保与 Radio 组件的实现保持同步。


Let me gather more information about the Radio component implementation to verify the class name change.


Let me try a different approach to find the class name usage in the Radio component.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 Radio 组件中选中状态的类名使用
rg -A 2 'nut-radio-self-checked' src/packages/radio/

Length of output: 263


Script:

#!/bin/bash
# Search for class names in Radio component implementation
ast-grep --pattern 'className={$_}' src/packages/radio/radio.tsx

Length of output: 66


Script:

#!/bin/bash
# Search for the Radio component implementation and class names
cat src/packages/radio/radio.tsx

Length of output: 4313

@@ -87,10 +83,6 @@ const RadioDemo = () => {
</Cell>
<View className="h2">{translated.c1bae1ec}</View>
<Demo6 />
<View className="h2">{translated['8a2e2847']}</View>
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个不支持了吗

@oasis-cloud
Copy link
Collaborator Author

等待 Icon,所以转 Draft

@oasis-cloud oasis-cloud marked this pull request as draft December 5, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants