SeekBar and CurrentTime only updating when actions are dispatched

[sslotsky]

I'm upgrading to 0.6.1, and I'm having an issue with some of the controls.

In the attached video, you can see that the SeekBar and the CurrentTime only update occasionally. Notice that they update when I mouseover & mouseout of the textarea beneath. That's because an action is being dispatched when that happens, and that's causing the components to re-render. If no actions are dispatched, then SeekBar and CurrentTime do not move at all.

I'll try to paste the minimum of what's needed to show how it's setup:

Outermost relevant component:

import React, { Component } from 'react';
import { withMediaProps } from 'react-media-player';
import CallPlayer from './LeadCall';
import { Media } from 'react-media-player';

export class CallPlayerContainer extends Component {
  componentWillReceiveProps(nextProps) {
    // some stuff that reads from media prop
  }

  render() {
    return (
      <CallPlayer {...this.props} />
    );
  }
}

const Decorated = withMediaProps(CallPlayerContainer);

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

Component that organized the controls:

import React from 'react';
import { Player } from 'react-media-player';
import PlayBar from './PlayBar';

export default function LeadCall(props) {
  return (
    <div>
      <div onClick={props.toggle} className='no-card-toggle'>
        <div className="media-player">
          <Player src={props.src} loop={false} />
        </div>
        <nav className='media-controls'>
          <PlayBar {...props} />
        </nav>
      </div>
    </div>
  );
}

A component renders react-media-player controls:

import React from 'react';
import { controls } from 'react-media-player';

const { CurrentTime, Duration } = controls;

export default function PlayBar(props) {
  return (
    <div>
      <CurrentTime className='media-control media-control--current-time' />
      &nbsp;/&nbsp;
      <Duration className='media-control media-control--duration' />
    </div>
  );
}

In the component above, CurrentTime is one example of a component that only updates if an action is dispatched. The same is true for SeekBar which is not shown here. Is there any obvious reason why these components aren't re-rendering as often as they should? Advice appreciated in advance. Thanks!

[souporserious]

Thanks for posting your issue. I'm not sure what is causing this off the top of my head, it could be a React context issue not sending an update for some reason. I see shouldComponentUpdate in your example above, maybe it's blocking updates to the player? Can you try removing that and seeing if everything works as normal? I have a feeling it might be related to this issue 😕

[sslotsky]

There is no shouldComponentUpdate, just a componentWillReceiveProps. Here's the contents of that hook:

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

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

I have many of these media players on a page, with a custom PlayPause button which sets the active player. That causes the above hook to fire so that the active player will start playing and any other players will stop.

[souporserious]

Doh! I read that totally wrong, my bad. Hmm I'm stumped on what it could be 😕 I'm guessing it has to do with the use of context since that passes the Player state up to the Media component. I'm currently tied up with work right now, but I will look at this as soon as I get a chance. Please feel free to post a PR if you find a solution in the meantime. If you need direction on anything please let me know.

[sslotsky]

I think your suspicion is correct. If I do this:

export default props => (
  <Media>
    <div>
      <CurrentTime className='media-control media-control--current-time' />
      <Connected {...props} />
    </div>
  </Media>
);

then the CurrentTime component shown above will update properly, but the one that lives deeper down the hierarchy still does not.

Do you have any suggestions for how to propagate this down the hierarchy correctly? I will look into ways to refactor it but this seems like a major limitation that could stand in the way of keeping maintainable components.

[sslotsky]

Ok, it turns out that my attempt to abridge my example left out something important. My CallPlayerContainer was also being decorated with the connect function from redux. I simply reversed the order of my decorators and now everything works! I don't think I would have discovered this without your tip about context, so thanks very much for listening!

[souporserious]

Awesome, I'm glad you were able to figure it out :) is there anything I can do library side to make it easier? Sounds like your using redux? Any pain points with that so far? I welcome any feedback and any way I can make it easier, more reasonable to work with.

[sslotsky]

Thanks for asking. I was using 0.4.0 previously, and I definitely think the API seems cleaner and less confusing now. However, the need to use the Media component made things slightly awkward for me to integrate with our redux app logic.

As you can see if you review my example, my top level component was just a container that read some values from the redux state and from the media props in order to decide whether or not to invoke playPause. So in order to have access to the redux state and the media props, while also keeping the hierarchy contained within the Media context, I wound up having to do this:

export class CallPlayerContainer extends Component {
  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(
  (state, ownProps) => ({
    customPlayPause: ownProps.actions.playPause,
    callPlayer: state.callPlayers.
      find(cp => cp.get('id') === ownProps.record.get('id')) ||
      Map()
  }),
  dispatch => ({
    actions: bindActionCreators({
      initialize: callPlayerActions.initialize,
      playPause: callPlayerActions.playPause,
      pauseCall: callPlayerActions.pause
    }, dispatch)
  })
)(CallPlayerContainer));

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

Maybe you can spot a better way. Seems like a fair amount of hoop jumping to me, but it might be that I'm just making it more complicated than it has to be.

[souporserious]

This is good to know :) thank you for sharing! It does seem like a lot of hoop jumping. This library somewhat goes against React methodologies since it controls all of the state. When I revisit this project I will take passing your own state into account since that should be easy to do by default. Either allow state to come from a Redux store or wherever, or if you don't care and just want to have a player on a page then allow the player to keep an internal state.