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

fix(progress): fix style & support icon demo #2729

Open
wants to merge 8 commits into
base: V3.0
Choose a base branch
from

Conversation

Alex-huxiyang
Copy link
Collaborator

@Alex-huxiyang Alex-huxiyang commented Nov 11, 2024

🤔 这个变动的性质是?

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

🔗 相关 Issue

💡 需求背景和解决方案

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

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

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

Summary by CodeRabbit

  • 新功能
    • Demo4 组件现在无条件渲染,提升了可见性。
    • Demo6 组件简化了 CheckedTips 组件的渲染逻辑。
  • 样式
    • 修改了 .nut-progress-text 类的 CSS 属性,以确保文本在进度条上居中显示。
    • 更新了 Progress 组件的文本位置样式,使其在不同平台上保持一致。

Copy link

coderabbitai bot commented Nov 11, 2024

Walkthrough

本次更改涉及多个组件的修改,主要集中在ProgressDemoDemo4Demo6progress.scssprogress.taro.tsx文件中。ProgressDemo组件的条件渲染被移除,Demo4组件的图像尺寸属性格式进行了调整,Demo6组件添加了新的样式和简化了条件渲染逻辑。此外,progress.scss文件中对文本位置的CSS样式进行了修改,而progress.taro.tsx文件中则调整了文本相对于进度条的定位。

Changes

文件路径 更改摘要
src/packages/progress/demo.taro.tsx 移除Demo4组件的条件渲染,Demo4及其头部无条件渲染。
src/packages/progress/demos/taro/demo4.tsx Image组件的widthheight属性从字符串格式改为数字格式。
src/packages/progress/demos/taro/demo6.tsx 添加pxTransform工具函数,简化CheckedTips组件的条件渲染,统一使用iconStyle样式。
src/packages/progress/progress.scss 修改.nut-progress-text类的CSS属性,调整文本位置以支持RTL布局。
src/packages/progress/progress.taro.tsx 修改getTextStyle函数中rightleft属性的返回值为固定的'100%',调整文本相对于进度条的位置。

Possibly related PRs

Suggested labels

size/L

Suggested reviewers

  • xiaoyatong
  • oasis-cloud

🐰 在进度条上跳跃,
组件变得更加明亮,
条件渲染已不再,
图像尺寸轻松改,
风格统一真美好,
兔子欢快乐逍遥! 🌟


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 11, 2024
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.98%. Comparing base (4b427a1) to head (a5a85ea).

Additional details and impacted files
@@           Coverage Diff           @@
##             V3.0    #2729   +/-   ##
=======================================
  Coverage   83.98%   83.98%           
=======================================
  Files         221      221           
  Lines       17879    17879           
  Branches     2676     2676           
=======================================
  Hits        15016    15016           
  Misses       2858     2858           
  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: 0

🧹 Outside diff range and nitpick comments (3)
src/packages/progress/demos/taro/demo4.tsx (1)

10-11: 优化图片尺寸的响应式处理

将固定像素值改为使用 pxTransform 是一个很好的改进,这样可以:

  • 确保在不同设备上保持适当的显示比例
  • 提高组件的跨平台兼容性
  • 符合 Taro 的最佳实践

建议考虑将这些转换后的值定义为常量,以提高代码的可维护性。

const Demo4 = () => {
+ const ICON_SIZE = pxTransform(20)
  return (
    <Cell>
      <Progress percent={60} showText>
        <Image
-         width={pxTransform(20)}
-         height={pxTransform(20)}
+         width={ICON_SIZE}
+         height={ICON_SIZE}
          src="https://img12.360buyimg.com/imagetools/jfs/t1/229362/18/22746/11607/669f8bfeF5fcbeaab/d4162bbf29bb1b00.png"
        />
      </Progress>
    </Cell>
  )
}
src/packages/progress/demos/taro/demo6.tsx (1)

8-12: 建议优化 iconStyle 对象的声明方式

虽然当前实现可以正常工作,但建议考虑以下优化:

  1. 将 iconStyle 提升到组件外部以避免重复创建
  2. 考虑使用 useMemo 来优化性能

建议修改如下:

