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

fix(FEC-11400): live seekbar doesn't work properly - regression #622

Merged
merged 6 commits into from
Jul 14, 2021

Conversation

yairans
Copy link
Contributor

@yairans yairans commented Jul 13, 2021

Description of the Changes

revert #589 and #615
make the live seekbar range between dvr start to the live duration

Solves FEC-11400

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

OrenMe
OrenMe previously approved these changes Jul 14, 2021
}
});
this.props.eventManager.listen(player, player.Event.DURATION_CHANGE, () => {
this.props.updateDuration(player.liveDuration - player.getStartTimeOfDvrWindow());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename to updateSeekBarDuration to avoid confusion with engine updateDuration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest opening a separate ticket to refactor the seekbar - remove duplicate handling of currentTime and duration in the seekbar reducer. Use Engine reducer for these props and add internal getter calculation which reduces the dvrstarttime

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decided not to change the names since need to change in other places and distinguish between these calls and the engine's calls

@@ -47,11 +47,24 @@ class SeekBarLivePlaybackContainer extends Component {
* @memberof SeekBarLivePlaybackContainer
*/
componentDidMount() {
this.props.eventManager.listen(this.props.player, this.props.player.Event.TIME_UPDATE, () => {
const {player} = this.props;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if already extracting props then extract all that is used (updateCurrentTime, updateDuration...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -554,7 +547,7 @@ class SeekBar extends Component {
*/
render(props: any, state: Object): React$Element<any> {
const virtualProgressWidth = `${(props.virtualTime / props.duration) * 100}%`;
const progressWidth = `${props.forceFullProgress ? 100 : (props.currentTime / props.duration) * 100}%`;
const progressWidth = `${(props.currentTime / props.duration) * 100}%`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider leaving forceFullProgress as it might fix a bad experience when DVR is very small

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@yairans yairans merged commit 12dcac3 into master Jul 14, 2021
@yairans yairans deleted the FEC-11400 branch July 14, 2021 10:18
yairans added a commit that referenced this pull request Jul 15, 2021
revert #589 and #615
make the live seekbar range between dvr start to the live duration

Solves FEC-11400

(cherry picked from commit 12dcac3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants