Media still looping with loop={false}

[sslotsky]

Hello again! I am working from the same setup that I described in #23. I swear this was working before I'd solved that problem, but now my media loops despite setting the loop prop to false. This was actually the problem we set out to solve by upgrading the package!

Here's the outermost container:

export class CallPlayerContainer extends Component {
  static propTypes = {
    ...
  };

  componentWillReceiveProps(nextProps) {
    const {
      displayOnly,
      callPlayer,
      media
    } = nextProps;

    if (!displayOnly && media.isPlaying !== !!callPlayer.get('active')) {
      media.playPause();
    }
  }

  isStale() {
    const { record } = this.props;
    const duration = record.getIn(['call', 'duration']);
    const occurredAt = moment(record.getIn(['call', 'occured_at']));
    return duration === 0 || occurredAt.isBefore(moment().subtract(90, 'days'));
  }

  render() {
    const toggle = () => {
      const {
        record,
        callPlayer,
        customPlayPause,
        actions: { playPause: defaultPlayPause }
      } = this.props;

      (customPlayPause || defaultPlayPause)(record, !!callPlayer.get('active'));
    };

    return (
      <CallPlayer {...this.props} toggle={toggle} isStale={this.isStale()} />
    );
  }
}

const Connected = withMediaProps(connect(
  ...
)(CallPlayerContainer));

export default props => (
  <Media>
    <Connected {...props} />
  </Media>
);

And here's the <CallPlayer />

export default function CallPlayer(props) {
  const { toggle, src, record } = props;

  const tooltipText = record.getIn(['call', 'duration']) === 0 ?
    I18n.t('lead.tooltip.duration_is_zero') : I18n.t('lead.tooltip.over_ninety_days');

  return (
    <div>
      <div onClick={toggle} className='no-card-toggle'>
        <Player src={src} loop={false} />
      </div>
      <nav className='media-controls'>
        <LeadTitle toggle={toggle} {...props} message={tooltipText} />
        <PlayBar {...props} message={tooltipText} />
      </nav>
    </div>
  );
}

Any idea why it might be looping?

[souporserious]

Ah dang 😞 sorry about that. I'll look at this ASAP. Hopefully tomorrow or later this week. If you want to take a look, this is where looping is handled. Wondering if something is preventing onEnded from being called.

[sslotsky]

Thanks for the link. I set a breakpoint in that handler and it does fire at the end of the call. loop evaluates to false so we're good there. Then when I get to the end of the handler, the onEnded prop is undefined. I tried passing in my custom toggle function:

        <Player src={src} loop={false} onEnded={toggle} />

This works, but it seems like I shouldn't have to do that, right?

[sslotsky]

Actually I have to correct myself. The above only works if I pause at a breakpoint to inspect things. If I clear the breakpoints, the call continues looping and an error is raised.

Uncaught (in promise) DOMException: The play() request was interrupted by a call to pause().

[souporserious]

Interesting.. I'm guessing somewhere onEnded is getting dorked. Thanks for digging in a little more! I appreciate the help. Still busy with other things right now. But this will be the first thing I look at when I get a chance.

[sslotsky]

@souporserious Just making sure this doesn't go stale. We might be revisiting this in the next couple weeks. I totally get that OSS doesn't pay the bills though.

[souporserious]

Sorry, I've been trying to find time for this. I'm hoping to be able to fix this along with another bug we found in our app.

I was also going to start looking at allowing the library to play with Redux better. This is my initial sketch for what I thought it could look like:

const onPlayPause = (isPlaying) => dispatch(isPlaying)

<Media state={reduxStoreOrWhatever} callbacks={{ onPlayPause }}>
  <Player src={...}/>
  <PlayPause/>
</Media>

Does that makes sense and does it seem like it could work? Sorry if that's totally wrong haha, I don't have much experience with Redux yet.

[sslotsky]

Currently, react-media-player tells us whether or not it's playing. I think the key to having this work with redux is that we should tell it whether or not it's playing. So instead of relying on the media object to control play/pause behavior, maybe it would be something like:

<Media isPlaying={somePropFromMyReduxStore} onPlayPause={someActionBoundToDispatch}>

Your code could then use its own state & callbacks if the user hasn't provided any. If they have been provided, we would only use the media object for things like seekTo, etc, and allow the user provided props to control whether or not the media is playing.

Another option would be to do something like react-player has done, where simply attaching a ref to the ReactPlayer component gives you total control over its behavior. This would work fine with redux as well.

I'm not sure how well either one of these works with your current design. Maybe there are other ways.

[souporserious]

Sweet, that's what I had in mind :)

I had an idea last night that Player can live on it's own without Media and it would just be a dumb component that received props. And based on what props it received, it would call the internal vendor methods like play, pause, etc. I just took a look at react-player and it seems like they do something like this.

Then you could add the Media component as a helper if you wanted or build your own if you needed Redux or something similar. I think doing it that way would avoid having to pass callbacks and state to Media and just get rid of it all together if you don't need it since it's somewhat acting like a store. Does that sound like it would work?

Thanks for the feedback! I appreciate it :)

[sslotsky]

Yes, I think you could render the Media component completely obselete. You could also expose hooks into the Player component that report the current time or any other data that the controls rely on.