Skip to content

fix: key should be a optional param #514

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Nov 6, 2022
Merged

fix: key should be a optional param #514

merged 8 commits into from
Nov 6, 2022

Conversation

kiner-tang
Copy link
Contributor

  • fix: key should be a optional param

@vercel
Copy link

vercel bot commented Nov 6, 2022 •
edited
Loading

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

Name Status Preview Updated
picker ✅ Ready (Inspect) Visit Preview Nov 6, 2022 at 8:30AM (UTC)

@kiner-tang kiner-tang mentioned this pull request Nov 6, 2022
Merged
@codecov
Copy link

codecov bot commented Nov 6, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (2.x@55f6571). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             2.x     #514   +/-   ##
======================================
  Coverage       ?   99.53%           
======================================
  Files          ?       46           
  Lines          ?     2160           
  Branches       ?      636           
======================================
  Hits           ?     2150           
  Misses         ?        8           
  Partials       ?        2

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

afc163
afc163 reviewed Nov 6, 2022
View reviewed changes
afc163
afc163 reviewed Nov 6, 2022
View reviewed changes
kiner-tang and others added 2 commits November 6, 2022 14:15
@kiner-tang @afc163
Update src/hooks/usePickerInput.ts
ad9a5d8 
Co-authored-by: afc163 <afc163@gmail.com>
@kiner-tang @afc163
Update src/hooks/usePickerInput.ts
675de79 
Co-authored-by: afc163 <afc163@gmail.com>
afc163
afc163 reviewed Nov 6, 2022
View reviewed changes
@afc163 afc163 merged commit 3e0214f into react-component:2.x Nov 6, 2022
@afc163
Copy link
Member

afc163 commented Nov 6, 2022

https://github.com/react-component/picker/releases/tag/v2.7.0 已发，需要去 antd 仓库发个 PR 把 rc-picker 升级上来并确认相关问题已修复。

afc163
afc163 reviewed Nov 6, 2022
View reviewed changes
src/hooks/usePickerInput.ts
onBlur(e);
}
}, 100);
if (currentFocusedKey) {
Copy link
Member

Choose a reason for hiding this comment

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

currentFocusedKey 有可能不存在么？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前只有测试用例中可能不传，我们需要给所有的测试用例都加上吗？

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.

如果不是范围选择框，而是单个的时间选择器的时候，我们也不需要穿currentFocusedKey.
image
所以实际上只有范围选择器因为有多个输入框，才需要传入这个

@kiner-tang
Copy link
Contributor Author

https://github.com/react-component/picker/releases/tag/v2.7.0 已发，需要去 antd 仓库发个 PR 把 rc-picker 升级上来并确认相关问题已修复。

好的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@afc163 afc163 afc163 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kiner-tang @afc163