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

Special 'satus_remove' value signaling default option, remove instead of storing #2220

Closed
wants to merge 24 commits into from

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Apr 28, 2024

Special 'satus_remove' value signaling default option. Storing 'satus_remove' in any config variable automagically deletes it. For example User switching player_quality back to auto will set it to 'satus_remove' value which in turn will automatically delete player_quality key from config.

Converted components.switch to only store active usage, flipping switch to default position deletes it from config. This respects skeleton.value: true witches that start as default ON like up_next_autoplay. Those only save False value and get automatically deleted when switched to True.

In principle no additional code changes should be required to \web-accessible\ side of extension, but this opens possibility of simplifying code a lot. After automatically converting all user configs on next upgrade we will no longer have to check for config_variable != 'disabled', != 'auto',=== true, === false, this.isset(option) etc.

@ImprovedTube
Copy link
Member

wow! 🥰

…hot_save_as

fixed embed_subtitle setting - it wasnt working with default embed_subtitle.value: true
…ettings()

subtitlesUserSettings() =
ImprovedTube.subtitlesFontFamily();
ImprovedTube.subtitlesUserSettings();
ImprovedTube.subtitlesFontColor();
ImprovedTube.subtitlesFontSize();
ImprovedTube.subtitlesBackgroundColor();
ImprovedTube.subtitlesWindowColor();
ImprovedTube.subtitlesWindowOpacity();
ImprovedTube.subtitlesCharacterEdgeStyle();
ImprovedTube.subtitlesFontOpacity();
ImprovedTube.subtitlesBackgroundOpacity();
@raszpl raszpl changed the title special key signalling default option, remove instead of storing Special 'satus_remove' value signaling default option, remove instead of storing Apr 29, 2024
@ImprovedTube
Copy link
Member

😏

@raszpl
Copy link
Contributor Author

raszpl commented Apr 30, 2024

Tested, working. Could be merged right now after adding code converting user configs (deleting all redundant true/false settings).

ImprovedTube
ImprovedTube previously approved these changes May 1, 2024
@ImprovedTube
Copy link
Member

ImprovedTube commented May 1, 2024

we currently have: (old news:)
3 defaults at installation (extra buttons below the player. Thanks for caring at #2085),

  • (which always appeared inverted in "my active features", in favor of the inverted ones😲. Yet should even appear as "my active features(3)" on the front screen . as a workaround currently they could have the skeleton value false and be enabled at installation in background.js )

and
7+ inverted switches just representing youtube's defaults covered by 36131ca ( d18cea6 3342afc 1253b9e),
( which are also "trashbin/cleaning" features (like many of our toggles and dropdown options called hidden, disabled, block). ( #1685 "long term structure" )

  • could alternatively only have an inverted switch/light by default visually only/CSS, just indicate YouTube's default and that it might be nice/performant to get rid of these. so using another toggle type in the skeleton "inverted=true". (and can be called negated NOhdr or hdrOFF in the code, like you started)

@ImprovedTube
Copy link
Member

( satus_remove ~= default? besides

#subtitles-language[data-value='default'] + .satus-switch,
html[it-theme]:not([it-theme=default]):not([it-theme=dark]) .yt-core-attributed-string__link--call-to-action-color {color: var(--yt-spec-call-to-action)}
)

@ImprovedTube
Copy link
Member

ImprovedTube commented May 1, 2024

} else if (ImprovedTube.storage.transcript === false) {
document.querySelector('*[target-id*=transcript] #visibility-button button')?.click();

(as of 6b56ee0:
satus.storage.remove(key);
parent.dispatchEvent(new CustomEvent('change'));


then need to search & replace ...storage.... === false 💭 /if\s*\(([^\)]*storage[^\)]*)\s*(=+\s*false|\!=+\s*true)\s*)\s*{/if (\!$1)/ (except for the true's by default: /if\s*\(([^\)]*storage[^\)]*)\s*(=+\s*true|\!=+\s*false)\s*)\s*{/if (\$1)/ )

@raszpl
Copy link
Contributor Author

raszpl commented May 1, 2024

( satus_remove ~= default?

yes but mostly no :) satus_remove makes the selection delete itself, so it works like user never touched that option in the first place.

#subtitles-language[data-value='default'] + .satus-switch,

1 ha! I was wondering why this code

this.dataset.value = this.value;

now I know

2 hmm #subtitles-language is never created

subtitles_language: {
component: 'select',
text: 'language',
id: 'language_closed_caption',

we create #language_closed_caption instead, looks like this entry is corrupted? its text references "language" while it should be "language_closed_caption"?
https://github.com/search?q=repo%3Acode-charity%2Fyoutube+language_closed_caption+language%3AJSON&type=code&l=JSON

so this never worked to begin with :o
probably should be

text: 'language_closed_caption', 
id: 'subtitles-language'

or CSS rule modified to reference #language_closed_caption

html[it-theme]:not([it-theme=default]):not([it-theme=dark]) .yt-core-attributed-string__link--call-to-action-color {color: var(--yt-spec-call-to-action)}

not gonna be a problem. User setting theme back to default will remove satus.storage.data.theme / ImprovedTube.storage.theme

} else if (ImprovedTube.storage.transcript === false) {
document.querySelector('*[target-id*=transcript] #visibility-button button')?.click();

💭need to search & replace ...storage.... === false

when extension is first installed(or all settings reset) ImprovedTube.storage.transcript doesnt exist at all, so this is bad code? it will only work if User first flips transcript ON then OFF. It was always broken :(

Another similar, but borderline working example is

if (ImprovedTube.storage.player_screenshot_save_as !== 'clipboard') {
var a = document.createElement('a');
a.href = URL.createObjectURL(blob); console.log("screeeeeeenshot tada!");
a.download = (ImprovedTube.videoId() || location.href.match) + ' ' + new Date(ImprovedTube.elements.player.getCurrentTime() * 1000).toISOString().substr(11, 8).replace(/:/g, '-') + ' ' + ImprovedTube.videoTitle() + (subText ? ' - ' + subText.trim() : '') + '.png';
a.click();
} else {
navigator.clipboard.write([
new ClipboardItem({
'image/png': blob
})
]);
}

it checks for ImprovedTube.storage.player_screenshot_save_as !== 'clipboard', and when you install extension
player_screenshot_save_as: {
component: 'select',
text: 'saveAs',
options: [{
text: 'file',
value: 'file'
}, {
text: 'clipboard',
value: 'clipboard'
}]

its neither clipboard nor file so it works by shear luck/someone using reverse reverse logic :)
4abaeeb flips that around to straightforward check for == 'clipboard', otherwise perform default action (file)

@ImprovedTube
Copy link
Member

ImprovedTube commented May 3, 2024

sorry, edit: #2220 (comment)

it will only work if User first flips.

yes, in the section (most of core.js) is for demonstrating the effect of features immediately.
and undoing them immediately too.

// REACTION OR VISUAL FEEDBACK WHEN THE USER CHANGES A SETTING (already automated for our CSS features):

Checking === false 17 times yet.

( one default true

if (ImprovedTube.storage.player_60fps === false) {
localStorage['it-player30fps'] = true;
)


while others could be shortened by regex

if (ImprovedTube.storage.below_player_loop !== false) {

if (this.storage.below_player_loop !== false) {


Aaand one default:true-toggle's default actually ultimately depends country
(some have the 12-hour format when written ISO 3166-1: ae,af,al,am,bd,bh,bt,co,cr,dj,do,dz,eg,eh,er,et,gh,gm,gt,hn,iq,jo,kh,kp,kw,lb,lr,ly,ma,mm,mo,mr,my,na,ni,np,om,pa,ph,pr,ps,qa,sa,sd,sl,so,ss,sv,sy,td,tn,to,us,ve,vu,xs,xz,ye)

if (satus.storage.get('use_24_hour_format') === false) {

return satus.storage.get('use_24_hour_format') === false;

return satus.storage.get('use_24_hour_format') === false;


and to be complete /mentioned before: a few Features / "Experiment Flags" might also meaningfully be stored on both sides, disable or enable - if youtube doesnt have a default like currently for comments on the side (disable Youtube's?; enable Youtube's?; our Version?). Still we don't need to store any "false" values or 3x bool, deleted when a competing one is enabled.

@raszpl
Copy link
Contributor Author

raszpl commented May 3, 2024

I dont think you can regex all changes, there are logical traps like !== 'clipboard'.

Anyway after good sleep I had an epiphany - Im going at it all wrong :). Magic 'satus_remove' value approach was wrong direction, we already have all what needed in current menu/*.js files as is, no need for additional magic values.

Ill start again, third times a charm as they say.

@raszpl
Copy link
Contributor Author

raszpl commented May 15, 2024

Ill start again, third times a charm as they say.

done #2262 . No more magic values, No more data markers. Every Satus function detects default value automatically and acts accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants