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

Don't change fileList when beforeUpload returns false #8299

Merged
merged 1 commit into from Nov 29, 2017
Merged

Conversation

yesmeck
Copy link
Member

@yesmeck yesmeck commented Nov 23, 2017

Close #8036

@codecov
Copy link

codecov bot commented Nov 23, 2017

Codecov Report

Merging #8299 into master will increase coverage by 0.23%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #8299      +/-   ##
=========================================
+ Coverage   83.76%     84%   +0.23%     
=========================================
  Files         209     209              
  Lines        4226    4270      +44     
  Branches     1251    1278      +27     
=========================================
+ Hits         3540    3587      +47     
+ Misses        686     683       -3
Impacted Files Coverage Δ
components/upload/Upload.tsx 73.04% <ø> (+1.73%) ⬆️
components/form/FormItem.tsx 98.23% <0%> (+1.4%) ⬆️

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 72741db...b5193e4. Read the comment docs.

this.onChange({
file,
fileList,
fileList: nextFileList,
Copy link
Member

Choose a reason for hiding this comment

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

应该把 fistList 直接剔除掉。

@yesmeck
Copy link
Member Author

yesmeck commented Nov 27, 2017

Updated.

@yesmeck yesmeck force-pushed the fix-upload branch 2 times, most recently from 13e1855 to 8dea36b Compare November 27, 2017 12:49
@yesmeck
Copy link
Member Author

yesmeck commented Nov 27, 2017

这个问题不好解,如果这里 fileList 返回空,那么 #7762 的问题又会出现(见 PR 里的用例)。作为变通, #7762 里给的 codepen 可以改成基于 this.state.fileList 去做验证。建议 revert #7773

@afc163 @nikogu

@yesmeck
Copy link
Member Author

yesmeck commented Nov 27, 2017

想多了,还是可以的。。

@yesmeck yesmeck merged commit 179528a into master Nov 29, 2017
@yesmeck yesmeck deleted the fix-upload branch November 29, 2017 12:28
@kikyousky
Copy link

kikyousky commented Dec 15, 2017

I think this issue is not properly fixed for ver 2.13.11, original fileList was replaced by the unacceptable file in onChange callback

  beforeUpload = (file: UploadFile, fileList: UploadFile[]) => {
    if (!this.props.beforeUpload) {
      return true;
    }
    const result = this.props.beforeUpload(file, fileList);
    if (result === false) {
      this.onChange({
        file,
        fileList,
      }, false);
      return false;
    } else if (result && (result as PromiseLike<any>).then) {
      return result;
    }
    return true;
  }
  onChange = (info: UploadChangeParam, updateState = true) => {
    if (!('fileList' in this.props) && updateState) {
      this.setState({ fileList: info.fileList }); //didn't set state when beforeUpload returns false
    }
    const { onChange } = this.props;
    if (onChange) {
      onChange(info); // the fileList in info only contains the unacceptable file, not the expected current file list , 
    }
  }

@yesmeck
Copy link
Member Author

yesmeck commented Jan 10, 2018

@kikyousky I found this problem too, do you have any advise?

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