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(elevator): harmony适配 #2836

Merged
merged 4 commits into from
Dec 12, 2024
Merged

Conversation

irisSong
Copy link
Collaborator

@irisSong irisSong commented Dec 6, 2024

🤔 这个变动的性质是?

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

🔗 相关 Issue

💡 需求背景和解决方案

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

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

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

Summary by CodeRabbit

  • 新功能

    • 更新了电梯组件的样式,增强了固定项的视觉展示。
    • 引入了新的 .nut-elevator-list-fixed-title 类,用于更好地展示固定列表标题。
  • 改进

    • 优化了电梯组件的结构和功能,提升了可维护性。
    • 更新了触摸交互的索引计算方式,改善用户体验。

Copy link

coderabbitai bot commented Dec 6, 2024

Walkthrough

本次更改涉及对elevator.scsselevator.taro.tsx文件的修改。elevator.scss中对.nut-elevator-list-fixed类进行了样式调整,增加了display: flexalign-items: center属性,并引入了新的嵌套类.nut-elevator-list-fixed-title。在elevator.taro.tsx中,重构了Elevator组件,使用useMemo优化了样式计算,并调整了触摸移动的索引计算方式,增强了组件的功能性和可维护性。

Changes

文件路径 更改摘要
src/packages/elevator/elevator.scss 修改了.nut-elevator-list-fixed类,增加display: flexalign-items: center,移除了一些字体属性,新增嵌套类.nut-elevator-list-fixed-title
src/packages/elevator/elevator.taro.tsx 调整了Elevator组件,重新排列了createContext的导入,新增了useMemoText组件的导入。引入getWrapStyle函数替代内联样式,修改了touchMove函数的索引计算方式。

Possibly related PRs

Suggested labels

size/L

Suggested reviewers

  • xiaoyatong
  • oasis-cloud

兔子欢快跳跃,样式焕新颜,
电梯更灵动,功能更优良。
代码如春风,轻拂每一行,
让我们一起庆祝,欢声笑语扬! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa55e1 and 6d0d987.

📒 Files selected for processing (1)
  • src/packages/elevator/elevator.taro.tsx (7 hunks)
🔇 Additional comments (6)
src/packages/elevator/elevator.taro.tsx (6)

6-13: 导入更改看起来合理!

新增的 useMemoharmony 工具函数的导入符合组件的需求,导入顺序也符合最佳实践。


152-152: 触摸移动计算的精确度提升

Math.floor 改为 Math.round 是个很好的改进:

  • 使用 Math.round 可以更准确地计算目标索引位置
  • 提供了更自然的滚动体验,因为它会四舍五入到最接近的索引值

236-236: 样式应用优化得当

使用 getWrapStyle 替代内联样式计算是很好的改进,可以避免不必要的重复计算。


216-220: 样式计算优化建议

使用 useMemo 缓存样式计算是个好的优化,但是有以下几点需要注意:

  1. harmony 平台的高度处理逻辑需要验证
  2. 建议添加注释说明为什么 harmony 平台需要特殊处理
✅ Verification successful

Based on the search results, I can now generate the final response since I have enough context about how harmony() is used across the codebase.

harmony() 平台样式处理符合规范

经过验证,当前的 harmony 平台高度处理逻辑是合理的:

  1. pxTransform 工具函数中可以看到,harmony 平台需要特殊的数值转换
  2. 其他组件如 Progress、Button、Toast 等也都对 harmony 平台的样式进行了特殊处理
  3. 当前实现将非数字的 height 保持原样,数字类型则转为 px 单位,这与 pxTransform 的处理逻辑一致

建议在代码中添加注释说明这种特殊处理的原因,以便其他开发者理解。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查其他组件是否有类似的 harmony 平台适配逻辑

# 搜索其他文件中的 harmony 相关代码
rg "harmony\(\)" -A 3 -B 3

Length of output: 9303


315-318: 🛠️ Refactor suggestion

触摸事件处理的改进建议

事件处理的改动有以下需要注意的点:

  1. 使用 as any 进行类型转换不是最佳实践,建议定义具体的事件类型
  2. touchAction: 'pan-y' 的兼容性需要验证,特别是在不同平台上

建议改进类型定义:

- onTouchStart={(event) => touchStart(event as any)}
+ onTouchStart={(event: React.TouchEvent<HTMLDivElement>) => touchStart(event)}
✅ Verification successful

Let me gather more information about the touch event handling implementation.


Based on the search results, I can now provide the final response:

触摸事件处理实现符合最佳实践

  1. touchAction: 'pan-y' 的使用是合适的,在 elevator 组件中已经得到验证,且在其他组件中也有相同的实现
  2. 事件类型转换可以保持现状,因为:
    • 在 Taro 环境中使用 as any 是常见做法
    • 代码库中大量使用了 ITouchEvent 类型,表明这是项目的既定实践
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查其他触摸相关组件的实现方式

# 搜索其他文件中的 touchAction 使用情况
rg "touchAction:" -A 2 -B 2

Length of output: 823


Script:

#!/bin/bash
# 检查触摸事件处理的实现
ast-grep --pattern 'onTouch$_: ($$$) => $_'

# 检查是否有其他使用 TouchEvent 类型的地方
rg "TouchEvent" -A 2 -B 2

# 检查其他文件中的事件类型定义
ast-grep --pattern 'type $_Event = $_'

Length of output: 46490


