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

[Image] Old images on Movies example app listings #89

Labels
Resolution: Locked This issue was locked by the bot.

Comments

@jchris
Copy link

jchris commented Feb 21, 2015

To reproduce, search for a movie and browse into it, then back out of the movie view to the search, then delete your search terms by holding down the delete key (on your laptop while running in simulator). It seems to be timing dependent but with a few tries I was able to generate the screenshots below.

wayne

apollo

Maybe this is throttling from the data API? Maybe caching would help?

not found

@vjeux
Copy link
Contributor

vjeux commented Feb 21, 2015

(There's probably a bug here but let me digress a bit with a general problem here. cc @a2)

When you have <Image source={a} /> and then render <Image source={b} />, while the new source is being loaded, what should we do.

  • Blank: The "correct" response is to blank out the image, then when b is loaded and decoded update the image. The big issue with this solution is that your UI is going to flash all the time leading to a very ugly end user experience.
  • Keep a: The better user experience is to keep a around for the few milliseconds and when b is ready then display b. This avoids the flash and is fine as long as it doesn't take too long.

What's interesting is that on the web, there's actually both behaviors implemented. <img> tag keeps a while background-image blanks out.

We haven't made a concrete decision on this one yet and I'm unsure what's the current behavior, but this behavior is worth thinking about.

@jchris
Copy link
Author

jchris commented Feb 21, 2015

Maybe keep a around to make a smooth transition to b, but with a timeout where it transitions to a loading / can't load indicator if it hangs around too long?

@vjeux
Copy link
Contributor

vjeux commented Feb 21, 2015

Good idea :)

@sophiebits
Copy link
Contributor

The "keep a" experience is probably better at an infrastructural level because you can always get the "blank" way by adding a key to the Image equal to the src.

@ghost
Copy link

ghost commented Apr 17, 2015

@vjeux I've tried using <Image defaultSource=... /> to show some kind of a loading placeholder image, but it shows the old image indefinitely. I'll get a minimal repro set up for you.

Meanwhile is there workaround we can use to blank the image and force loading the correct one?

@ghost
Copy link

ghost commented Apr 21, 2015

workaround

/* @flow */
'use strict';

var React = require('react-native');
var { Image } = React;

// Hack to work around https://github.com/facebook/react-native/issues/89
//
// Render an <Image /> normally.
// When react reuses the component with a new url, force react to render a
// blank image once before rendering the correct url later.

var DelayedImage = React.createClass({
  propTypes: Image.propTypes,
  getInitialState(): { showImage: boolean } {
    return { showImage: true };
  },
  componentWillReceiveProps: function(nextProps: any) {
    if(this.props.source.uri != nextProps.source.uri) {
      this.setState({ showImage: false });
      setTimeout(() => this.setState({ showImage: true }), 0);
    }
  },
  render: function(): React.Component {
    return <Image {...this.props} source={{uri: this.state.showImage ? this.props.source.uri : null}} />
  },
});

module.exports = DelayedImage;

@brentvatne brentvatne changed the title Old images on Movies example app listings [ListView] Old images on Movies example app listings May 30, 2015
@brentvatne
Copy link
Collaborator

The issue here now is that the images change but the text does not

bug

@vjeux - who is the owner of ListView?

@sophiebits
Copy link
Contributor

Probably an Image issue; I can't imagine it being specific to ListView.

@sahrens
Copy link
Contributor

sahrens commented Jun 12, 2015

This is pretty busted - any updates, @nicklockwood?

@sahrens sahrens changed the title [ListView] Old images on Movies example app listings [Image] Old images on Movies example app listings Jun 12, 2015
@christopherdro
Copy link
Contributor

This is a tough one to catch since the results are never consistent. I was still able to reproduce the issue, maybe 1 out of every 4-5 attempts, following @jchris explanation.

See demo below for steps...

I think the catch here is to make sure each <Image /> inside of a ListView row has a key property set.
In the case of the Movie Example, adding a key to each image inside of our MovieCell seems to resolve the issue.

I've noticed some other people having issues with Images rendering properly within ListView and this did the trick.

<Image
  source={getImageSource(this.props.movie, 'det')}
  style={styles.cellImage}
  key={this.props.movie.id}
/>

Not sure if having to add a key a bug or just a requirement to get the images to re-render properly.
Thoughts?

Image of Demo

@jchris
Copy link
Author

jchris commented Jul 21, 2015

My thought is that a "loadingImage" property? would force the state machine to represent that the image isn't ready.

@brentvatne
Copy link
Collaborator

@jchris - we can do this now by leveraging the onLoad callbacks 😄

@MossP
Copy link

MossP commented Jul 22, 2015

Quick observation on this. I've been seeing this bug for sometime now elsewhere. I've tried keying the image, setting it in state, then calling form there. I've dropped debug points and can see the url that was being assigned to the image wasn't what I was seeing. It only happened on one image in my render row though which got me thinking...

Through elimination the thing that got it working for me was to remove the <TouchableOpacity/> and replace with <TouchableWithoutFeedback />. I have no idea why this works and it may be a Red Herring (maybe something was cached?) but mine has worked consistently through 15 builds now, and I can trigger the bug again as soon as I swap back.

@brentvatne
Copy link
Collaborator

@christopherdro fixed this in the commit referenced above 😄

@facebook facebook locked as resolved and limited conversation to collaborators Jul 23, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.