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

Update SDWebImage version to 4.4.3 #387

Merged
merged 1 commit into from
Feb 22, 2019

Conversation

ds8k
Copy link

@ds8k ds8k commented Jan 3, 2019

This updates the version of SDWebImage to 4.4.3, which has fixes for webp images rendering in iOS 12. This fixes #298 #385

@ds8k ds8k changed the title Update submodule commit Update SDWebImage version to 4.4.3 Jan 3, 2019
@obj-p
Copy link

obj-p commented Jan 10, 2019

I will preface this with I could have messed up a step. I made my fork and merged these changes in here: https://github.com/jjgp/react-native-fast-image

Then I did the following to integrate:

$ yarn add https://github.com/jjgp/react-native-fast-image/tarball/master 
$ react-native link react-native-fast-image

And upon building I am seeing. edit within FFFastImageView.h:
screen shot 2019-01-10 at 2 23 04 pm

Just wanted to try out these changes. I appreciate the update!

@obj-p
Copy link

obj-p commented Jan 10, 2019

I'm definitely going to chalk it up as I didn't do integrate it correctly. I was curious how you linked it up to test it?

@ds8k
Copy link
Author

ds8k commented Jan 11, 2019

@jjgp I actually haven't tested it. I could not get it installed and linked properly due to the submodule not initializing correctly

@obj-p
Copy link

obj-p commented Jan 11, 2019

I'll look into it more too. Maybe @DylanVann would have some insight too 👍

@Almouro
Copy link
Contributor

Almouro commented Jan 24, 2019

Hello,

I successfully tested that PR and it fixes the issue of ios12 webp images not being displayed.

To test I did:

cd node_modules
rm -rf react-native-fast-image
git clone ds8k/react-native-fast-image
cd react-native-fast-image
git submodule update --init --recursive

This also seemed to annoy XCode so I did the usual clean/quit XCode/remove DerivedData folder before rebuilding.

@DylanVann any chance we can get this in? :)

@ds8k note that your commit isn't linked to your GitHub profile (your profile picture doesn't appear). In case that's not intended, you should be able to fix it by setting your email in your local git config with git config --global user.email email@example.com 😉

@ds8k
Copy link
Author

ds8k commented Jan 24, 2019

@Almouro That's awesome! Thanks for testing it out

@ds8k
Copy link
Author

ds8k commented Feb 5, 2019

@DylanVann Could this get looked at soon?

@humarkx
Copy link

humarkx commented Feb 7, 2019

This is a must have PR. @DylanVann can you please take a look how to get this update of SDWebImage into play?

Regards

@introin
Copy link

introin commented Feb 14, 2019

+1 Please @DylanVann

@Almouro
Copy link
Contributor

Almouro commented Feb 15, 2019

In case that can help you guys in the meantime, in our project, we decided to remove the Vendors and install SDWebImage as pod, making it way easier to manage SDWebImage dependencies.

To remove the Vendors folder we created a patch with patch-package

Podfile

target 'YOUR_TARGET' do
  ...
  pod 'SDWebImage', '= 4.4.3'
  pod 'SDWebImage/GIF', '= 4.4.3'
  pod 'SDWebImage/WebP', '= 4.4.3'
end

@tjbenton
Copy link

When is this PR going to get merged? I really need this update for my current project

@DylanVann DylanVann changed the base branch from master to update-sdwebimage February 22, 2019 03:50
@DylanVann DylanVann merged commit 1effb60 into DylanVann:update-sdwebimage Feb 22, 2019
@DylanVann
Copy link
Owner

DylanVann commented Feb 22, 2019

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.

7 participants