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

Compatible with non-image format file preview in UploadList #9621

Merged
merged 2 commits into from Mar 11, 2018

Conversation

@zswang
Copy link
Contributor

@zswang zswang commented Mar 11, 2018

Before

snip20180311_3

After

snip20180310_1

Checklist

  • Run npm run lint and fix those errors before submitting in order to keep consistent code style.
  • Add some descriptions and refer relative issues for you PR.

isNewFeature

  • Add unit tests for the feature.
@codecov
Copy link

@codecov codecov bot commented Mar 11, 2018

Codecov Report

Merging #9621 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9621      +/-   ##
==========================================
+ Coverage   86.01%   86.02%   +0.01%     
==========================================
  Files         195      195              
  Lines        4676     4680       +4     
  Branches     1305     1308       +3     
==========================================
+ Hits         4022     4026       +4     
  Misses        651      651              
  Partials        3        3
Impacted Files Coverage Δ
components/upload/UploadList.tsx 87.95% <100%> (+0.61%) ⬆️

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 ed70ba6...ef87ff8. Read the comment docs.

@afc163
Copy link
Member

@afc163 afc163 commented Mar 11, 2018

Great job! @zswang

@afc163 afc163 merged commit decb6d8 into ant-design:master Mar 11, 2018
4 checks passed
4 checks passed
codecov/patch 100% of diff hit (target 86.01%)
Details
codecov/project 86.02% (+0.01%) compared to ed70ba6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk No new issues
Details
@afc163 afc163 mentioned this pull request Mar 11, 2018
@lee813
Copy link

@lee813 lee813 commented Mar 12, 2018

@zswang Sorry to report, but this could cause some problems. For example, the image url I use is something like: /api/Resources/Image/1/Preview, so your regex cannot detect it, and automatically recognize it as a file. Maybe we can add a switch to turn on this feature or disable it.

Thanks.

@zswang
Copy link
Contributor Author

@zswang zswang commented Mar 13, 2018

@lee813

Have try

/api/Resources/Image/1/Preview?1.png

or

/api/Resources/Image/1/Preview#1.png

Thanks

@lee813
Copy link

@lee813 lee813 commented Mar 13, 2018

@zswang So you suggest me to modify the api response? what if I can't modify the api service?

@yutingzhao1991
Copy link
Contributor

@yutingzhao1991 yutingzhao1991 commented Mar 19, 2018

@lee813 not need to modify the api response, Just modify your image url. append #1.png or ?1.png to your url.


But... Maybe should add a property for this.

@eaTong
Copy link

@eaTong eaTong commented Mar 26, 2018

It's really a bad idea to change the url , how about make function isImageUrl configurable in each file of child .
By default , one file is a image ,and if any one wants to show as an object , just pass isImageUrl in every file , bool and function should both be supported.

@lee813
Copy link

@lee813 lee813 commented Mar 26, 2018

@afc163 Can we improve this feature? We really think that modify URL is a bad idea. Thanks.

@yutingzhao1991 yutingzhao1991 mentioned this pull request Mar 28, 2018
1 of 1 task complete
@dengfuping dengfuping mentioned this pull request Mar 30, 2018
1 of 1 task complete
afc163 added a commit that referenced this pull request Apr 4, 2018
now we need to use `file.isNotImage` to indicate the non-image files in uploadList

close #9835

close #9681

ref #9621
afc163 added a commit that referenced this pull request Apr 4, 2018
now we treat url following below rules:

1. `data:image..` => image
2. `http://xxx.com/xxxx.(webp|svg|png|gif|jpg|jpeg)` => image
3. `http://xxx.com/xxx.zip` other extensions => non-image
4. `data:application..` other minetypes in base64 text => non-image
5. `http://xxx.com/xxx` without any extensions => image

close #9835

close #9681

ref #9621
afc163 added a commit that referenced this pull request Apr 4, 2018
now we treat url following below rules:

1. `data:image..` => image
2. `http://xxx.com/xxxx.(webp|svg|png|gif|jpg|jpeg)` => image
3. `http://xxx.com/xxx.zip` other extensions => non-image
4. `data:application..` other minetypes in base64 text => non-image
5. `http://xxx.com/xxx` without any extensions => image

close #9835

close #9681

ref #9621
@afc163
Copy link
Member

@afc163 afc163 commented Apr 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants