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(PickerGroup): support "confirm" method, support "showToolbar" props #12760

Closed

Conversation

bluesky335
Copy link
Contributor

issue:#12711
为 PickerGroup 组件添加了 confirm 方法
为 PickerGroup 组件添加了 showToolbar 属性

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 56.25000% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 89.75%. Comparing base (103b0ba) to head (ddd2c84).

Files Patch % Lines
packages/vant/src/picker-group/PickerGroup.tsx 53.33% 5 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12760      +/-   ##
==========================================
- Coverage   89.76%   89.75%   -0.02%     
==========================================
  Files         257      257              
  Lines        6850     6861      +11     
  Branches     1663     1664       +1     
==========================================
+ Hits         6149     6158       +9     
- Misses        375      376       +1     
- Partials      326      327       +1     

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

@chenjiahan
Copy link
Member

PickerGroup 真的需要 confirm 方法嘛? 是不是直接调用 Picker 的 confirm 就足够了

@bluesky335
Copy link
Contributor Author

PickerGroup 真的需要 confirm 方法嘛? 是不是直接调用 Picker 的 confirm 就足够了

主要是 PickerGroup 自带 Toolbar的样式有时候并不符合UI设计的样式要求,我在使用是遇到了需要隐藏 toolbar 并将确定按钮放到下面的情况,此时 我发现它并没有 confirm 方法,如果调用 picker 的 confirm 方法的话需要自己实现一遍“下一步”的逻辑,而且不能使用 PickerGroup 的 confirm 事件了。

@chenjiahan
Copy link
Member

了解了,这个场景还是比较特殊的。

如果调用 picker 的 confirm 方法的话需要自己实现一遍“下一步”的逻辑

在隐藏 toolbar 的情况下,本身就需要自行处理“下一步”的逻辑,这个应该也比较方便把,直接修改 v-model:active-tab 的值就可以

@bluesky335
Copy link
Contributor Author

好吧,可能这并不是一个很通用的功能,不过我觉得提供一个隐藏 toolbar 的 props 总是比调整样式来的优雅一些
如果没有必要的话,那这个 pr 感觉可以关闭了

@chenjiahan
Copy link
Member

提供一个隐藏 toolbar 的 props

这个需求没问题,可以单独提 PR 哈

@chenjiahan
Copy link
Member

不建议把不同的改动内容混在一个 PR 里面,这样影响 code review / merge

@inottn
Copy link
Collaborator

inottn commented May 1, 2024

resolved by #12839

@inottn inottn closed this May 1, 2024
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.

4 participants