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: RangePicker allowClear does not disable the clear button #44015

Merged
merged 6 commits into from Aug 4, 2023

Conversation

bartpio
Copy link
Contributor

@bartpio bartpio commented Aug 3, 2023

[中文版模板 / Chinese template]

🤔 This is a ...

  • 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
  • Other (about what?)

🔗 Related issue link

Fix #44012

💡 Background and solution

A regression was introduced in 24697bb that results in generatePicker and generateRangePicker building the final allowClear prop solely from the clearIcon prop, without respecting allowClear===false. This makes it impossible for the library user to make a date picker non-clearable.

The proposed solution uses allowClear, as provided to the date picker, when it is false or an object bearing clearIcon. Otherwise, the final allowClear prop is built from the clearIcon prop.

The proposed solution skips building an icon-bearing allowClear prop, when the allowClear prop on the date picker is set to false, and the clearIcon prop on the date picker is undefined.

📝 Changelog

Language Changelog
🇺🇸 English Disable DatePicker and RangePicker clear button when allowClear prop is false
🇨🇳 Chinese 当allowClear属性为false时禁用DatePicker和RangePicker清除按钮

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

🚀 Summary

🤖 Generated by Copilot at 025ada1

The pull request adds a new feature to the DatePicker and RangePicker components that allows users to customize the clear icon. The pull request also improves the testing of these components by using new modules, components, and utilities. The pull request modifies the files components/date-picker/generatePicker/generateRangePicker.tsx, components/date-picker/generatePicker/generateSinglePicker.tsx, components/date-picker/util.ts, components/date-picker/__tests__/DatePicker.test.tsx, components/date-picker/__tests__/RangePicker.test.tsx, and components/date-picker/__tests__/utils.ts.

🔍 Walkthrough

🤖 Generated by Copilot at 025ada1

  • Add the allowClear prop to the DatePicker and RangePicker components, which can be a boolean or an object that specifies the clearIcon (link, link, link, link, link)
  • Import the mergeAllowClear function from the util.ts module in the generateSinglePicker.tsx and generateRangePicker.tsx modules, which handles the logic of the allowClear prop (link, link, link)
  • Add test cases for the allowClear prop in the DatePicker.test.tsx and RangePicker.test.tsx modules, which verify the existence and appearance of the clear icon (link, link)
  • Import new modules and components for testing, such as userEvent, screen, waitFor, and CloseCircleFilled in the DatePicker.test.tsx and RangePicker.test.tsx modules, and use them in the new test cases (link, link, link)
  • Add utility functions for querying and asserting the clear icon element in the utils.ts module, and use them in the new test cases (link)

@stackblitz
Copy link

stackblitz bot commented Aug 3, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

@bartpio bartpio force-pushed the date-picker-allow-clear-false branch 2 times, most recently from fd7e757 to 70d79af Compare August 3, 2023 19:49
@argos-ci
Copy link

argos-ci bot commented Aug 3, 2023

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 1 change Aug 4, 2023, 7:39 AM

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (991dbe6) 100.00% compared to head (853089c) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #44015   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          658       658           
  Lines        11178     11186    +8     
  Branches      3027      3029    +2     
=========================================
+ Hits         11178     11186    +8     
Files Changed Coverage Δ
...date-picker/generatePicker/generateRangePicker.tsx 100.00% <ø> (ø)
...ate-picker/generatePicker/generateSinglePicker.tsx 100.00% <ø> (ø)
components/date-picker/__tests__/utils.ts 100.00% <100.00%> (ø)
components/date-picker/util.ts 100.00% <100.00%> (ø)

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

@bartpio bartpio force-pushed the date-picker-allow-clear-false branch 2 times, most recently from d50dc55 to c7f5643 Compare August 3, 2023 21:51
@bartpio bartpio force-pushed the date-picker-allow-clear-false branch 2 times, most recently from bae3fa7 to 025ada1 Compare August 4, 2023 05:20
@bartpio

This comment was marked as resolved.

@bartpio bartpio force-pushed the date-picker-allow-clear-false branch from 025ada1 to 8c66709 Compare August 4, 2023 07:10
@bartpio bartpio force-pushed the date-picker-allow-clear-false branch from 8c66709 to 853089c Compare August 4, 2023 07:27
@bartpio

This comment was marked as resolved.

@kiner-tang
Copy link
Member

@kiner-tang We are getting a screenshot discrepancy now. It looks like the newer screenshot is of a larger viewport. Any suggestion on how to address?

It doesn't matter! That maybe a bug of argos!

@kiner-tang kiner-tang merged commit fb00f1c into ant-design:master Aug 4, 2023
52 checks passed
@PeachScript PeachScript mentioned this pull request Aug 11, 2023
20 tasks
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.

RangePicker allowClear does not disable the clear button
2 participants