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: support border-radius on cursor #8

Merged
merged 7 commits into from
Oct 12, 2024
Merged

feat: support border-radius on cursor #8

merged 7 commits into from
Oct 12, 2024

Conversation

huhuanming
Copy link
Contributor

@huhuanming huhuanming commented Oct 11, 2024

Summary by CodeRabbit

  • 新功能

    • 增强了页面内容视图和页眉光标的动画处理。
    • 新增了可选的样式选项,提升了页面头部的灵活性。
    • 新增了用于跟踪滚动视图宽度的属性。
  • 改进

    • 更新了滚动视图中选定项的居中逻辑。
    • 优化了光标的宽度计算和渲染行为。
    • 改进了组件的响应性和灵活性。
    • 更新了动画处理的配置,提升了动画效果。

Copy link

coderabbitai bot commented Oct 11, 2024

Walkthrough

此次更改涉及多个组件,包括 PageContentViewPageHeaderCursorPageHeaderView。主要修改包括动画配置中的 useNativeDriver 属性更改、状态更新逻辑的增强、光标尺寸和渲染行为的优化,以及新属性的添加。这些更新提升了组件的灵活性和响应性。

Changes

文件路径 更改摘要
.gitignore, package.json - 在 .gitignore 中新增条目 .history/
- 更新 package.json@onekeyfe/react-native-tab-page-view 的版本号,从 1.0.15 改为 1.0.16
src/PageContentView.tsx - _event 动画配置中的 useNativeDrivertrue 更改为 false
- _onLayout 方法添加条件,检查 reloadWidthscrollViewWidth 的值。
src/PageHeaderCursor.tsx - reloadItemListContainerLayout 方法检查所有项目是否已测量。
- _findPercentCursorWidth 方法的条件从 == 改为 ===
- _reloadPageIndexValue 方法调整光标宽度计算。
- 重命名变量 scaleXwidthXcontainerStyle 仅应用 translateX
- 更新渲染逻辑,使用 Animated.View
src/PageHeaderView.tsx - 新增属性 scrollViewWidthPageHeaderView 类。
- scrollPageIndexValue 初始化中 useNativeDriver 设置为 false
- itemContainerStyle 方法接受可选的 index 参数。
- scrollTo 方法居中选定项目。
- onLayout 事件处理器捕获滚动视图的宽度。
- shouldComponentUpdate 方法重置 nextScrollPageIndex

🪧 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 or 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.

@huhuanming huhuanming enabled auto-merge (squash) October 11, 2024 15:53
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: 4

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 750a406 and 5cc676c.

📒 Files selected for processing (3)
  • src/PageContentView.tsx (2 hunks)
  • src/PageHeaderCursor.tsx (4 hunks)
  • src/PageHeaderView.tsx (7 hunks)
🧰 Additional context used
🪛 Biome
src/PageHeaderCursor.tsx

[error] 152-152: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.

Unsafe fix: Use a SelfClosingElement instead

(lint/style/useSelfClosingElements)

🔇 Additional comments (7)
src/PageContentView.tsx (2)

67-67: 性能影响:useNativeDriver 设置为 false

useNativeDriver 改为 false 可能会影响滚动性能。动画现在将在 JavaScript 线程上运行,而不是原生 UI 线程。

请解释这一更改的原因。是否有特定的非原生支持的属性需要动画?


94-98: 条件优化:_onLayout 方法

新增的条件检查提高了组件的稳健性,避免了不必要的状态更新。好的改进!

小建议:考虑使用可选链和空值合并运算符简化 data 长度检查:

(this.props.data?.length ?? 0) > 0

这样可以让代码更简洁。

src/PageHeaderCursor.tsx (4)

81-81: 代码看起来不错

该行修改逻辑正确。


116-116: 代码看起来不错

该行修改逻辑正确。


119-120: 代码看起来不错

这两行修改逻辑正确。


130-130: 代码看起来不错

该行修改逻辑正确。

src/PageHeaderView.tsx (1)

135-135: 确保 scrollViewWidth 在使用前已初始化

计算滚动位置时,使用了 scrollViewWidth。请确保在调用 scrollTo 方法前,scrollViewWidth 已通过 onLayout 事件正确赋值,避免滚动计算错误。

src/PageHeaderCursor.tsx Outdated Show resolved Hide resolved
src/PageHeaderView.tsx Show resolved Hide resolved
src/PageHeaderView.tsx Show resolved Hide resolved
src/PageHeaderView.tsx Outdated Show resolved Hide resolved
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 comments (1)
src/PageContentView.tsx (1)

Line range hint 78-85: 避免在异常处理中吞掉错误