-const Demo6 = () => {
-  const iconStyle = {
-    marginTop: 0,
-    marginLeft: pxTransform(5),
-    marginRight: pxTransform(5),
-  }
+const iconStyle = {
+  marginTop: 0,
+  marginLeft: pxTransform(5),
+  marginRight: pxTransform(5),
+}
+
+const Demo6 = () => {
src/packages/progress/progress.taro.tsx (1)

163-163: 文本定位逻辑优化

将动态百分比值改为固定值 '100%' 是一个更好的实现方式,因为:

  1. 配合 CSS transform 可以更准确地实现文本居中
  2. 简化了 RTL 和 LTR 布局的处理逻辑
  3. 提高了跨平台兼容性

建议添加注释说明这个固定值配合 CSS transform 使用的原因:

  getTextStyle = () => {
+   // 使用固定的 100% 配合 CSS transform 实现文本居中,确保在 RTL/LTR 布局下都能正确定位
    return rtl ? { right: '100%' } : { left: '100%' }
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ce66f75 and 2e1b878.

📒 Files selected for processing (6)
  • packages/nutui-taro-demo/src/app.config.ts (5 hunks)
  • src/packages/progress/demo.taro.tsx (1 hunks)
  • src/packages/progress/demos/taro/demo4.tsx (1 hunks)
  • src/packages/progress/demos/taro/demo6.tsx (2 hunks)
  • src/packages/progress/progress.scss (1 hunks)
  • src/packages/progress/progress.taro.tsx (1 hunks)
🔇 Additional comments (7)
src/packages/progress/demos/taro/demo4.tsx (1)

3-3: 导入 pxTransform 工具函数用于响应式单位转换

引入 pxTransform 工具函数是一个很好的改进,这有助于确保组件在不同设备上具有一致的显示效果。

src/packages/progress/demos/taro/demo6.tsx (2)

5-5: 导入 pxTransform 工具函数以统一单位转换

引入 pxTransform 工具函数有助于确保在不同平台上保持一致的单位转换逻辑。


28-28: 图标样式统一化处理得当

使用统一的 iconStyle 对象来处理 Checked 和 Tips 图标的样式是个很好的改进,这样可以:

  1. 确保样式的一致性
  2. 便于后续的维护和修改

Also applies to: 32-32

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

42-42: 文本居中定位属性修复已确认!

启用 transform: translate(-50%) 属性是正确的做法,这确保了进度条文本能够在水平方向上精确居中,与绝对定位配合使用时能够避免文本偏移问题。

让我们验证在不同屏幕尺寸下的显示效果:

✅ Verification successful

文本居中定位属性验证完成,RTL支持正确!

经过代码分析,transform: translate(-50%) 属性的使用是正确的,并且已经正确处理了RTL布局的支持:

  • LTR布局下使用 transform: translate(-50%) 实现水平居中
  • RTL布局下使用 transform: translate(50%) 实现水平居中
  • 相关样式已经在 .nut-rtl[dir=rtl] 选择器中正确配置

这种实现方式确保了进度条文本在不同语言方向下都能保持正确的居中定位。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查是否存在与进度条文本定位相关的响应式样式
# 预期:确认文本居中样式在各种屏幕尺寸下的一致性

# 搜索所有与进度条相关的媒体查询
rg -l "@media.*nut-progress" 

# 检查是否有其他地方覆盖了文本定位样式
rg "transform:.*translate.*nut-progress-text"

Length of output: 78


Script:

#!/bin/bash
# 检查进度条组件的所有样式文件
fd -e scss -e css progress

# 检查所有包含 transform 或 translate 的样式规则
rg -A 2 -B 2 "transform|translate" --type css

# 检查所有媒体查询相关样式
rg "@media" --type css -A 3 -B 3

# 检查RTL相关样式
rg "rtl|direction" --type css -A 3 -B 3

Length of output: 373544

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

59-60: 代码逻辑简化,但需要验证平台兼容性

移除条件渲染使代码结构更加清晰,但建议确认 Demo4 在所有目标平台上都能正常工作。

运行以下脚本以验证 Demo4 的平台兼容性:

✅ Verification successful

移除条件渲染是安全的,代码验证通过

Demo4 组件使用了跨平台的 @nutui/nutui-react-taro 组件库和通用的 pxTransform 工具函数,没有使用任何平台特定的 API 或组件。移除条件渲染不会影响其在不同平台上的兼容性。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查 Demo4 中是否存在特定平台的 API 或组件使用

# 测试:搜索 Demo4 实现中的平台特定代码
ast-grep --pattern 'import { $_ } from "@tarojs/components"' src/packages/progress/demos/taro/demo4.tsx

# 测试:检查是否有平台特定的条件判断
rg -A 3 'process\.env\.TARO_ENV' src/packages/progress/demos/taro/demo4.tsx

Length of output: 183


Script:

#!/bin/bash
# 首先检查 Demo4 文件是否存在
fd "demo4\.tsx" src/packages/progress

# 如果文件不在预期位置,尝试其他可能的位置
fd "demo4\.(tsx|jsx)" src/packages

# 检查 demo.taro.tsx 中 Demo4 的导入
rg "import.*Demo4" src/packages/progress/demo.taro.tsx -A 2

# 查看 demo.taro.tsx 的完整内容以了解上下文
cat src/packages/progress/demo.taro.tsx

Length of output: 8578


Script:

#!/bin/bash
# 检查 Demo4 的具体实现
cat src/packages/progress/demos/taro/demo4.tsx

# 检查 Demo4 是否使用了平台特定的 API
rg "process\.env\.TARO_ENV|@tarojs/(components|api)" src/packages/progress/demos/taro/demo4.tsx

# 检查是否有其他平台特定的导入
rg "import.*from.*@tarojs" src/packages/progress/demos/taro/demo4.tsx

Length of output: 714

packages/nutui-taro-demo/src/app.config.ts (2)

124-133: 配置结构优化合理

应用配置的改动保持了良好的一致性,window 配置的属性值符合规范。


51-52: 请确认路由命名变更的影响范围

将子包的 root 从 "dataentry" 更改为 "dentry1" 可能会影响现有的路由跳转逻辑。建议:

  1. 确认是否需要更新相关文档
  2. 检查是否有依赖原路径的代码需要同步修改

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.

关注下 js 中截的图

},
{
"root": "dataentry",
"pages": [
root: "dentry1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个不大对~

packages/nutui-taro-demo/src/app.config.ts Outdated Show resolved Hide resolved
src/packages/progress/demos/taro/demo4.tsx Show resolved Hide resolved
src/packages/progress/progress.taro.tsx Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/M and removed size/L labels Nov 11, 2024
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