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: multiple select none current panel should not trigger change #896

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

zombieJ
Copy link
Member

@zombieJ zombieJ commented Nov 25, 2024

fix ant-design/ant-design#51721

Summary by CodeRabbit

  • 新功能

    • 更新了单选器组件,以在多选模式下防止不必要的值更改。
    • 改进了回调函数传递的数据结构,确保更清晰的信息。
  • 测试

    • 为多选器测试套件添加了新测试,验证在选择年份面板时不会触发回调,直到选择特定日期。

Copy link

vercel bot commented Nov 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
picker ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 25, 2024 6:56am

Copy link

coderabbitai bot commented Nov 25, 2024

Walkthrough

本次更改主要集中在 SinglePicker 组件的逻辑更新上,目的是在多选模式下防止不必要的值更改。通过在 onPanelSelect 函数中添加条件检查,以确保在特定条件下提前返回。此外,onInternalPickerValueChange 函数也进行了更新,以清理传递给回调的 info 对象,去除 range 属性。这些更改旨在提高数据结构的清晰度和错误处理的准确性。

Changes

文件路径 更改摘要
src/PickerInput/SinglePicker.tsx 更新 onPanelSelect 函数以防止在多选模式下的值更改;清理 onInternalPickerValueChange 中的 info 对象。
tests/multiple.spec.tsx Picker.Multiple 测试套件中新增测试用例,确保点击年份面板时不会触发 onChangeonCalendarChange 回调。

Assessment against linked issues

Objective Addressed Explanation
当只有选择某天的时候,日期选择框的值才会发生改变。 (#51721)

Possibly related PRs

Poem

🐰 在选择器中跳跃,
年月轻轻摇摆,
点击不再混乱,
清晰值如明灯,
兔子欢快歌唱,
选择只为那一天! 🌟

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint (1.23.1)

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/PickerInput/SinglePicker.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Cannot read config file: /.eslintrc.js
Error: Cannot find module '@umijs/fabric/dist/eslint'
Require stack:

  • /.eslintrc.js
  • /node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  • /node_modules/eslint/lib/cli-engine/cli-engine.js
  • /node_modules/eslint/lib/eslint/eslint.js
  • /node_modules/eslint/lib/eslint/index.js
  • /node_modules/eslint/lib/cli.js
  • /node_modules/eslint/bin/eslint.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Function.resolve (node:internal/modules/helpers:145:19)
    at Object. (/.eslintrc.js:2:21)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)
    at Module._load (node:internal/modules/cjs/loader:1127:12)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Module.require (node:internal/modules/cjs/loader:1339:12)

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

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.46%. Comparing base (d44d7bc) to head (df22ef2).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #896   +/-   ##
=======================================
  Coverage   95.46%   95.46%           
=======================================
  Files          64       64           
  Lines        2732     2734    +2     
  Branches      769      742   -27     
=======================================
+ Hits         2608     2610    +2     
  Misses        121      121           
  Partials        3        3           

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


🚨 Try these New Features:

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)
tests/multiple.spec.tsx (3)

171-174: 建议增加年份面板可见性的断言

在选择年份之前,建议先验证年份面板是否正确显示。

建议添加如下断言:

  fireEvent.click(document.querySelector('.rc-picker-year-btn'));
+ expect(document.querySelector('.rc-picker-year-panel')).toBeTruthy();
  selectCell(1998);

176-180: 建议增加月份面板可见性的断言

类似地,在选择月份之前,建议先验证月份面板是否正确显示。

建议添加如下断言:

  // Select Month
+ expect(document.querySelector('.rc-picker-month-panel')).toBeTruthy();
  selectCell('Oct');

184-188: 验证回调参数的完整性

onCalendarChange 的回调参数验证可以更详细一些,建议检查完整的日期格式。

建议修改为:

  expect(onCalendarChange).toHaveBeenCalledWith(
-   expect.anything(),
-   ['1998-10-23'],
-   expect.anything(),
+   expect.arrayContaining([expect.any(Date)]),
+   ['1998-10-23'],
+   { source: 'date' }
  );
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d44d7bc and df22ef2.

📒 Files selected for processing (2)
  • src/PickerInput/SinglePicker.tsx (1 hunks)
  • tests/multiple.spec.tsx (1 hunks)
🔇 Additional comments (3)
tests/multiple.spec.tsx (1)

159-193: 测试用例完整地覆盖了年份面板选择的场景

测试用例通过以下步骤验证了多选模式下的选择行为:

  1. 打开年份面板并选择年份
  2. 选择月份
  3. 选择具体日期
  4. 确认选择

验证要点:

  • 选择年份和月份时不会触发 onChangeonCalendarChange
  • 只有在选择具体日期时才会触发 onCalendarChange
  • 点击确认按钮后才会触发 onChange

这个测试用例很好地覆盖了 PR #51721 中报告的问题。

src/PickerInput/SinglePicker.tsx (2)

444-447: 优化了多选模式下的面板选择逻辑

这个条件判断很好地解决了多选模式下中间面板(年份/月份)选择触发值变更的问题。通过确保只有在最终面板(与picker类型匹配)选择时才更新值,避免了不必要的状态更新。


444-447: 建议验证信息对象清理的一致性

代码中清理了传递给回调的info对象中的range属性,这是个好的实践。建议验证其他类似回调中是否也需要同样的清理操作,以保持代码一致性。

✅ Verification successful

Let me gather more context about the info object handling patterns.


Let me check for any other similar cleanup patterns in the codebase.


Let me try one final check to understand the full context of info object usage.


清理info对象的操作是合理且一致的

根据代码分析,info对象的清理操作仅在SinglePicker组件中出现,且只在特定场景(将panel模式的数据传递给外部回调时)需要清理range属性。这是一个局部性的数据清理需求,不需要在其他地方实施相同的清理模式。当前的实现是恰当且一致的。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查其他地方是否有类似的info对象处理模式
rg "delete \w+\.range" --type ts

Length of output: 163


Script:

#!/bin/bash
# Search for info object handling patterns in SinglePicker.tsx
rg "info\." -A 5 -B 5 src/PickerInput/SinglePicker.tsx

# Search for similar patterns in other picker-related files
fd "Picker" --type f --exec rg "info\." -A 5 -B 5 {}

Length of output: 1929


Script:

#!/bin/bash
# Search for similar cleanup patterns with object properties
rg "const clean" -A 5 -B 5 --type ts

# Search for BaseInfo type definition
ast-grep --pattern 'type BaseInfo = {
  $$$
}'

Length of output: 782


Script:

#!/bin/bash
# Search for all info object type definitions and interfaces
ast-grep --pattern 'interface $_ {
  $$$
  info: $_
  $$$
}'

# Search for all onPickerValueChange implementations
ast-grep --pattern 'onPickerValueChange($$$) {
  $$$
}'

Length of output: 120

@zombieJ zombieJ merged commit 46ff2c4 into master Nov 25, 2024
12 checks passed
@zombieJ zombieJ deleted the fix-multiple branch November 25, 2024 06:59
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.

DatePicker multiple多选时,选择某年份、某月份时,value都会增加一个选中日期
1 participant