Fix/reset progress bar

[amtins]

Description

When player.reset is called the load-progress-bar, seek-bar and current-time-display components are not correctly reset.

Specific Changes proposed

Adds the missing UI components to the resetProgressBar_ method so that they are properly reset.

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Change has been verified in an actual browser (Chrome, Firefox, IE)
  • Unit Tests updated or fixed
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[codecov]

Codecov Report

Merging #8160 (9d58ef7) into main (a27ee05) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #8160      +/-   ##
==========================================
+ Coverage   82.02%   82.12%   +0.09%     
==========================================
  Files         110      112       +2     
  Lines        7344     7405      +61     
  Branches     1773     1786      +13     
==========================================
+ Hits         6024     6081      +57     
- Misses       1320     1324       +4     

Impacted Files	Coverage Δ
src/js/player.js	89.88% <100.00%> (+0.11%)	⬆️
src/js/component.js	93.09% <0.00%> (-1.03%)	⬇️
src/js/menu/menu.js	66.30% <0.00%> (ø)
src/js/tech/tech.js	83.71% <0.00%> (ø)
src/js/utils/time.js	76.47% <0.00%> (ø)
src/js/tech/loader.js	92.85% <0.00%> (ø)
src/js/close-button.js	92.85% <0.00%> (ø)
src/js/event-target.js	100.00% <0.00%> (ø)
src/js/live-tracker.js	98.42% <0.00%> (ø)
src/js/modal-dialog.js	87.97% <0.00%> (ø)
... and 61 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[amtins]

I also tried to use a unit test to add the test coverage, but it seems that the test below does not do what it says it does 😅

video.js/test/unit/reset-ui.test.js

Lines 5 to 20 in 54830cd

	QUnit.test('Calling resetProgressBar player method should place progress bar at 0% width', function(assert) {
	const player = TestHelpers.makePlayer();
	player.currentTime(20);
	player.trigger('timeupdate');
	player.resetProgressBar_();
	assert.equal(
	player.controlBar.progressControl.seekBar.playProgressBar.el().offsetWidth, 0,
	'progress bar is reset to width 0%'
	);
	assert.equal(
	player.currentTime(), 0,
	'player current time is 0'
	);
	player.dispose();
	});

From PR #5684 release in version v7.5.0

[usmanonazim]

I also tried to use a unit test to add the test coverage, but it seems that the test below does not do what it says it does 😅

video.js/test/unit/reset-ui.test.js

Lines 5 to 20 in 54830cd

	QUnit.test('Calling resetProgressBar player method should place progress bar at 0% width', function(assert) {
	const player = TestHelpers.makePlayer();
	player.currentTime(20);
	player.trigger('timeupdate');
	player.resetProgressBar_();
	assert.equal(
	player.controlBar.progressControl.seekBar.playProgressBar.el().offsetWidth, 0,
	'progress bar is reset to width 0%'
	);
	assert.equal(
	player.currentTime(), 0,
	'player current time is 0'
	);
	player.dispose();
	});

From PR #5684 release in version v7.5.0

Definitely a good catch! It would be worth either rewriting this unit test or writing new tests for all the functionality you've added

[amtins]

Definitely a good catch! It would be worth either rewriting this unit test or writing new tests for all the functionality you've added

@usmanonazim Thank you for taking the time to review this PR. I will try to add the test coverage this weekend.

[usmanonazim]

Looks good! Just had a question about the test 😁

[amtins]

Do resetProgressBar_ really need to call this.currentTime(0); since the call to el.load() will reset everything ?

video.js/src/js/tech/html5.js

Lines 1356 to 1382 in 2a99a78

	Html5.resetMediaElement = function(el) {
	if (!el) {
	return;
	}
	const sources = el.querySelectorAll('source');
	let i = sources.length;
	while (i--) {
	el.removeChild(sources[i]);
	}
	// remove any src reference.
	// not setting `src=''` because that throws an error
	el.removeAttribute('src');
	if (typeof el.load === 'function') {
	// wrapping in an iife so it's not deoptimized (#1060#discussion_r10324473)
	(function() {
	try {
	el.load();
	} catch (e) {
	// satisfy linter
	}
	}());
	}
	};

[usmanonazim]

It might not be necessary to call this.currentTime(0) but I'm not 100% sure about that so I'd be happy still keeping that in the resetProgressBar_ fn

[usmanonazim]

Looks good to me! 🥳

[amtins]

@usmanonazim thanks again for your review. 👍🏽