scrollPageIndex 方法中,catch 块为空,这可能导致错误被忽略。建议至少记录错误信息,方便调试和维护。

建议如下修改:

try {
  const scrollConfig = { index: pageIndex, animated: animated ?? false };
  this?.scrollView?.current?.scrollToIndex?.(scrollConfig);
} catch (e) {
+  console.error('scrollPageIndex 出现错误:', e);
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 6ffa579 and 4b9e817.

📒 Files selected for processing (2)
  • src/PageContentView.tsx (1 hunks)
  • src/PageHeaderCursor.tsx (5 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/PageContentView.tsx (1)

67-67: ⚠️ Potential issue

注意:将 useNativeDriver 从 true 更改为 false

useNativeDriver 设置为 false 可能会导致动画在 JavaScript 线程上运行,影响性能。请确认此更改是必要的,并已考虑可能的性能影响。

src/PageHeaderCursor.tsx (1)

47-47: 使用 '===' 进行严格类型检查

将 '==' 改为 '===',确保类型和值的严格比较,提高代码的可靠性。

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

🧹 Outside diff range comments (4)
src/PageContentView.tsx (1)

Line range hint 89-95: 布局更新逻辑优化

新增的条件检查提高了状态更新的精确性。这是个好改进!

建议:为提高可读性,考虑将复杂的条件拆分:

const shouldUpdateLayout = 
  reloadWidth !== this.state.scrollViewWidth &&
  reloadWidth > 0 &&
  (this?.props?.data?.length ?? 0) > 0;

if (shouldUpdateLayout) {
  // 现有的更新逻辑
}

这样可以让代码更易理解和维护。

src/PageHeaderView.tsx (3)

Line range hint 72-74: 修正拼写错误:'_animtedEnabledValue'

在第 74 行的 itemConfigList 初始化中,_animtedEnabledValue 拼写错误,应该是 _animatedEnabledValue

请修改如下:

- _animtedEnabledValue: new Animated.Value(1),
+ _animatedEnabledValue: new Animated.Value(1),

Line range hint 124-132: 改进 shouldComponentUpdate 的数据比较逻辑

当前使用 nextProps.data !== this.props.data 进行浅比较,可能无法检测到 data 内容的变化。如果数组内容改变但引用未变,组件可能不会更新。

建议使用深度比较或根据数据的关键属性进行比较,例如:

- if (nextProps.data !== this.props.data) {
+ if (JSON.stringify(nextProps.data) !== JSON.stringify(this.props.data)) {

或者引入 lodash.isequal

+ import isEqual from 'lodash.isequal';
...
- if (nextProps.data !== this.props.data) {
+ if (!isEqual(nextProps.data, this.props.data)) {

Line range hint 88-96: 确保 scrollViewWidth 已初始化

addScrollPageIndexListener 方法中,可能在 scrollViewWidth 未初始化时就使用了它,导致滚动计算不准确。

建议在使用 scrollViewWidth 前,确保它已在 onLayout 中正确设置。或者在计算滚动位置时,添加检查:

- x: itemLayout.x + itemLayout.width / 2.0 - this.scrollViewWidth / 2.0,
+ x: itemLayout.x + itemLayout.width / 2.0 - (this.scrollViewWidth || 0) / 2.0,

这样可以避免因未初始化导致的计算错误。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 4b9e817 and 9ee205e.

📒 Files selected for processing (4)
  • .gitignore (1 hunks)
  • package.json (1 hunks)
  • src/PageContentView.tsx (1 hunks)
  • src/PageHeaderView.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (3)
.gitignore (1)

80-80: 很好,忽略了 .history/ 目录。

这个改动很合理。.history/ 通常是由某些 IDE 或插件创建的,用于存储本地文件历史。忽略它可以保持仓库整洁。

package.json (1)

3-3: 版本号更新正确。

版本从 1.0.15 升至 1.0.16,表明有新特性或非破坏性更改。好的版本管理!

src/PageContentView.tsx (1)

Line range hint 1-154: 总体评价

这些更改主要涉及动画处理和布局更新逻辑的优化。虽然整体方向是积极的,但有几点需要注意:

  1. 动画驱动方式的改变可能会影响性能,建议进一步评估其影响。
  2. 布局更新逻辑的优化很好,可以考虑进一步提高其可读性。

总的来说,这些改动是有益的,但建议在合并前仔细考虑性能影响,并可能进行一些小的代码优化。

src/PageContentView.tsx Show resolved Hide resolved
@huhuanming huhuanming requested review from hellohublot and a team October 12, 2024 01:38
@huhuanming huhuanming merged commit 5565421 into main Oct 12, 2024
7 checks passed
@huhuanming huhuanming deleted the border-radius branch October 12, 2024 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants