-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Feature/preload callbacks #437
Conversation
# Conflicts: # android/src/main/java/com/dylanvann/fastimage/FastImagePreloaderModule.java # src/index.d.ts # src/index.js
…preload-callbacks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename FastImagePreloaderModule
back to FastImageViewModule
and extract method into new class instead from preload()
import com.facebook.react.bridge.ReactApplicationContext; | ||
import com.facebook.react.bridge.ReactContextBaseJavaModule; | ||
import com.facebook.react.bridge.ReactMethod; | ||
import com.facebook.react.bridge.ReadableArray; | ||
import com.facebook.react.bridge.ReadableMap; | ||
import com.facebook.react.views.imagehelper.ImageSource; | ||
|
||
class FastImageViewModule extends ReactContextBaseJavaModule { | ||
class FastImagePreloaderModule extends ReactContextBaseJavaModule { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why rename class just like that. I have #425 which edit FastImageViewModule.java
Codecov Report
@@ Coverage Diff @@
## main #437 +/- ##
===========================================
- Coverage 96.29% 51.28% -45.02%
===========================================
Files 1 2 +1
Lines 27 39 +12
Branches 2 0 -2
===========================================
- Hits 26 20 -6
- Misses 1 19 +18
Continue to review full report at Codecov.
|
Hey all! Is this going somewhere? |
Is this PR still being worked on? Would be such a nice feature to have. |
There are some conflicts, I think you should resolve them @Mickagd before @DylanVann can take a look at this PR ^^ |
Please consider implementing this! It would be so useful! |
Update from original repository
Really happy to see this still moving forward! I can't wait to remove my component hack 😄 |
Hello. Any update on this? |
@LuisRodriguezLD As far as I can see there are conflicts. |
I've reimplemented this. Confirmed to work on iOS. Android I'll check later. I'll make a PR when I'm done. On a sidenote, my solution uses a lot less code so I'm hoping that makes it easier to get it merged. |
Hi @RWOverdijk. Thanks for your work! Could you publish your reimplementation on your fork that we can help to test it and speed up this to get it merged? We really need this to be compatible with RN v0.60+. |
@Elindorath Sure thing. I have to finish android first though. The module is compatible with 0.60+ for me, so I'm not sure what you mean |
Sync with the original repo
Hi @RWOverdijk , how is it going with Android ? :) Need any help ? |
Promise should also return which pictures from array couldn't be fetched, not only how many couldn't be fetched. |
@joan-saum It's going all right. The biggest problem right now is time and priorities. I can push the iOS stuff at some point this or next week though, maybe someone else can then PR the android stuff to my branch and we can get this thing going! Sorry for my lack of time... It bothers me as well. |
Please, consider to also add id to every image, so we can check from react-native side which images failed to load. |
@sebinq I will not be doing that but you could add that after if you want. |
@RWOverdijk thanks for info,, I will try. |
The problem is that my project isn't selling well enough to prioritize this feature (My sales strategy was well calculated but man, am I bad at math.). |
…allbacks-fetch-updates-2021-08-18
…dd callback to preload function
…dates-2021-08-18 Feature/preload callbacks fetch updates 2021 08 18
We have updated our PR with your last changes. |
Please merge this PR. I really need it 👍 |
Hi guys just a last call if it's possible to merge this PR, we invested time here to bring last changes to the PR and I think this will help a lot of people. |
@@ -14,7 +13,7 @@ import { | |||
ViewProps, | |||
} from 'react-native' | |||
|
|||
const FastImageViewNativeModule = NativeModules.FastImageView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should remain because it is needed to use "clearMemoryCache" and "clearDiskCache". It also generate a typescript error.
how to implement this into my project? because i don't have src folder in my node_modules fast image |
Hi @putuoka, did you download the stable version of the package or did you download it from this pull request? If you have downloaded the stable version in your |
Hi @nicomontanari , thanks for the reply. i have downloaded stable version using sorry because i'm new with rn i might missing something here. sorry for my english. please enlighten me because i need to use preload promise/callback to know when it done downloading resources and then i can use it as cache with fast images & redux persist |
@putuoka you need to install this package with |
Hi @nicomontanari i got this error:
edit: run solved!! it's working!! here what i do:
note: it can't preload 151 images. sometimes just 32, sometimes 113 |
Btw This pull make FastImage.clearDiskCache() broken
code:
|
really need this! |
Any plans yet to merge this? Any forks that have this merged with upstream? |
I added the changes to my fork with the updated stream: #986 |
This would be so nice to have! |
Had the same need, couldn't patch the PR due to conflicts. I am using another patch to get the cache folder path
|
We also need this PR to be merged