Skip to content

fix(Picker): should reset subsequent columns when a column is changed#13366

Open
JerryWu1234 wants to merge 1 commit intoyouzan:mainfrom
JerryWu1234:main
Open

fix(Picker): should reset subsequent columns when a column is changed#13366
JerryWu1234 wants to merge 1 commit intoyouzan:mainfrom
JerryWu1234:main

Conversation

@JerryWu1234
Copy link
Copy Markdown

@JerryWu1234 JerryWu1234 commented Feb 26, 2025

… is changed somehow

fixes: #13365

add a new property for resetChildren

Before submitting a pull request, please read the contributing guide.

在提交 pull request 之前,请阅读 贡献指南

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.81%. Comparing base (ec5b45b) to head (5d4d536).
⚠️ Report is 177 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13366      +/-   ##
==========================================
+ Coverage   89.60%   89.81%   +0.20%     
==========================================
  Files         257      257              
  Lines        7013     7048      +35     
  Branches     1736     1746      +10     
==========================================
+ Hits         6284     6330      +46     
+ Misses        384      379       -5     
+ Partials      345      339       -6     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JerryWu1234
Copy link
Copy Markdown
Author

@inottn Please help to review. I done and fixed all fail tests .

Comment thread packages/vant/src/picker/Picker.tsx Outdated
@inottn
Copy link
Copy Markdown
Collaborator

inottn commented Feb 27, 2025

I don't know if the previous interactions were intentional; if there is an issue, the onChange logic might need to be modified. If I have time, I will see how to adjust it.

@JerryWu1234
Copy link
Copy Markdown
Author

JerryWu1234 commented Feb 27, 2025

I don't know if the previous interactions were intentional; if there is an issue, the onChange logic might need to be modified. If I have time, I will see how to adjust it.

I definitely sure that it wasn't intentional. and it is a bug.
I would like do it if you are busy.

@JerryWu1234
Copy link
Copy Markdown
Author

JerryWu1234 commented Mar 1, 2025

thanks for point out.
now no Area component error, and added edge case.
please help to review it again
@inottn

Comment thread packages/vant/src/picker/Picker.tsx Outdated
Comment thread packages/vant/src/picker/test/index.spec.tsx Outdated
Comment thread packages/vant/src/picker/test/index.spec.tsx Outdated
Comment thread packages/vant/src/picker/test/index.spec.tsx Outdated
Comment thread packages/vant/src/picker/Picker.tsx Outdated
@chenjiahan
Copy link
Copy Markdown
Member

I prefer to keep the current behavior. If we reset the option it will break the user experience of date picker or time picker.

@JerryWu1234
Copy link
Copy Markdown
Author

I prefer to keep the current behavior. If we reset the option it will break the user experience of date picker or time picker.

ok, if we keep the current behavior.
Is there another workaround to fix it?
@chenjiahan

@chenjiahan
Copy link
Copy Markdown
Member

How about adding a new prop to enable this new behavior? Such as resetChildren.

@JerryWu1234
Copy link
Copy Markdown
Author

How about adding a new prop to enable this new behavior? Such as resetChildren.

I think the default should be false if we add a new prop.

@JerryWu1234
Copy link
Copy Markdown
Author

@chenjiahan @inottn I done.

@JerryWu1234
Copy link
Copy Markdown
Author

@inottn please help to review. because my production needs this feature , thanks

@inottn inottn self-requested a review March 23, 2025 01:42
@inottn
Copy link
Copy Markdown
Collaborator

inottn commented Mar 29, 2025

@inottn please help to review. because my production needs this feature , thanks

Sorry for the delay. Vant is currently community-driven, and PR reviews might not be prompt, I've been very busy lately, and I apologize again. If needed, you can temporarily resolve issues using methods like pnpm patch.

@JerryWu1234
Copy link
Copy Markdown
Author

@inottn please help to review. because my production needs this feature , thanks

Sorry for the delay. Vant is currently community-driven, and PR reviews might not be prompt, I've been very busy lately, and I apologize again. If needed, you can temporarily resolve issues using methods like pnpm patch.

Please don't forget my PR

@lllomh
Copy link
Copy Markdown
Contributor

lllomh commented Sep 17, 2025

@inottn please help to review. because my production needs this feature , thanks

Sorry for the delay. Vant is currently community-driven, and PR reviews might not be prompt, I've been very busy lately, and I apologize again. If needed, you can temporarily resolve issues using methods like pnpm patch.

Please don't forget my PR

@JerryWu1234 You have been forgotten ....

@inottn inottn changed the title fix(Picker): should reset the second argument when the first argument… fix(Picker): should reset subsequent columns when a column is changed Dec 8, 2025
Copy link
Copy Markdown
Collaborator

@inottn inottn left a comment

Choose a reason for hiding this comment

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

参考了其他组件库的设计,Picker 支持两种模式:级联模式和非级联模式。在级联模式下,上下级选项存在关联关系,当上级选项变更时,下级会自动重置为第一项,这种设计是合理且用户体验友好的。而 DatePicker 等组件采用的是非级联模式,因此不会受到这种联动逻辑的影响。

@JerryWu1234
Copy link
Copy Markdown
Author

thanks for your approval

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug Report] picker切换的时候,如果有子元素的时候,子元素都是从0开始的

5 participants