Add autoplay property

[AquiGorka]

I have a list of GIFs showing and I wanted to have the ones above the fold to autoplay.

[benwiley4000]

Should it just be a Boolean? Or are you also suggesting we should have code to test what is "above the fold"?

[AquiGorka]

I don't believe your library should be responsible for above the fold dynamics, in this PR I only added the relevant code to take care of the autoPlay param for first render and follow ups.

I only mentioned my use case so that you could understand where this PR came from.

[AquiGorka]

I do however, want to be able to stop a GIF that's playing (my use case: when the GIF is no longer in the viewport).

Should I rename the property to play/playing?

[benwiley4000]

Hm, I see what you mean.

My inclination is to say we shouldn't be using props for this... props should describe ongoing state, not events.

It doesn't make sense to me to have a play prop that is true, but have internal playing state that is false because the user clicked the image.

The other issue is that your play prop might already be false, and then if you want to update it to false again, there's no way for the component to know your intent is fresh.

However I understand your use case, and I'd like to solve it. Do you have any other ideas? I'll try to think this over.

[benwiley4000]

One wild idea I have is having a prop similar to a ref which is a function which receives another function which you can call to play or pause the gif on command.

e.g.

class MyComponent extends React.Component {
  componentDidMount() {
    setTimeout(() => this.togglePlay(true), 5000);
  }

  render() {
    return (
      <GifPlayer
        gif={gif}
        still={still}
        togglePlayRef={ref => this.togglePlay = ref}
      />
    );
  }
}

[AquiGorka]

props should describe ongoing state, not events.

You are right on this too.

How about a prop that describes if the Component is able to play the GIF. If it is not the case, then the state would show the still state–no matter the playing value. It could default to true (backwards compatible).

<GifPlayer ... autoPlay={true | false} canPlay={true | false} />

[benwiley4000]

Ok, that's interesting... would the gif start playing again when it re-enters the viewport? Or stay paused?

Also what did you think of my other proposal?

[AquiGorka]

would the gif start playing again when it re-enters the viewport?

For my use case I would keep it paused and ask the user to click on it to start playing again–but with canPlay and autoPlay both to true it could work the other way too.

I like your idea, but having a function that can set the internal state come from outside?

[AquiGorka]

If you agree, I'll work on the code for my proposal and update the PR. Let me know what you think.

[benwiley4000]

@AquiGorka I agree it's a little weird (having a function that can set internal state from the outside) but it's not the weirdest hack I've seen in React land. 🙂

Your idea is probably better.

I think just making it so playing has to be manually re-initiated by the user after canPlay has been false is the best way to go. I can't think of a use case where we would want to change the state only when the GIF is off-screen. 😄 And I don't think autoplay should take effect unless the GIF is fresh.

I think we agree... could you update your PR? Thanks! 😄

[benwiley4000]

I should note I'm hesitant to let autoplay have any effect at all after the initial render (i.e. don't ever process it in componentWillReceiveProps). My thought for the spec is to:

1. Autoplay the gif if autoplay is truthy on the first render
2. Stop the GIF from playing whenever canPlay becomes falsy
3. Prevent clicks from playing the GIF while canPlay is still falsy

Step 3 makes me nervous. I'm not sure I want people to be able to have a button that looks clickable but doesn't do anything. I think, at all times, even if we decide to stop the GIF from playing, the user should be able to resume it. I understand this isn't relevant to your use case, but I'm thinking about general consumption of the library.

I keep going back and forth and wondering if the callback function is better...

Do you think the canPlay prop makes sense beyond your use case? Do we need to do something to the css while canPlay is false? (maybe remove the button?)

[benwiley4000]

Sorry for the mental back-and-forth. I've thought about it more, and while I think your solution looks a bit nicer, I have decided I strongly prefer my togglePlayRef idea I discussed earlier: https://github.com/benwiley4000/react-gif-player/pull/17#issuecomment-368556510

The reason being that while it's unconventional to modify state from outside a component, we're effectively doing that anyway by trying to control internal state via props, and this solution wouldn't lead to weird state inconsistencies or UI issues the way our other solutions might.

It's an escape hatch for edge cases where we want to override state previously set by the user, but not in a way that would block future user intent. Since it's an escape hatch, I don't mind that it's weird.

If you're still interested in completing this PR, feel free to implement that! Otherwise if I don't hear back in several days, I might try to find time to get to it myself.

Thank you for bearing with me. 🙂

[AquiGorka]

@benwiley4000 that's alright, this is how we reach consensus and collaborate to find the best solution. On my side I had to focus on other things before continuing with this PR. Today I'll give it a shot with your proposed solution and let you know how it went.

[benwiley4000]

Thanks @AquiGorka - any updates? 🙂

[AquiGorka]

@benwiley4000 sorry, I have been pulled into some work with higher priority, I need this feature for my project definitely but I have to put a pin on it for the time being, if you work on it I'll definitely use it and if not I'll come back eventually and work on it myself. :)

[benwiley4000]

Sure! I can do it this week.

[benwiley4000]

@AquiGorka I pushed some changes. Instead of implementing toggleRef I just implemented a pauseRef prop which allows you to remotely pause, but not remotely play. So it's a bit like your canPlay solution except it doesn't block future user intent.

Let me know what you think - I'll merge a bit later unless you have more comments.

[AquiGorka]

Superb, I tested this and it works.

Thank you @benwiley4000 this addresses my use cases.

[benwiley4000]

Awesome! I'll release shortly.

[benwiley4000]

@AquiGorka sorry for the delay - 0.3.0 is now published! https://github.com/benwiley4000/react-gif-player/releases/tag/v0.3.0