330-332: 固定标题渲染的改进建议

使用 Taro 的 Text 组件替代 span 是正确的改动,但建议:

  1. 确保其他类似场景也使用了 Text 组件
  2. 验证新增的样式类 nut-elevator-list-fixed-title 在所有主题下的表现
✅ Verification successful

使用 Text 组件和样式类的改动已验证

代码检查显示:

  1. 在 Taro 版本的 Elevator 组件中已经统一使用了 Text 组件来渲染固定标题
  2. 样式类 nut-elevator-list-fixed-title 在 SCSS 文件中有正确定义,包含了字体大小、颜色和字重等样式属性
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查组件中 span 到 Text 的迁移情况

# 搜索仍在使用 span 的地方
rg "<span" --type tsx

# 搜索 Text 组件的使用情况
rg "<Text" --type tsx

Length of output: 103


Script:

#!/bin/bash
# Search for span and Text components in all files
rg "<span" 
rg "<Text"

# Search specifically in the elevator component directory
fd . src/packages/elevator -t f -e tsx -e jsx | xargs rg "<span"
fd . src/packages/elevator -t f -e tsx -e jsx | xargs rg "<Text"

# Check style definitions
rg "list-fixed-title" 

Length of output: 65756


Script:

#!/bin/bash
# Search for Text component usage in elevator component
fd . src/packages/elevator -t f -e tsx -e jsx | xargs cat

# Search for style definitions related to fixed title
fd . src/packages/elevator -t f -e scss -e css | xargs cat

Length of output: 46389


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

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.13%. Comparing base (9bd8230) to head (6d0d987).
Report is 20 commits behind head on feat_v3.x.

Additional details and impacted files
@@            Coverage Diff             @@
##           feat_v3.x    #2836   +/-   ##
==========================================
  Coverage      84.13%   84.13%           
==========================================
  Files            273      273           
  Lines          18101    18101           
  Branches        2691     2691           
==========================================
  Hits           15230    15230           
  Misses          2866     2866           
  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 (1)
src/packages/elevator/elevator.taro.tsx (1)

9-13: 建议:优化导入顺序

建议将相关的导入语句组织在一起,比如将所有Taro相关的导入放在一组,这样可以提高代码的可读性。

-import Taro, { nextTick, createSelectorQuery } from '@tarojs/taro'
-import { ScrollView, View, Text } from '@tarojs/components'
-import classNames from 'classnames'
-import { BasicComponent, ComponentDefaults } from '@/utils/typings'
-import { harmony } from '@/utils/platform-taro'
+// Taro相关导入
+import Taro, { nextTick, createSelectorQuery } from '@tarojs/taro'
+import { ScrollView, View, Text } from '@tarojs/components'
+
+// 工具库导入
+import classNames from 'classnames'
+
+// 内部工具导入
+import { BasicComponent, ComponentDefaults } from '@/utils/typings'
+import { harmony } from '@/utils/platform-taro'
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd8230 and 2fa55e1.

📒 Files selected for processing (2)
  • src/packages/elevator/elevator.scss (1 hunks)
  • src/packages/elevator/elevator.taro.tsx (7 hunks)
🔇 Additional comments (6)
src/packages/elevator/elevator.scss (2)

54-55: 布局优化:使用flex布局改进对齐方式

使用display: flexalign-items: center优化了固定标题的布局,这样可以确保内容垂直居中对齐,提高了视觉表现。


66-70: 结构优化:将标题样式移至独立的子类

将标题的样式属性移至.nut-elevator-list-fixed-title子类中,这种结构更清晰,也更易于维护。

src/packages/elevator/elevator.taro.tsx (4)

216-220: 性能优化:使用useMemo缓存样式计算

使用useMemo缓存样式计算结果是一个很好的优化,可以避免不必要的重复计算。同时,通过harmony()函数的判断来适配不同平台的高度表示方式也很合理。


315-318: 改进:触摸事件处理优化

添加了touchAction: 'pan-y'样式属性,这有助于改善移动端的滚动体验。同时事件处理函数的类型转换也更加规范。


330-332: 适配:更新为Taro组件

span替换为Taro的Text组件,这符合跨平台开发的最佳实践。


152-154: 建议:验证索引计算的精确性

Math.floor改为Math.round可能会影响滚动的精确性。建议进行充分测试,确保这个改动在各种场景下都能正常工作。

✅ Verification successful

Let me search for more context about how this index calculation is used.


从代码分析来看,我可以生成最终响应了。

Math.round 的使用是合理的

在电梯组件(Elevator)中使用 Math.round 是正确的选择,原因如下:

  • 该计算用于确定滚动位置时的索引值,Math.round 可以确保滚动停止在最接近的项目位置
  • 查看实现可知,delta 是基于触摸移动距离计算得出的浮点数,需要四舍五入到最近的整数索引
  • 这种实现方式在其他类似的滚动组件中也有使用,如 Picker 组件也使用了类似的逻辑
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 搜索相关的测试用例
rg -l "Math\.(floor|round)" "test" || echo "未找到相关测试文件,建议添加测试用例验证此改动"

Length of output: 390


Script:

#!/bin/bash
# 搜索 elevator.taro.tsx 文件中的相关实现
fd -e tsx -e ts "elevator" --exec cat {}

# 搜索其他可能使用到 Math.round 或 Math.floor 的相关文件
rg "Math\.(floor|round)" -A 3

Length of output: 40234

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