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

Fix list item attachments #286

Merged
merged 2 commits into from
May 17, 2019
Merged

Fix list item attachments #286

merged 2 commits into from
May 17, 2019

Conversation

ltdu
Copy link
Contributor

@ltdu ltdu commented Apr 28, 2019

Q A
Bug fix? [x]
New feature? [ ]
New sample? [ ]
Related issues? fixes #282

This fixes #282 by changing how attachment files and previews are loaded. Additionally, it improves attachment related methods in SPService.

There are couple of things to keep in mind:

  • because this changes public API it may require major version increase. SPService.getListItemAttachments() used to return null if no attachments there found. Now it returns []. But this is up to debate as I don't know whether anyone actually uses them directly.
  • I'm pretty sure check for existing attachment didn't work before. Now it does and this control replaces existing file with new version. This may be unexpected to some users. Or not...
  • I don't use this control in any of my projects so if messed up something let me know :)

@codecov-io
Copy link

Codecov Report

Merging #286 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #286   +/-   ##
=======================================
  Coverage   72.36%   72.36%           
=======================================
  Files          17       17           
  Lines         760      760           
  Branches      157      157           
=======================================
  Hits          550      550           
  Misses        163      163           
  Partials       47       47

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 0bd5dce...9671b69. Read the comment docs.

@estruyf estruyf merged commit 9671b69 into pnp:dev May 17, 2019
@estruyf
Copy link
Member

estruyf commented May 17, 2019

Thanks @ltdu - this will be released as a bug fix. As you mention that there wasn't working something before, it might be that there wasn't a lot of usages yet + we will bump the major version soon as we plan to update to Office UI Fabric version 6 + SPFx 1.8.2 or higher.

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