react-native-fast-image
react-native-fast-image copied to clipboard
DylanVann
Feature/preload callbacks
Codecov Report
Merging #437 (c86abda) into main (a7a8643) will decrease coverage by
45.01%. The diff coverage is17.39%.
:exclamation: Current head c86abda differs from pull request most recent head 0ee6e1f. Consider uploading reports for the commit 0ee6e1f to get more accurate results
@@ 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
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/preloaderManager.js | 15.78% <15.78%> (ø) |
|
| src/index.js | 85.00% <25.00%> (ø) |
|
| src/index.tsx |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 4d8c749...0ee6e1f. Read the comment docs.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
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 ^^
Really happy to see this still moving forward! I can't wait to remove my component hack 😄
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
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 are you still working on it ? If you did the iOS part, don't hesitate to open a PR so we can work on the Android side Just let me know so I can know if I should just wait or work on it !
@joan-saum https://github.com/RWOverdijk/react-native-fast-image/tree/feat/preload-callbackl
The Android code is nonsense. I will work on it but I don't know when exactly so feel free to PR to that branch and I'll test and PR to this repo.
I've been hopefully anticipating this feature for over a year now. There's some unfortunate off-putting visual behavior in the apps where i use react-native-fast-image because of it, and i'm guessing i'm not the only one. Implementing preload with no way of knowing when it completed is as near as makes no difference as useful as no preload at all.
Had i the ability and confidence to build this feature myself i would, but i don't. At this point i'd happily pay someone to have a PR with this feature merged. DM me if interested.
I tried the android side but I don't have enough Java skills to make it or it would take me too much time. For now, I'm using the fork which was used for this PR: https://github.com/DylanVann/react-native-fast-image/pull/268 to have the callback (and it works well). Maybe you should consider doing this @augustbjornberg
@DylanVann It is pitty this PR has not been merged, even if SDWebImage does not give enough informations (see the conversation in the PR). It could be a great step for everyone who wait for having a callback of the preload.
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.).
We have updated our PR with your last changes. There is a lot of people that need these changes. Could you please merge this request @DylanVann ? Thanks a lot !
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. So I hope my words will find you @DylanVann Thanks a lot for your work and maybe we could help to maintain this package for other needs !
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 node_modules folder you should see the compiled package in the dist folder, if not you should see the entire project as the below 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
node_modulesfolder you should see the compiled package in thedistfolder, if not you should see the entire project as the below image:
Hi @nicomontanari , thanks for the reply. i have downloaded stable version using yarn add react-native-fast-image
what i mean is, in this pull request there is 12 files change one of them is PreloaderManager.tsx which i can't find in my node_modules react-native-fast-image
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 yarn add DylanVann/react-native-fast-image#pull/437/head or npm install DylanVann/react-native-fast-image#pull/437/head if you use npm