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: 修复 Image 组件开启 lazy 后在 fixed 定位容器下检查交叉异常的问题 #820

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

KMS-Bismarck
Copy link
Collaborator

@KMS-Bismarck KMS-Bismarck commented Nov 27, 2024

Types of changes

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Others

Changelog

  • 修复 Image 组件开启 lazy 后在 fixed 定位容器下检查交叉异常的问题

Playground id

21b3032e-6f51-418b-97ad-a0e02d2da125

Other information

Summary by CodeRabbit

  • 新功能

    • 引入了 getClosestFixedContainer 函数,优化懒加载机制,提升交叉观察器的准确性。
    • Image 组件新增 inViewOnly 功能,允许仅在视口内加载图像。
  • 错误修复

    • 修复了 Image 组件在启用懒加载时与固定定位容器的交叉错误。
  • 版本更新

    • 版本号更新至 3.5.2-beta.10

@KMS-Bismarck KMS-Bismarck added bug Something isn't working playground 在 Playground 有示例 labels Nov 27, 2024
Copy link

coderabbitai bot commented Nov 27, 2024

Walkthrough

此次更改在packages/hooks/src/utils/dom/element.tsx模块中引入了一个新函数getClosestFixedContainer,该函数用于遍历给定元素的父元素,以查找最近的具有CSS位置为'fixed'的祖先元素。此外,getClosestScrollContainer函数经过修改以提高代码可读性,但其逻辑和功能保持不变。在packages/hooks/src/utils/lazyload.ts文件中,更新了getObserver函数,以便使用getClosestFixedContainer来设置IntersectionObserver的根元素,从而优化懒加载机制的功能。同时,package.json文件的版本号也进行了更新。

Changes

文件路径 更改摘要
packages/hooks/src/utils/dom/element.tsx 添加了新函数getClosestFixedContainer,用于查找最近的固定定位祖先元素;修改了getClosestScrollContainer以提高可读性。
packages/hooks/src/utils/lazyload.ts 更新了getObserver函数的参数结构,添加了element属性,并引入getClosestFixedContainer以优化懒加载观察者的根元素。
package.json 更新了版本号,从"3.5.2-beta.9"更改为"3.5.2-beta.10"
packages/shineout/src/image/doc/changelog.cn.md 更新了Image组件的变更日志,记录了新版本的发布及相关修复。

Possibly related PRs

Poem

在兔子洞里跳跃欢,
新函数来助懒加载,
固定容器找得快,
代码清晰如春花。
兔子乐在新变化,
一起欢庆新乐章! 🐇✨


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.

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/hooks/src/utils/lazyload.ts (1)

Line range hint 74-91: 建议考虑增强容器管理机制

当前的实现解决了固定定位容器的问题,但从长远来看,建议考虑以下优化方向:

  1. 实现容器类型的优先级管理系统
  2. 添加容器变更的监听机制
  3. 考虑缓存常用容器的引用以提升性能
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 702b1ff and fba2bfe.

📒 Files selected for processing (2)
  • packages/hooks/src/utils/dom/element.tsx (2 hunks)
  • packages/hooks/src/utils/lazyload.ts (3 hunks)
🔇 Additional comments (4)
packages/hooks/src/utils/lazyload.ts (2)

2-2: 导入新的工具函数以支持固定定位容器的检测

新增的 getClosestFixedContainer 导入符合修复目标,用于解决固定定位容器中的懒加载检测问题。


74-75: 优化 IntersectionObserver 的根元素设置逻辑

实现方案合理,通过 getClosestFixedContainer 正确处理了固定定位容器的场景。建议验证容器优先级是否符合预期。

Also applies to: 88-88

✅ Verification successful

Let me check the implementation of getClosestFixedContainer and the container precedence logic.


Let me check how the fixedContainer is used in the IntersectionObserver configuration.


IntersectionObserver 的容器优先级设置正确

根据代码实现,容器的优先级逻辑完全符合预期:

  • getClosestFixedContainer 正确实现了对固定定位容器的查找
  • 在 IntersectionObserver 配置中,fixedContainer 优先于 container 使用
  • 当两者都不存在时,默认使用视口作为根容器
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否存在其他使用 IntersectionObserver 的相关代码
rg -l "IntersectionObserver" --type ts

# 检查是否有相关测试用例
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts

Length of output: 3543


Script:

#!/bin/bash
# 检查 getClosestFixedContainer 的实现
ast-grep --pattern 'function getClosestFixedContainer($_) { $$$ }'

# 检查 lazyload.ts 中的容器优先级逻辑
rg -A 10 "const \{ container = null, element" packages/hooks/src/utils/lazyload.ts

Length of output: 2006


Script:

#!/bin/bash
# 检查 IntersectionObserver 的配置
rg -A 15 "new IntersectionObserver\(" packages/hooks/src/utils/lazyload.ts

Length of output: 467

packages/hooks/src/utils/dom/element.tsx (2)

116-122: 代码格式调整提高了可读性

返回语句的重新格式化使条件判断更清晰易读。


143-164: 新增的 getClosestFixedContainer 函数实现合理

函数实现符合以下要点:

  • 与现有的 getClosestScrollContainer 保持一致的代码风格
  • 正确处理了空值情况
  • 实现简洁且职责单一

建议考虑以下优化:

 export function getClosestFixedContainer(element: Element | HTMLElement | null) {
   if (!element) {
     return null;
   }
 
   const isFixable = (el: HTMLElement) => {
     const style = window.getComputedStyle(el);
-    return style.position === 'fixed';
+    // 缓存计算样式以提高性能
+    const { position } = style;
+    return position === 'fixed';
   };
 
   // 遍历父元素
   let parent = element.parentElement;
 
   while (parent) {
     if (isFixable(parent)) {
       return parent;
     }
     parent = parent.parentElement;
   }
 
   return null;
 }

另外,建议验证此函数在各种 DOM 结构下的行为是否符合预期。

✅ Verification successful

Let me check the CSS file to understand the usage context better.


Let me check the usage context in lazyload.ts to understand how the function is being used.


getClosestFixedContainer 函数实现合理且使用场景明确

通过代码分析,该函数在 lazyload.ts 中被用于 IntersectionObserver 的配置中,用于确定正确的滚动容器。实现符合以下要点:

  • 正确处理了 fixed 定位元素作为 IntersectionObserver 的 root 容器
  • 与现有代码风格保持一致
  • 实现简洁且职责单一

关于性能优化的建议可以保留,但不是必需的,因为:

  • 该函数仅在初始化 IntersectionObserver 时调用一次
  • 遍历的层级通常较少
  • getComputedStyle 的性能开销在此场景下可以接受
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 搜索代码库中可能的边界情况
rg -l 'position:\s*fixed' --type css

Length of output: 79


Script:

#!/bin/bash
# Check the content of the CSS file
cat packages/ssr-test/styles/Home.module.css

# Also search for any tests or usage of getClosestFixedContainer
rg "getClosestFixedContainer" -A 3

Length of output: 4824


Script:

#!/bin/bash
# Check the implementation context in lazyload.ts
cat packages/hooks/src/utils/lazyload.ts

# Also check for any tests
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts lazyload

Length of output: 3307

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/shineout/src/image/__doc__/changelog.cn.md (1)

4-6: Bug修复描述完整且清晰

修复描述准确反映了PR的目标,并正确链接到相关PR #820。建议补充一下具体的使用场景,以帮助用户更好地理解此修复的适用情况。

建议将描述扩展为:

- 修复 `Image` 组件开启 `lazy` 后在 fixed 定位容器下检查交叉异常的问题
+ 修复 `Image` 组件开启 `lazy` 后,当图片位于 position: fixed 定位的容器内时,无法正确触发懒加载的问题
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between fba2bfe and 7442217.

📒 Files selected for processing (2)
  • package.json (1 hunks)
  • packages/shineout/src/image/__doc__/changelog.cn.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • package.json
🔇 Additional comments (2)
packages/shineout/src/image/__doc__/changelog.cn.md (2)

1-3: 版本号格式规范且日期记录准确

版本号和发布日期的格式符合项目规范。


1-6: 变更日志与PR目标保持一致

变更记录完整地体现了PR的修复目标,包含了受影响的组件、功能特性和具体问题描述。

@KMS-Bismarck KMS-Bismarck merged commit 3206bd7 into main Nov 27, 2024
1 check passed
@KMS-Bismarck KMS-Bismarck deleted the fix-bug-image-lazy branch November 27, 2024 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working playground 在 Playground 有示例
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant