Update functions.js fix Repeat button display

[raszpl]

#2019 went overboard and broke Repeat button display

fix broken player_always_repeat button display
moving functionality to function
playerRepeat renamed to playerAlwaysRepeat, modified to change it-repeat-button when pressed, previously it only changed when reloading page or clicking twice playerRepeatButton (disabling&enabling)
synchronizing playerRepeatButton with playerAlwaysRepeat and improvedtubeYoutubeButtonsUnderPlayer below_player_loop. No need to reload page to se effect of one on others, clicking one updates all.
formatting, indentation
Update styles.css
cant have this style with dynamically injected improvedtubeYoutubeButtonsUnderPlayer

Update appearance.js
added createUnderPlayerButton helper function, dynamic improvedtubeYoutubeButtonsUnderPlayer injection

Update functions.js
added createUnderPlayerButton helper function

Update core.js
dynamic improvedtubeYoutubeButtonsUnderPlayer injection and switching

Update blocklist.js
Made blocklist dynamic, Disabling Blocklist automatically shows blocklisted videos without the need to refresh page.

Well, this has escalated quickly, wherever I look there is something to fix.

[ImprovedTube]

lets merge this today without the "repeat"-commits for today. #2019 (comment)

[ImprovedTube]

hi @raszpl! read your repeat buttons code by now. And all changes, missed appearance.js too at first (review will be accelerated when running automatic formatting [such as indentation, & spaces] as in a separate PR on the whole repo for once. Or maybe/ultimately continuously as a github action/routine/workflow)

( Previously only reacted on this frustration:

oh great, there is also improvedtubeYoutubeButtonsUnderPlayer

)
and was confused because the two buttons already sync with no reload.

so,

• Sorry BrainStorming: 1. 'normal' vs null ?? & 2. Active Features GUI  #1685 is still open

• the following creates overhead. And should be moved to core.js to run when settings storage changes only:

  for (blocked of document.querySelectorAll('div.it-blocklisted-video')) {
  	blocked.classList.remove('it-blocklisted-video')
  }

• Finally, we can keep the ImprovedTube.storage.below_player_loop !== false, not to query the DOM unnecessarily.

so long! thanks!

(hope i didnt interrupt you? @raszpl)

[raszpl]

I stopped because it was too much changes for one patch, became messy.

) and was confused because the two buttons already sync with no reload.

but there are three buttons :) 4.804 :
1 Enable Repeat
2 Enable Always active
3 Enable Buttons (below the pla.. Loop
4 Disable Always active
5 forward video to last few seconds
6 hit play

doesnt repeat despite both Loop buttons being active. Only refreshing whole page can get them to sync, but dont refresh, continue:

7 click one of the Loop buttons

nothing visually happens, bad

8 Enable Always active
9 Disable Always active
10 Disable Repeat
11 Enable Repeat

nothing matches :) one shots loop active, other shows inactive

• if( this.storage.player_always_repeat is missing === true (as mentioned in Repeat stuck on #2019 (comment))

and I replied: "its tautology. What values can this.storage.player_always_repeat have?"
its boolean, there is no need for elaborate compares.

unless we'd remove all default settings from storage to avoid this misunderstanding once and for all

that would be ideal, settings that dont do anything just pollute user settings. Only settings that actually change from default YT behavior should be stored.

• the following creates overhead. And should be moved to core.js to run when settings storage changes only:

  for (blocked of document.querySelectorAll('div.it-blocklisted-video')) {
  	blocked.classList.remove('it-blocklisted-video')
  }

it was just preliminary code, I rewrote it couple of times since this and #2098, still testing locally

• Finally, we can keep the ImprovedTube.storage.below_player_loop !== false, not to query the DOM unnecessarily.

forest for the trees :) You dont like this:
if (this.storage.below_player_screenshot && !document.getElementById('it-below-player-screenshot')) {

			} else if (camelized_key === 'belowPlayerLoop' ) {
				if (ImprovedTube.storage.below_player_loop) {
					ImprovedTube.improvedtubeYoutubeButtonsUnderPlayer();
				} else {
					document.getElementById('it-below-player-loop')?.remove();
				}

while original code does DOM manipulations removing 3 elements every time its called

				case 'belowPlayerLoop':
					if (ImprovedTube.storage.below_player_loop === false) {
						document.querySelector('.improvedtube-player-button[data-tooltip="Loop"]')?.remove();
					} else if (ImprovedTube.storage.below_player_loop === true) {
						document.querySelectorAll('.improvedtube-player-button').forEach(e => e.remove());
						ImprovedTube.improvedtubeYoutubeButtonsUnderPlayer();
					}
					break

getElementById is practically free https://gomakethings.com/javascript-selector-performance/

Still this is all meaningless as improvedtubeYoutubeButtonsUnderPlayer will be called exactly once per page load, and belowPlayerLoop only when switching options.

edit: called exactly once per page load well, looking at

youtube/js&css/web-accessible/init.js

Lines 8 to 15 in b8f441e

	ImprovedTube.observer = new MutationObserver(function (mutationList) {
	for (var i = 0, l = mutationList.length; i < l; i++) {
	var mutation = mutationList[i];
	if (mutation.type === 'childList') {
	for (var j = 0, k = mutation.addedNodes.length; j < k; j++) {
	ImprovedTube.childHandler(mutation.addedNodes[j]);

everything is potentially being run hundreds of times :( need to investigate further

[ImprovedTube]

Syncing the two buttons just started here 282f7d6
Then meant to keep code short and ok to work from next page load in some scenarios fd9dec9 73b4091
before possibly also working on it at (message.action === 'storage-changed') { // FEEDBACK WHEN THE USER CHANGED A SETTING Was just wondering to avoid voluntary complexity. (Not sure how many people will intentionally enable both)(or 3 or 4 of them (of course the button could come in more ways like position:fixed or following the mouse.)

This is bad UI design, users wont know its turned on and complain like they did here in this bug.

Like i wondered years ago besides many of our features are set permanently

it might look nice, but breaks search

Right🤔 i guess search can consider tags, head or pathes in future

Finally, we can keep the ImprovedTube.storage.below_player_loop !== false, not to query the DOM unnecessarily.
forest

Yes, meant hasAttribute etc. in /* REPEAT BUTTON */ ImprovedTube.playerRepeatButton = function (node) {
checking storage is just free and principle for readability.

---

mutation ... investigate further

👍