benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
184 stars 28 forks source link

Progress bar seek renders faster and before rest of app. #386

Closed benwiley4000 closed 5 years ago

benwiley4000 commented 5 years ago

Closes #385.

danielr18 commented 5 years ago

Was there a breaking change here? Seeking is broken in my app on alpha 29

benwiley4000 commented 5 years ago

Not intentionally!

benwiley4000 commented 5 years ago

@danielr18 can you provide any more info on how exactly it's broken? It's working for me in all the examples.

danielr18 commented 5 years ago

When I seek to any position, the currentTime resets to 0

danielr18 commented 5 years ago
import React from 'react'
import classNames from 'classnames'
import { ProgressBar } from '@cassette/components'
import Handle from './Handle'
import * as styles from './styles'

class SeekBar extends React.PureComponent {
  handleSeekPreview = progress => {
    this.props.onSeekPreview(progress * this.props.duration)
  }

  render() {
    const { duration, currentTime, onSeekComplete } = this.props
    const displayedProgress = duration ? currentTime / duration : 0

    return (
      <div className={this.props.className}>
        <ProgressBar
          className={classNames('progressbar', styles.progressbar.className)}
          progressClassName={classNames('progress', styles.progressbar.className)}
          progress={displayedProgress}
          progressDirection="right"
          readonly={false}
          onAdjustProgress={this.handleSeekPreview}
          onAdjustComplete={onSeekComplete}
          handle={<Handle />}
        />
        {styles.progressbar.styles}
      </div>
    )
  }
}

export default SeekBar
danielr18 commented 5 years ago

When I seek to any position, onAdjustComplete is called with a time like: 0.8875

benwiley4000 commented 5 years ago

Ah yes... I didn't intend this as a breaking change but I see how it was. I added a new handleSeekComplete to the MediaProgressBar component to deal with the change.

https://github.com/benwiley4000/cassette/blob/next/packages/components/src/MediaProgressBar.js#L28

Does this seem ok? I'll update the release notes to mark this as a breaking change.

benwiley4000 commented 5 years ago

The reason I did this is because now the onAdjustProgress callback gets called after component update and I wanted to make sure the latest progress value does get sent in case the last handleAdjustProgress and handleAdjustComplete updates get batched.

danielr18 commented 5 years ago

It works now, missed that.

benwiley4000 commented 5 years ago

That's my fault.. thanks for reporting it!