add placeholder functionality
#409
Conversation
|
Curious to see if any performance testing has been done with this. I believe I've done something similar in the past, however I ran into issues with 100+ instances running. |
|
Currently our app uses this feature. Haven't noticed any issues. |
|
Is there anything pending before this can get merged? |
|
We'd love to see this merged too |
added ✔️ |
|
The error thrown by CI is related to the version of node. |
show placeholder when timer is expired
|
This PR adds exactly what we need, is there anything blocking the merge? :3 |
|
@Ilphrin nobody knows... |
|
@DylanVann Do you need help in maintaining the package or managing the PRs? Your project is really cool, and as our team rely on it, that would be awesome if new features/fixes arrives :D |
| @@ -82,6 +82,7 @@ export interface OnProgressEvent { | |||
|
|
|||
| export interface FastImageProperties { | |||
| source: FastImageSource | number | |||
| placeholder?: React.ReactNode | |||
There was a problem hiding this comment.
Maybe you can allow number here too? So that one could write:
import myPlaceholderPicture from 'src/images/myPlaceholderPicture.png';
....
<FastImage source={{ uri: myUri }} placeholder={myPlaceholderPicture} />
Instead of being forced to do this, for example:
import { Image } from 'react-native';
import myPlaceholderPicture from 'src/images/myPlaceholderPicture.png';
....
<FastImage source={{ uri: myUri }} placeholder={<Image source={myPlaceHolderImage}} />} />
Though I am assuming that the default placeholder for an image is another image.
|
@baldarian any news on this PR? :3 There is a conflict in index.js and I've made a proposal for a fix. When this is ok Dylan can take a look at this I think ^^ |
|
It would be pretty cool if this new feature can be merged into the project. |
|
Sure! But before theses conflicts need to be resolved :/ |
| @@ -2,6 +2,8 @@ module.exports = { | |||
| parser: 'babel-eslint', | |||
| env: { | |||
| es6: true, | |||
| browser: true, | |||
| node: true, | |||
There was a problem hiding this comment.
Btw, why did you modified the eslintrc? ^^
|
Up ! This feature will be very useful, when can we have it ? :) |
|
Would be very excited if this were to be merged. Without placeholder, this library isn't very useful to me. |
|
Also necessary for my needs. |
|
(Just my 2 cents), after digging around more into the very awesome react-native-fast-image 🔥 , I don't think this PR is a good solution for placeholder. The PR #498, is a much better solution for iOS, as it lets the native side do it's thang. This PR has timeouts, which i get very nervous about performance over bridge, especially thinking of lists of hundreds of images, all relying on placeholder prop. The placeholder support on android glide, is supported in a newer version of glide as well. |
|
Can't believe this hasn't been merged yet. |
|
@osikes Oh that's cool you looked that much into it! #498 is cool but it is sad that it only adds support for iOS, having both would be cool, even though as a first step I guess it'd be ok. What do you think @DylanVann? :3 @lucasfeijo this project is the work of people who use their spare time to give devs a pretty awesome tool, you just need to be patient, and if you wish to make things go faster you can always help ;) |
|
@Ilphrin I wasn't trying to be condescending, just disappointed, I would love to help, but I just stumbled upon this issue in my project (lack of placeholder) recently, I'll try to see if I can be of any help. |
|
No problem text messages can be easily misunderstood ;) If you can help that'd be awesome! |
|
Can this be resolved and merged? It would be a nice feature. |
|
hey guys, any updates? |
|
Hello! Really wonder to see this merged =) |
|
how much work is left? I wonder if this could be merged so we could all used a placeholder and drop some custom implementation that we all probably doing. |
|
Any plan on merge this? Thanks. |
|
2021 |
|
this lib is DEAD? |
Display placeholder in case image is loading or happened any errors.