Optimize Contentful image loading #593
Conversation
…progress implementation of ImageBackground that uses static values
Lily418
changed the title
…e test for image rendering
Lily418
changed the title
There was a problem hiding this comment.
Looks mostly good. I would add a safety null check for an existing image to be safe.
We had crashes in the beginning because we had references to non-existing featured events, which was quite annoying. (A draft event was selected as a featured event. So we got the reference to it, but not the event object itself.)
src/data/image.js
Outdated
| } | ||
|
|
||
| const imageDetails = images[id]; | ||
| const imageUriWithResizingBehaviourParameters = `${imageDetails.uri}?w=${ |
There was a problem hiding this comment.
I would not count on the image details being present all the time, i.e. imageDetails might be undefined.
I think it might be possible in Contentful to select an image in an event, which is still a draft. This means we get the event with the image id. But we don't load the image because we only download published images.
src/components/ImageBackground.js
Outdated
| this.state = { imageSize: null }; | ||
| } | ||
|
|
||
| onLayout = (event: Object) => { |
There was a problem hiding this comment.
React Native exposes the flow type for the layout event. You could use this here.
Example:
pride-london-app/src/components/Collapsible.js
Lines 65 to 67 in 1b40ff6
|
@Lily418 Btw, you might want to update your git settings for |
|
@frigus02 Thank you, have updated with a new test for that case and updated based on other comments |
Implemention for #587
Render transparent image initally and then request an image of the rendered size.
Tester check-list
Tested on multiple devices / OS versions (Test on the physical device(s) you have access to, otherwise use BrowserStack):
Optional: