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

more CSS cleanups #2073

Merged
merged 20 commits into from
Mar 7, 2024
Merged

more CSS cleanups #2073

merged 20 commits into from
Mar 7, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Mar 5, 2024

formatting, spaces, uncovered several typos and errors hiding in tightly bundled definitions, validated with http://csslint.net

@ImprovedTube
Copy link
Member

ImprovedTube commented Mar 6, 2024

Thank you! Appreciate removing spaces & adding tabs

Adding line breaks and spaces:

  • if you wanted to adjust the following font-sizes or margins for example,
    aren't fewer line breaks more efficient?
  • More importantly: It might never be edited again, or once in many years,
    meanwhile the file might be scrolled through many times, so the wasted scrolling kilometers are often more relevant

css linebreaks

@raszpl
Copy link
Contributor Author

raszpl commented Mar 6, 2024

Whole point of more lines is making it human readable. The way it was done with cramming everything as tight as possible lead to several typos and bugs I found along the way.
HTML/JS/CSS files are not loaded and stored in ram as is. There is no point "optimizing" by removing CR LF from a file loaded once from local disk and immediately turned into binary representation in browsers ram.

If you are worried about performance look into all very slow CSS rules used for theming. All the ones with wildcards are very expensive. Throwing CSS files into http://csslint.net/ will give you hints on what to look for.

@raszpl
Copy link
Contributor Author

raszpl commented Mar 6, 2024

this patch still needs few tweaks, I broke something in the process of copy&paste and themes stopped working :)

edit: nvm the patch is fine, some themes are just so weird I thought I broke it and panicked :]

@ImprovedTube
Copy link
Member

errors

saw them 👍🏿

themes... wildcards

👍🏿
#1634 (comment)

the themes' author also once prepared for more custom colors:
https://github.com/code-charity/youtube/blob/master/js%26css/web-accessible/www.youtube.com/themes.js


readability

(i know the standard, but challenge it, at least for css & json.
but) can't confirm compact views "cause" errors. (is it just me?)

In conclusion readability of simple code structures could be re-defined in the world considering scrollheight & 2 dimensions

  • So, more specifically, since i started this topic before 😂:

(CSS, JSON, ...) : "Excessive scroll heights (300%+)" = "human readability" ?

  • As of scroll-height, Humans don't like/tend to edit JSON or CSS (a lot of time is spent just scrolling.)
  • Humans might only need to mind or edit the actually values. ( since syntax-debugging can be automated, humans work isn't required. ) ( Besides it's more likely to see a bug as human when it is not scrolled away already ) (Programmer should at least have something like tab-key = skip to next css or json value)
  • People might want to actually use their vital ability to look left & right AND diagonally with nested structures (instead of up & down only ever. ( Imagine someone's full-time job is to generate CSS with chatGPT and review the values. You might chose compact view and only scrolling 1km per day instead of several )
  • Specifically a lot is structured in 2-dimensions (example above) (besides some of it could be replaced by arrays and loops)
    • This should be formatted automatically with values aligned above each other as multiple columns, and editable with arrow keys like a spreadsheet.
  • The world's data format should be a combination of objects and tables already...

+("Philosophy":) no "non-required regular single spaces" should be edited by humans in code all the time. (Not worth the human time, nor storage.) (Editing tools can just add them by each individual viewer's preference (utopia. You'd state the left- and right-margin of specific chars like { and } and the color (or inherit your defaults from another language). On the scale of the world, that would still save some co2, even in efficient computers )

@raszpl
Copy link
Contributor Author

raszpl commented Mar 6, 2024

but) can't confirm compact views "cause" errors. (is it just me?)

other than my last three pull request for bugs hiding in compacted code? :) Its bad and counterproductive. Computers no longer work the way this would make any practical sense to cram text tight for speed. I challenge you to measure any difference in speed or ram usage between the two.

All that you wrote is contradictory to numerous UI studies. What is "Excessive scroll height"? Used 24" monitors are ~$30-50, turn it 90 degrees and you have >100 lines of text at once. I dont know any programmers complaining about scrolling. It takes 2-5x longer to read and understand bad code.

@ImprovedTube ImprovedTube merged commit f54e972 into code-charity:master Mar 7, 2024
@ImprovedTube
Copy link
Member

ImprovedTube commented Mar 7, 2024

