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 customize clear icon #660

Merged
merged 8 commits into from
Jul 26, 2023

Conversation

@vercel
Copy link

vercel bot commented Jul 25, 2023

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 Jul 25, 2023 9:50am

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #660 (1c21d99) into master (c377ca0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 1c21d99 differs from pull request most recent head 0d24fdc. Consider uploading reports for the commit 0d24fdc to get more accurate results

@@           Coverage Diff           @@
##           master     #660   +/-   ##
=======================================
  Coverage   99.04%   99.05%           
=======================================
  Files          55       56    +1     
  Lines        2416     2426   +10     
  Branches      716      720    +4     
=======================================
+ Hits         2393     2403   +10     
  Misses         21       21           
  Partials        2        2           
Files Changed Coverage Δ
src/Picker.tsx 100.00% <100.00%> (ø)
src/RangePicker.tsx 99.27% <100.00%> (+<0.01%) ⬆️
src/utils/dateUtil.ts 100.00% <100.00%> (ø)
src/utils/getClearIcon.tsx 100.00% <100.00%> (ø)
tests/util/commonUtil.tsx 93.10% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 54 to 55
e.preventDefault();
e.stopPropagation();
Copy link
Member

Choose a reason for hiding this comment

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

这里可以去掉吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这是现有逻辑,去掉后不知道会不会导致一些不可意料的问题

clearIcon: (
<span
onMouseDown={(e) => {
e.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

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

preventDefault 有用么?有可能导致用户自定义的 icon 事件失效

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这是现有逻辑,去掉后不知道会不会导致一些不可意料的问题

src/RangePicker.tsx Fixed Show fixed Hide fixed
src/utils/getClearIcon.tsx Outdated Show resolved Hide resolved
@zombieJ zombieJ merged commit 049cbf4 into react-component:master Jul 26, 2023
8 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.

3 participants