Skip to content

Conversation

@AmyFoxFN
Copy link
Contributor

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow DiDi's contributing guide.
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.

@codecov-io
Copy link

codecov-io commented Dec 12, 2017

Codecov Report

Merging #45 into dev will decrease coverage by 0.27%.
The diff coverage is 80.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #45      +/-   ##
==========================================
- Coverage   88.94%   88.66%   -0.28%     
==========================================
  Files          45       49       +4     
  Lines         841      891      +50     
  Branches      113      121       +8     
==========================================
+ Hits          748      790      +42     
- Misses         49       52       +3     
- Partials       44       49       +5
Impacted Files Coverage Δ
example/data/cascade.js 100% <100%> (ø)
src/components/picker/picker.vue 84.31% <100%> (+1.83%) ⬆️
src/modules/cascade-picker/index.js 100% <100%> (ø)
src/modules/cascade-picker/api.js 50% <50%> (ø)
src/components/cascade-picker/cascade-picker.vue 76.47% <76.47%> (ø)
src/components/time-picker/time-picker.vue 76.82% <0%> (+0.66%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a8c24e...382228d. Read the comment docs.

Copy link
Contributor

@dolymood dolymood left a comment

Choose a reason for hiding this comment

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

Nice 👍

createAPI(Vue, DatePicker, ['select', 'cancel'], false)
const linkageData = [
Copy link
Contributor

Choose a reason for hiding this comment

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

数据放 data/ 中把

return [provinces, cities, areas]
}
return {}
Copy link
Contributor

Choose a reason for hiding this comment

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

没有 data 的话 整个 data() 可以删除了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍get~

<cube-picker
ref="picker"
:title="title"
:data="pickerData"
Copy link
Contributor

Choose a reason for hiding this comment

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

pickerData 初始放在 data 中

return {
linkageData: this.data.slice(),
pickerSelectedIndex: this.selectedIndex.slice(),
tempIndex: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

如果变量不需要 watch 或者 computed 或者当做 prop 的话,可以直接写在 this 上,可以在 created 中初始化,没必要放在 data 中,data 中之存放 observe 数据。这里的 tempIndex 和 changeI 都可以移除

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍get~

}
},
computed: {
depth() {
Copy link
Contributor

Choose a reason for hiding this comment

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

depth 可以不需要,看下边的 updatePickerData 那里

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍get~

updatePickerData(init) {
const pickerData = []
let data = this.linkageData
for (let i = 0; i < this.depth; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这种树状结构的数据重新梳理的话,使用递归即可,也可以直接使用循环,但是判断条件是判断 data 还有没有children。

Copy link
Contributor

Choose a reason for hiding this comment

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

另,这个对数据结构的梳理可以重新加一个函数梳理

if (!init) {
this.$refs.picker.setData(this.pickerData, this.tempIndex)
this.$refs.picker.refresh()
for (let j = this.changeI + 1; j < this.depth; j++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这部分不需要吧循环不需要吧,已经setData了,应该不需要重新调用picker 的 scrollTo了吧;如果不是的话,那可以考虑进一步修改 picker 的逻辑,使其适应性更强?
还有一个问题是 如果是没show的情况下调用refresh 会有问题吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

现在在show的时候,如果不scrollTo是没有滚动相应tempIndex的~嗯,明天看一下picker的逻辑~~如果是没show的情况下调用refresh 会有问题,所以我在init的时候不会执行这段

@dolymood
Copy link
Contributor

Need unit tests.

@AmyFoxFN AmyFoxFN changed the title Add Linkage picker component Add CascadePicker component Dec 13, 2017
@dolymood dolymood merged commit c3cbe43 into dev Dec 14, 2017
@AmyFoxFN AmyFoxFN deleted the linkage-picker branch December 15, 2017 09:37
hu0950 pushed a commit that referenced this pull request Nov 1, 2019
* CascadePicker init

* add tests
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.

4 participants