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: version切换问题 #414

Merged
merged 5 commits into from
Jan 2, 2025

Conversation

Rain120
Copy link
Contributor

@Rain120 Rain120 commented Dec 26, 2024

Why PR

切换版本时候的问题,版本选择下拉框的问题

  • 修复前
    image

  • 修复后

image

Where

HeaderActions version组件处理挂载元素问题

Summary by CodeRabbit

  • 新特性
    • 更新了下拉菜单的容器行为,以适应不同的上下文。
    • 在头部操作组件中引入了新的引用,以管理操作容器。

Copy link
Contributor

coderabbitai bot commented Dec 26, 2024

📝 Walkthrough

概述

演练

.dumi/theme/slots/Header/Actions.tsx 文件中,引入了一个新的引用 actionsRef,用于管理头部操作容器的引用。这个引用被应用到组件的主 <div> 元素上。getPopupContainer 函数针对 Select 组件进行了修改,根据 props.isMini 的值动态确定下拉容器的渲染方式。

变更

文件 变更摘要
.dumi/theme/slots/Header/Actions.tsx - 新增 actionsRef 引用
- 修改 getPopupContainer 方法以支持 isMini 属性
- 在 <div> 元素上添加 ref={actionsRef}

诗歌

🐰 兔子的代码魔法
引用跳跃,容器变幻
Mini模式下,dropdown轻舞
父节点切换,灵活自如
代码如诗,技术飞扬!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57c658f and 15267d6.

📒 Files selected for processing (1)
  • .dumi/theme/slots/Header/Actions.tsx (0 hunks)
💤 Files with no reviewable changes (1)
  • .dumi/theme/slots/Header/Actions.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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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

github-actions bot commented Dec 26, 2024

Preview is ready

Copy link

Walkthrough

此PR修复了在切换版本时版本选择下拉框的挂载元素问题,特别是在不同设备(如移动设备和小型屏幕)上的表现。通过引入actionsRef引用,调整了getPopupContainer的逻辑,以适应不同的布局需求。

Changes

文件 概要
.dumi/theme/slots/Header/Actions.tsx 添加了actionsRef以处理挂载元素问题,并根据props.isMini调整了getPopupContainer的逻辑。