hi @raszpl! sorry😃, i think in this optional topic it is the 3rd? time (since 2023), that i made you react about computing speed. (Which i didn't mean) and i guess this could be in chat (need not fill your PR-threads)

Lets just make a blockly with a minimal theme; added spreadsheets blocks and 3D-navigation.

yes, i like info-graphics & monitors square-shaped, making use of 2 dimensional space, with the vain wish for logical structures. (Rare to be found)(With Gradients of from details to overview). Or for code: from critical to never-to-be-edited-again (such prediction could save a lot of developer frustration)

Short lines are more motivating to start at all in the first minutes. But if ever scrolling over megabytes of CSS or so in a row, then the short lines can equally be painful, unless the viewer likes scrolling as an e-sport
~= UI-principles: "Little/tidy is easy" vs. "content pixels are payload; empty pixels are waste (can distract or exhaustion if they aren't black")

So I can't think short lines are a holy grail making brains run "2-5 times" faster. (Unless the assumption would include many proofreaders searching syntax errors which should be auto-discovered)

All that you

bad code

Uncaught TypeError : Cannot Read Properties of Undefined

ImprovedTube added a commit that referenced this pull request Mar 7, 2024
@raszpl raszpl deleted the css3 branch March 7, 2024 15:33
@raszpl
Copy link
Contributor Author

raszpl commented Mar 8, 2024

If you are worried about performance look into all very slow CSS rules used for theming. All the ones with wildcards are very expensive. Throwing CSS files into http://csslint.net/

for more details watch https://nolanlawson.com/2023/01/17/my-talk-on-css-runtime-performance/ Theming currently is full of worst case scenario selectors :( Crimes in order of bad:

  • injecting all of them, even not used themes
  • [data-page-type='video'][it-player-size='fit_to_window'] ytd-app:not([player-fullscreen_]) ytd-watch-flexy:not([theater]):not([fullscreen]) Attribute Selector with value with nothing preceding it - this forces browser to keep applying rule to every single div added by the page. I fixed this one in this patch.
  • html[it-ads=subscribed_channels] [target-id='engagement-panel-ads'] Attribute Selector with value, wildcard AND Attribute Selector with wildcard value
  • html[data-system-color-scheme=dark][it-theme=black][it-schedule=system_peference_dark]:not([style-scope]):not(.style-scope) *:not([style-scope]):not(.style-scope) three attribute selectors by value and a wildcard
  • html[it-pathname*="/shorts/"] "just" attribute selector with wildcard value
  • html[data-page-type='video'][it-player-fit-to-win-button='true'] "just" attribute selectors with value

The thing with CSS that is not very intuitive is even the rules you dont use, or are very rarely applied slow down whole page at all times. Ideally we should break Themes apart into individual themexxxx.css files and only load appropriate one. Get rid of attribute selectors, and especially attribute selectors by value.

But if ever scrolling over megabytes of CC or so in a row, then the short lines can equally be painful, unless the viewer likes scrolling as an e-sport

professional programmers dont scroll :) /, ctrl-s or ctrl-f or grepping is all you need. Not to mention pagedown scrolls by >100 lines.

can distract or exhaustion if they aren't black")

use dark mode in your editor :-)

So I can't think short lines are a holy grail making brains run "2-5 times" faster.

Its not the length of line, its keeping logically separate things in separate lines. This:

setTimeout(function(){try{document.querySelectorAll("tp-yt-iron-dropdown").forEach(el => el.style.opacity = 0); document.querySelector('tp-yt-iron-dropdown svg path[d^="M13.18,4l0.24,1.2L13.58,6h0.82H19v7h-5.18l-0"]').closest("tp-yt-paper-item").click();}
catch{console.log("report failed");setTimeout(function() {try{document.querySelector('tp-yt-iron-dropdown svg path[d^="M13.18,4l0.24,1.2L13.58,6h0.82H19v7h-5.18l-0"]').closest("tp-yt-paper-item").click();}
catch{console.log("report failed2");document.querySelector('svg path[d^="M7.5,12c0,0.83-0.67,1.5-1.5"]').closest("button").click();}},800);
}
},200);

is a crime, and it hides a bug that is not obvious because you cant see anything in a bungled mess :)

(Unless the assumption would include many proofreaders searching syntax errors which should be auto-discovered)

not syntax, logical errors. This is what cramming things close together leads to:
https://github.com/code-charity/youtube/pull/2071/files
Someone stuffed two selectors into single line. "easy" error to find only if you read whole line and know what its doing. Using good practices like keeping everything in separate lines would prevent this from ever happening in the first place. I was so mentally exhausted when fixing it I left it "as is" crammed into single line, leaving more problems for the next guy :)

@raszpl
Copy link
Contributor Author

raszpl commented Mar 8, 2024

Another minor crime, cant tell on first glance what is going on because indentation is all wrong:

ImprovedTube.myColors = function () {
if (
this.storage.theme === 'custom' &&
Array.isArray(this.storage.theme_primary_color) &&
Array.isArray(this.storage.theme_text_color)
) {
var style = this.elements.my_colors || document.createElement('style'),
primary_color = this.storage.theme_primary_color,
text_color = this.storage.theme_text_color;

Can you tell whats happening in the next one?

this.elements.my_colors = style;
document.documentElement.appendChild(style);
(document.documentElement.hasAttribute('dark') !== null && document.documentElement.hasAttribute('dark') )
{document.documentElement.removeAttribute('dark');}
if(document.getElementById("cinematics") !== null)
{document.getElementById("cinematics").style.visibility = 'hidden';
document.getElementById("cinematics").style.display = 'none !important';}
if(document.querySelector('ytd-masthead') !== null)
{document.querySelector('ytd-masthead').style.backgroundColor = ''+primary_color+''; }

Crime scene, a bug is hiding in there because:

its fixed by #2076 :)

@raszpl
Copy link
Contributor Author

raszpl commented Mar 8, 2024

Another good coding style template https://peps.python.org/pep-0008/
https://peps.python.org/pep-0008/#should-a-line-break-before-or-after-a-binary-operator would have made

ImprovedTube.myColors = function () {
if (
this.storage.theme === 'custom' &&
Array.isArray(this.storage.theme_primary_color) &&
Array.isArray(this.storage.theme_text_color)
) {
var style = this.elements.my_colors || document.createElement('style'),
primary_color = this.storage.theme_primary_color,
text_color = this.storage.theme_text_color;

a little bit more readable

@ImprovedTube ImprovedTube added the Knowledge Base / Dokumenation for developers We should repurpose this for future reference / Wiki / Education / Introduction label Mar 9, 2024
@ImprovedTube
Copy link
Member

good coding style template peps.python.org/pep-0008
peps.python.org/pep-0008/#should-a-line-break-before-or-after-a-binary-operator

👍👍 hi! @raszpl

Ideally we should break Themes apart into individual themexxxx.css files and only load appropriate one.

( and themes include many duplicate lines same for all 6 of them (as mentioned) )

CSS performance:

yes, while its convenient to allow tweaks with no JS (could even crowdsource/vote about them for any website /ublock origin could), we should avoid wastes of electricity even if minuscule per person. A relief, the mechanism for this (querying attributes of the element), should be performant. (it is available before the page is rendered.)

  • CSS allows nesting nowadays like SCSS & SASS used to: https://caniuse.com/css-nesting, so that we we can wrap blocks in a single checks
  • we can benchmark to point out specifically how a html[xyz="true"] compares to a javascript if(a) and how many unnecessary times it might re-run.

just for fun: #2039

@raszpl
Copy link
Contributor Author

raszpl commented Mar 11, 2024

Nesting doesnt do anything to performance https://medium.com/developer-rants/the-problem-1f121f6aecbf Its just a different way of declaring selectors. Its purely for readability, browser derives same rules when parsing.

html[xyz="true"] compares to a javascript if(a)

its not about replacing CSS selectors with javascript. Its about not using the slowest selectors possible, so instead of
html[it-theme=black] #cinematics,
and stuffing Attribute Selector with value (thats "it-theme=black") we put good old "it-theme-black" class on main Body element and declare our selector as
.it-theme-black #cinematics,
then switching themes is performed by swapping one class on Body element. Google hates users, google devs were already using 16core Xeon 64GB workstations 7 years ago when designing new YT layout, so they dont even notice when something is slow, thats why they did theming using Attribute Selectors () and didnt notice making same mistake I talked about earlier:

html[darker-dark-theme], [darker-dark-theme] [light] {
    --yt-spec-text-primary: #0f0f0f;
    --yt-spec-text-primary-inverse: #fff;
}

another explanation is https://www.zdnet.com/article/former-mozilla-exec-google-has-sabotaged-firefox-for-years/ Maybe they used slowest possible selectors to screw with Firefox knowing they work faster on Chrome.

just for fun: #2039

On first glance its a good patch, havent tested it yet

@ImprovedTube
Copy link
Member

ImprovedTube commented Mar 12, 2024

not about replacing CSS selectors with javascript

yes, just the one question remaining after tiding up the rest

re-run

and we can count/benchmark.
And load themes conditionally as you said
or finish the custom color sets.
ori can remove the duplicates already as said

nesting

For readability. +It could be processed faster if there was a nested block with 1000 children. Compared to a random unsorted order. Is looking at browser code on your bucket list yet?😅

utopia. You'd state the left- and right-margin of specific chars like { and } and the color (or inherit your defaults from another language). On the scale of the world, that would still save some co2, even in efficient computers )

just for fun: #2039

good patch

sorry, i just meant spaces still🤣
if beautification would be part of editing-tools only,
then some bandwidth would be saved. (just one thought about the world.)
(Not trying to complicate your work!!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Knowledge Base / Dokumenation for developers We should repurpose this for future reference / Wiki / Education / Introduction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants