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

[new feature] List 考虑错误情况,增加 error-text 属性 #2567 #2568

Merged
merged 7 commits into from Jan 20, 2019

Conversation

Rockergmail
Copy link
Contributor

Fixes #2567

Changes you made in this pull request:

  • props增加errorText
  • data增加error
  • load事件增加参数:handleError

@chenjiahan
Copy link
Member

chenjiahan commented Jan 20, 2019

👍 很棒的功能

提一点小建议,目前 vant 会尽量避免在 i18n 文件中添加新的字段(除非是比较通用的),避免体积膨胀,所以能否把 error-text 的默认值去除,

@codecov-io
Copy link

codecov-io commented Jan 20, 2019

Codecov Report

Merging #2568 into dev will decrease coverage by 0.08%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2568      +/-   ##
==========================================
- Coverage   87.21%   87.13%   -0.09%     
==========================================
  Files         104      104              
  Lines        1893     1896       +3     
  Branches      188      188              
==========================================
+ Hits         1651     1652       +1     
- Misses        211      213       +2     
  Partials       31       31
Impacted Files Coverage Δ
packages/list/index.js 92.59% <33.33%> (-7.41%) ⬇️
packages/card/index.js 100% <0%> (ø) ⬆️
packages/dialog/Dialog.js 100% <0%> (ø) ⬆️
packages/image-preview/index.js 100% <0%> (ø) ⬆️
packages/image-preview/ImagePreview.js 94.04% <0%> (+0.07%) ⬆️

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 e45780d...e2558d2. Read the comment docs.

@Rockergmail
Copy link
Contributor Author

@chenjiahan 谢谢。我先去掉吧。稍等下

@Rockergmail
Copy link
Contributor Author

@chenjiahan done.

@chenjiahan
Copy link
Member

刚想了一下,控制 error 状态可以考虑使用传入 props 配合 sync 修饰符来做

<van-list :error.sync="error" />
export default {
  data() {
    error: false
  },

  methods: {
    onLoad() {
      ajax().catch(() => {
        this.error = true;
      })
    }
  }
}

这样的好处是不依赖传入回调函数,使用者可以更灵活地控制错误状态,你看下是否能完成你的需求~

@Rockergmail
Copy link
Contributor Author

@chenjiahan 请教一下:

  1. .sync的jest测试怎么写?
const wrapper = mount(List, {
     propsData: {
      errorText: 'Request failed. Click to reload...',
      error: true
    },
    listeners: {
      'update:error': ($event) => {
        // 这个如何为error赋值?下面的写法是错的
        this.error = $event;
      }
    }
  });
  1. 为什么mockOffsetParent能够模拟父元素位置的偏移?我看了代码,就是为父元素的dom添加了offsetParent这个响应式变量啊

@chenjiahan
Copy link
Member

chenjiahan commented Jan 20, 2019

  1. 可以用 wrapper.setProps({ error }) 来更新属性
  2. offsetParent 是为了跳过代码里的 offsetParent !== null 这个判断

@Rockergmail
Copy link
Contributor Author

@chenjiahan ci检测说packages/dialog/test/index.spec.js不通过,我没有改过对应的代码

FAIL  packages/dialog/test/index.spec.js
  ● before close
    expect(received).toBeFalsy()
    Received: [[]]
      43 | 
      44 |   cancel.trigger('click');
    > 45 |   expect(wrapper.emitted('cancel')).toBeFalsy();
         |                                     ^
      46 | 
      47 |   wrapper.setProps({
      48 |     beforeClose: (action, done) => done()
      at Object.toBeFalsy (packages/dialog/test/index.spec.js:45:37)

@chenjiahan
Copy link
Member

Sorry 是我的失误,已经修复了

@chenjiahan chenjiahan merged commit c43edf9 into youzan:dev Jan 20, 2019
@chenjiahan
Copy link
Member

Merge 了哈~ 会在下个版本发布

@Rockergmail
Copy link
Contributor Author

@chenjiahan 这是我第一次PR,以后有机会多多指教哈

@chenjiahan
Copy link
Member

哈哈,非常欢迎,期待你的下个 PR~

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.

None yet

3 participants