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

Themes switching fixed #2075

Closed
wants to merge 7 commits into from
Closed

Themes switching fixed #2075

wants to merge 7 commits into from

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Mar 6, 2024

fixed theme switching, it was totally borked for as long as I remember

raszpl added 6 commits March 6, 2024 18:27
fixing theme switching, it was totally borked for as long as I remember
formatting
fixing theme switching
another typo caused by bunched up code
rewritten for readability
redundant call
@ImprovedTube
Copy link
Member

ImprovedTube commented Mar 7, 2024

hi @raszpl

what is cinematics??

#1967 #1892

cookie

#93 #507

// FEEDBACK WHEN THE USER CHANGED A SETTING

this section is another place where a compact table-like (horizontally parallel) view was chosen since each unit is so short/similar 😃😃. ( if multiple are completely the same, then redundant code could be shortened-away. That's why 🤔 I might think of making it compact as a natural step in between, just to switch from detail-view to overview or indicate that it could have fewer characters)

@raszpl
Copy link
Contributor Author

raszpl commented Mar 7, 2024

this is still in progress, got few things to tie up

@@ -83,7 +83,7 @@ ImprovedTube.init = function () {

if (window.matchMedia) {
document.documentElement.dataset.systemColorScheme = window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light';
} ImprovedTube.myColors();
Copy link
Member

Choose a reason for hiding this comment

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

why

@ImprovedTube
Copy link
Member

Looking forward. Didn't check themes.js changes yet.

@raszpl
Copy link
Contributor Author

raszpl commented Mar 7, 2024

#2076 fixed patch, switching themes finally works between all of them. Last time I tried I think a year ago I couldnt switch from dark back to default without reloading page, now fixed.
Some themes are weird or broken, like Desert - is it supposed to be blue yellow? because its only blue and white.
Also Themes dont change color of Comment field background.

what is cinematics??

#1967 #1892

ok not touching that :)

cookie

#93 #507

Code modifying F6 cookie has been broken for a long time here and in shortcuts.js, Im guessing a ton of users have corrupted cookie because of it :( Whats worse its only able to assign 200 to F6 and cant roll it back.
I just checked and at least on my account on Chrome setting F6 to 400 or 80000 does nothing, no change. google might have killed it.
Shortcut switching to dark mode is also super janky. I dont think it works.

// FEEDBACK WHEN THE USER CHANGED A SETTING

this section is another place where a compact table-like (horizontally parallel) view was chosen since each unit is so short/similar 😃😃. ( if multiple are completely the same, then redundant code could be shortened-away. That's why 🤔 I might think of making it compact as a natural step in between, just to switch from detail-view to overview or indicate that it could have fewer characters)

While it might look aesthetically pleasing from far away it turns this into jumbled unreadable mess, this is exactly where I found a typo :)
Good read on how to write clean readable code to avoid common typos and be able to easily spot bugs: https://www.kernel.org/doc/html/v4.10/process/coding-style.html
Editor with ESLint wont hurt either.

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