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.

Sign up for GitHub

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

Jump to bottom

feat(FEC-10817): expose the current time of the video element in live streams #589

Merged
merged 15 commits into from
Jun 17, 2021
Merged

feat(FEC-10817): expose the current time of the video element in live streams #589

merged 15 commits into from
Jun 17, 2021

Conversation

dan-ziv
Copy link
Contributor

@dan-ziv dan-ziv commented Apr 5, 2021

Description of the Changes

expose the current time of the video element in live streams

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

@dan-ziv dan-ziv self-assigned this Apr 5, 2021
@dan-ziv dan-ziv mentioned this pull request Apr 5, 2021
Merged
5 tasks
yairans
yairans suggested changes Jun 6, 2021
View reviewed changes
src/components/live-tag/live-tag.js Outdated
const tagStyleClass = [style.liveTag];
if (props.isDvr && !this.isOnLiveEdge()) tagStyleClass.push(style.nonLivePlayhead);
Copy link
Contributor

Choose a reason for hiding this comment

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

why isDvr removed? if needed so remove it from mapStateToProps as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are out-of-sync (paused for example) also on live without DVR, we would like to see indication on the live tag.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is new behavior. how is related to this change?

Copy link
Contributor Author

@dan-ziv dan-ziv Jun 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Now the calculation is different so we can support it as part of this change. Like before it's just a bug.

src/components/seekbar/seekbar.js
@@ -542,7 +542,7 @@ class SeekBar extends Component {
*/
render(props: any, state: Object): React$Element<any> {
const virtualProgressWidth = `${(props.virtualTime / props.duration) * 100}%`;
const progressWidth = `${(props.currentTime / props.duration) * 100}%`;
const progressWidth = `${props.forceFullProgress ? 100 : (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.

why is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now the live time is linear and duration is always bigger than the current time. For that reason you will see unfulfilled progress on the progress bar's edge. When we are on live edge this has no meaning so we would like to see full progress.

@dan-ziv dan-ziv requested a review from yairans June 17, 2021 05:49
yairans
yairans previously approved these changes Jun 17, 2021
View reviewed changes
@yairans yairans merged commit d257ef9 into master Jun 17, 2021
@yairans yairans deleted the FEC-10817 branch June 17, 2021 09:04
yairans added a commit that referenced this pull request Jul 13, 2021
@yairans
Revert "feat(FEC-10817): expose the current time of the video element…
b71bd07 
… in live streams (#589)"

This reverts commit d257ef9
yairans added a commit that referenced this pull request Jul 13, 2021
@yairans
Revert "Revert "feat(FEC-10817): expose the current time of the video…
5a08cd1 
… element in live streams (#589)""

This reverts commit b71bd07
@yairans yairans mentioned this pull request Jul 14, 2021
Merged
5 tasks
yairans added a commit that referenced this pull request Jul 14, 2021
@yairans
fix(FEC-11400): live seekbar doesn't work properly - regression (#622)
12dcac3 
revert #589 and #615 
make the live seekbar range between dvr start to the live duration

Solves FEC-11400
yairans added a commit that referenced this pull request Jul 15, 2021
@yairans
fix(FEC-11400): live seekbar doesn't work properly - regression (#622)
0d62026 
revert #589 and #615
make the live seekbar range between dvr start to the live duration

Solves FEC-11400

(cherry picked from commit 12dcac3)
borhandarabi pushed a commit to TasvirChi/playchi-js-ui that referenced this pull request May 15, 2024
fix(FEC-11400): live seekbar doesn't work properly - regression (#622)
b129ee9 
revert kaltura/playkit-js-ui#589 and kaltura/playkit-js-ui#615 
make the live seekbar range between dvr start to the live duration

Solves FEC-11400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yairans yairans yairans approved these changes

@OrenMe OrenMe Awaiting requested review from OrenMe

@Yuvalke Yuvalke Awaiting requested review from Yuvalke

@RoyBregman RoyBregman Awaiting requested review from RoyBregman

Assignees

@dan-ziv dan-ziv

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dan-ziv @yairans