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

Update themes.js no more cookie, no more [dark] dependence #2174

Merged
merged 4 commits into from
May 6, 2024
Merged

Update themes.js no more cookie, no more [dark] dependence #2174

merged 4 commits into from
May 6, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Apr 9, 2024

no more cookie, no more [dark] dependence
text-inverse should be different from text, related to #2150

@ImprovedTube
Copy link
Member

  • then our "dark" & "default" buttons do nothing anymore and shall be removed or summed up as one "none"?
    • or we can have a 3-ways switch for 3 options (Youtube's dark OR light OR system-default), just like youtube already has as said in Extension disabling Youtube dark mode #2138, since cookie code is ready and editing it once seems enough.

related /BTW: idk why, when a theme is set already, then our dark theme shortcut might only makes the search field black (

ImprovedTube.shortcutDarkTheme = function () {
) - it could just stay youtube's dark or our black - or the previously selected opposite light / dark as for #2161

@raszpl
Copy link
Contributor Author

raszpl commented Apr 9, 2024

  • then our "dark" & "default" buttons do nothing anymore and shall be removed or summed up as one "none"?
    • or we can have a 3-ways switch for 3 options (Youtube's dark OR light OR system-default), just like youtube already has as said in Extension disabling Youtube dark mode #2138, since cookie code is ready and editing it once seems enough.

Can be reworked to 3 separate entries in Themes:
1 "none/no change/default/disabled" left as is doing nothing. The default state with extension just installed.
2 "YT dark" forcing YT Dark theme without touching cookies
3 "YT light" forcing YT Light theme without touching cookies

After all the cookie trouble its safest to just remove that code altogether and do all overrides with pure CSS.
Currently 2 and 3 dont work at all yet, but its easy to fix that.

related /BTW: idk why, when a theme is set already, then our dark theme shortcut might only makes the search field black (

I havent touched shortcuts so no idea whats wrong there. After themes are fixed and solid shortcutDarkTheme should just switch themes without doing anything beyond. Can even let user configure two themes it would be switching between, same goes for #2161

@raszpl
Copy link
Contributor Author

raszpl commented Apr 9, 2024

btw

if ( this.storage.theme === 'custom' && Array.isArray(this.storage.theme_primary_color) && Array.isArray(this.storage.theme_text_color)) {

check for Array.isArray(this.storage.theme_primary_color) && Array.isArray(this.storage.theme_text_color) is not required, because
primary_color = 'rgb(200, 200, 200)';

text_color = 'rgb(25, 25, 25)';

but we need a better place to store default Custom theme colors, currently its also in here
value: [200, 200, 200]

value: [25, 25, 25]

ideally it should be in one central place. Also default Custom theme colors suck a little :) its just black and gray making it look just gray while Theme "Custom" button is Red :o Would be nice to make the "Custom" button reflect currently selected custom colors.

@ImprovedTube ImprovedTube force-pushed the master branch 2 times, most recently from 62a36d2 to 40aedd4 Compare April 27, 2024 17:02
@ImprovedTube ImprovedTube merged commit c91976e into code-charity:master May 6, 2024
@raszpl raszpl deleted the patch-10 branch May 6, 2024 23:36
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