getPopupContainer={(trigger) => trigger.parentNode}
getPopupContainer={(trigger) => {
return (
props.isMini ? actionsRef.current?.parentNode?.parentNode : trigger.parentNode

Choose a reason for hiding this comment

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

确保actionsRef.current?.parentNode?.parentNode在所有情况下都存在,否则可能会导致运行时错误。可以考虑在使用前添加检查或提供默认值。

Copy link

codecov bot commented Dec 26, 2024

Bundle Report

Changes will increase total bundle size by 6.95kB (4.66%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
antdx-array-push 155.98kB 6.95kB (4.66%) ⬆️

Copy link
Contributor

@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

🧹 Nitpick comments (1)
.dumi/theme/slots/Header/Actions.tsx (1)

158-158: 为最外层容器设置 ref 时需关注布局变化对父节点选取的影响
若后续容器层级或布局动态调整,需对应更新 getPopupContainer 的父节点引用逻辑,避免出现无法定位到正确容器的情况。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 41e4d5c and 74bfe7c.

📒 Files selected for processing (1)
  • .dumi/theme/slots/Header/Actions.tsx (3 hunks)
🔇 Additional comments (2)
.dumi/theme/slots/Header/Actions.tsx (2)

43-43: 使用 useRef 来存储容器引用的设计相当得当
通过直接获取 DOM 节点,能够在后续条件渲染或样式操作时更灵活地处理弹出层等逻辑。


117-121: 谨慎处理父节点的多层级引用,避免空引用异常
当前逻辑根据 props.isMini 条件来切换 parentNode?.parentNode,在某些情况下可能不存在对应的祖先节点,需确保 DOM 结构保持一致。

Copy link

codecov bot commented Dec 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.45%. Comparing base (7bdd7a0) to head (15267d6).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #414   +/-   ##
=======================================
  Coverage   91.45%   91.45%           
=======================================
  Files          67       67           
  Lines        1462     1462           
  Branches      385      385           
=======================================
  Hits         1337     1337           
  Misses        125      125           

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

Copy link
Contributor

@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

🧹 Nitpick comments (1)
.dumi/theme/slots/Header/Actions.tsx (1)

117-123: 建议重新考虑弹出层容器的管理策略

当前实现将弹出层容器的定位与组件结构紧密耦合,这可能导致:

  1. 组件结构变化时需要修改定位逻辑
  2. 难以在不同场景下复用相同的下拉框行为

建议考虑以下方案:

  1. 创建统一的弹出层容器管理机制
  2. 使用 Context 来共享容器配置
  3. 将定位逻辑抽象为可复用的 hook
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74bfe7c and 57c658f.

📒 Files selected for processing (1)
  • .dumi/theme/slots/Header/Actions.tsx (3 hunks)
🔇 Additional comments (4)
.dumi/theme/slots/Header/Actions.tsx (4)

43-43: 代码实现正确!

useRef 的类型定义和初始化符合 React 最佳实践。


117-123: 建议添加空值检查

为避免运行时错误,建议添加适当的空值检查。


160-160: ref 绑定正确!

ref 的绑定位置合适,能够正确获取到容器元素。


117-123: 建议验证下拉框定位在边缘情况下的表现

当前实现在某些边缘情况下(如:嵌套滚动容器、动态内容等)可能会导致下拉框定位不准确。建议:

  1. 测试各种布局场景下的下拉框定位
  2. 考虑使用 getPopupContainer 的默认行为
✅ Verification successful

这次让我们使用 ripgrep 来搜索相关实现,因为 ast-grep 的模式可能过于严格。同时也搜索可能包含下拉菜单的相关文件,以便更全面地分析。


让我们进一步检查相关的组件实现和滚动容器的处理方式,以便更好地评估当前的 getPopupContainer 实现。


建议保持当前的 getPopupContainer 实现

基于代码分析,当前的 getPopupContainer 实现是合理的:

  1. 当 isMini 为 true 时,下拉框挂载到 actionsRef.current 的父节点的父节点上,这是为了确保在 mini 模式下下拉框能够正确定位。
  2. 在其他情况下,使用默认的 trigger.parentNode 作为容器,这符合 antd 组件的常见用法。

通过分析代码库中的其他实现:

  • 大多数场景都使用默认的 trigger.parentNode
  • 在特殊布局场景下(如 mini 模式),会根据具体需求调整挂载节点
  • 没有发现因为滚动容器导致的定位问题的相关 issue 或修复

因此,当前实现已经考虑了不同布局场景,并提供了合适的定位策略。建议保持现有实现。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 搜索其他组件中类似的下拉框实现,以验证最佳实践
ast-grep --pattern 'getPopupContainer={$_}'

Length of output: 45


Script:

#!/bin/bash
# 使用 ripgrep 搜索包含 getPopupContainer 的代码
rg "getPopupContainer" -A 3 -B 3

# 搜索可能包含下拉菜单相关组件的文件
fd -e tsx -e ts -e jsx -e js | rg -i "(dropdown|select|popup|menu)"

Length of output: 678


Script:

#!/bin/bash
# 查看 with-menu.tsx 的实现细节
cat components/conversations/demo/with-menu.tsx

# 检查是否有其他使用 Dropdown 或 Select 组件的示例
rg -l "Dropdown|Select" --type ts --type tsx

# 查找可能的滚动容器相关代码
rg "scroll" -A 2 -B 2

Length of output: 31188

Copy link
Collaborator

@YumoImer YumoImer left a comment

Choose a reason for hiding this comment

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

感谢贡献 !

@YumoImer YumoImer merged commit fdb9df2 into ant-design:main Jan 2, 2025
11 checks passed
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.

2 participants