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

fix: #1732 #1734

Merged
merged 3 commits into from
Aug 15, 2023
Merged

fix: #1732 #1734

merged 3 commits into from
Aug 15, 2023

Conversation

vinay-nb
Copy link
Contributor

fixed coloring issue for description hyperlinks

@PoorChameleon
Copy link

Unfortunately this doesn't work because the line above this overrides everything else, probably because of the !important property.

.yt-core-attributed-string--link-inherit-color {color: var(--yt-spec-text-primary) !important}

I don't really understand the reason for this line existing anyway, or especially why it would need to have !important

removed the !important
@PoorChameleon
Copy link

Now it works but it's because the inherited style is overriding the non !important css styling. I think these lines should just not exist since they dont do anything unless using !important where they then override everything else.

So basically the original 581 line should just be removed and nothing new added imo.

Unless I'm missing something?

@vinay-nb
Copy link
Contributor Author

vinay-nb commented Aug 13, 2023

You are right, but is it going to work in the dark theme?

@PoorChameleon
Copy link

PoorChameleon commented Aug 13, 2023

Works for me on all themes, the colors the stay the same with the lines removed (white for text and blue~ for links).

However in very light themes the white text itself is quite hard to see, but this should be fixed with some theme specific variables or something so it doesn't affect everything else.

No clue why the original commit was made

For future reference here's black and desert themes. These are with 581 and the new lines removed. The shading for blue changes here, but it doesn't seem to have anything to do with having 581-584 or not.
black-desert

removing the styling for video description text color
@ImprovedTube
Copy link
Member

hi @vinay-nb! @PoorChameleon
thank you!

@ImprovedTube ImprovedTube merged commit ecae55c into code-charity:master Aug 15, 2023
ImprovedTube added a commit that referenced this pull request Aug 15, 2023
@ImprovedTube
Copy link
Member

am i making sense? 8297c1f :

/video detail text color./
.yt-core-attributed-string--link-inherit-color {color: var(--yt-spec-text-primary) !important}

/video detail text color. Fix. video detail text color was wrong in black theme/
[it-theme=black] .yt-core-attributed-string--link-inherit-color:not(a) {color: var(--yt-spec-text-primary) !important}

@dodieboy

@dodieboy
Copy link
Member

Yup, this should fix the problem. I will double check this tmr. Mostly likely youtube change the code again.

@PoorChameleon
Copy link

/video detail text color. Fix. video detail text color was wrong in black theme/ [it-theme=black] .yt-core-attributed-string--link-inherit-color:not(a) {color: var(--yt-spec-text-primary) !important}

Unfortunately this selector still affects links too (black theme)
wGcch7

@vinay-nb
Copy link
Contributor Author

Hi, @PoorChameleon Did you check from the master?

@PoorChameleon
Copy link

Yes
wwLer3

@vinay-nb
Copy link
Contributor Author

ok I will try to fix it

@PoorChameleon
Copy link

PoorChameleon commented Aug 19, 2023

Okay so I did some testing and something like this might work, but it needs to be tested thoroughly.

/*video detail text color. Fix. video detail text color was wrong in black theme*/
[it-theme=black] .yt-core-attributed-string--link-inherit-color:not(a) {color: var(--yt-spec-text-primary) !important}

[it-theme=black] .yt-core-attributed-string--link-inherit-color .yt-core-attributed-string__link--call-to-action-color, .yt-core-attributed-string__link--call-to-action-color {color: var(--yt-spec-call-to-action);}

The next two lines below can be removed if exact theming consistency isn't the goal and we just want to keep it simple.

[it-theme=black] a.yt-simple-endpoint.yt-formatted-string {color: var(--yt-spec-call-to-action) !important;}
Actually this above line is wrong. Default behavior is to have #tags near the views and upload date have two different colors depending if the video detail is expanded or not, but I don't know how to replicate that.

[it-theme=black] yt-formatted-string[has-link-only_]:not([force-default-style]) a.yt-simple-endpoint.yt-formatted-string {color: var(--yt-spec-text-primary) !important;}

Either YT changes things really often or something, because now the commit way earlier worked. Not sure what is going on.

Assuming that this works the desert theme text color could be changed too, maybe to something a bit darker.

Note that it's also very likely that this has some redundant styling, but please test thoroughly before removing or changing something so that the colors stay consistent across themes.

@vinay-nb
Copy link
Contributor Author

vinay-nb commented Aug 21, 2023

Hi @PoorChameleon

Is adding the !important necessary?

Also please let me know the steps to test it locally

@PoorChameleon
Copy link

!important is probably not necessary if only doing the top two lines. However with the lower ones they start applying or not applying on the wrong elements so having the flag kind of fixes it, but it's very messy.

For testing you can just paste each of the lines in for example a stylus userstyle that points to YouTube. Or alternately load the extension in dev mode with those changes applied.

The lower two lines aren't really finished so you'd have to test more and probably change them.

@PoorChameleon
Copy link

Okay here are mostly tested and probably working rules, they can be adapted to any theme by changing the value after
it-theme= (default is just default)

[it-theme=black] .yt-core-attributed-string--link-inherit-color {color: var(--yt-spec-text-primary)}

[it-theme=black] .yt-core-attributed-string--highlight-text-decorator .yt-core-attributed-string__link--display-type {color: var(--yt-spec-text-primary)}

[it-theme=black] .yt-core-attributed-string__link--call-to-action-color {color: var(--yt-spec-call-to-action);}

Keep in mind the description colors can still be hard to see on some themes, so the rgb values might have to be changed accordingly for each theme

@ImprovedTube
Copy link
Member

check: #1787
hi again @vinay-nb and @PoorChameleon and thank you so much for caring! (do you even make pull requests?)

@ImprovedTube
Copy link
Member

ImprovedTube commented Jan 4, 2024

...

please test thoroughly

enough thought went into it to make it better rather than worst. Sorry we didn't upload August 28 @PoorChameleon (or earlier. before publishing, we can double check for syntax errors breaking something)

do you even make pull requests?

Wanna? 😆😳 (you belong in the contributors list)

Actually this above line is wrong. Default behavior is to have #tags near the views and upload date have two different colors depending if the video detail is expanded or not, but I don't know how to replicate that.

youtube uses pseudo html attributes there is-expanded & hidden
#description-inline-expander[is-expanded] yt-attributed-string:not([hidden])
#description-inline-expander:not([is-expanded]) yt-attributed-string[hidden]

...now the commit way earlier worked. Not sure what is going on.

(possibly(?) it just looked like, when you clicked dark mode too in between? which is not a theme but changing Youtube's cookie)

Either YT changes things really often or..

hopefully not, however we can chose strategically maybe:

  • Short names might be good already / less likely anybody wants to change same.
  • Names at low levels are less likely/more troublesome to change.
just some alternative selectors:
/* text color */ 
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  #description-inline-expander  yt-attributed-string,
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  #description-inline-expander .ytd-text-inline-expander,
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  ytd-text-inline-expander:first-of-type .ytd-text-inline-expander,
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  ytd-text-inline-expander:first-of-type yt-attributed-string
{color:var(--yt-spec-text-primary)}
  /*(undo) Link color */ 
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  #description-inline-expander  yt-attributed-string a,
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  #description-inline-expander .ytd-text-inline-expander a,
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  ytd-text-inline-expander:first-of-type .ytd-text-inline-expander a,
 [it-theme]:not([it-theme=default]):not([it-theme=dark])  ytd-text-inline-expander:first-of-type yt-attributed-string a 
{color:#0073e6!important; color:var(--yt-spec-call-to-action)!important;}
        /*(YouTube's default link color, if the variable ever isn't defined?)*/

ImprovedTube added a commit that referenced this pull request Jan 5, 2024
ImprovedTube added a commit that referenced this pull request Jan 12, 2024
ImprovedTube added a commit that referenced this pull request Jan 12, 2024
ImprovedTube added a commit that referenced this pull request Jan 12, 2024
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.

None yet

4 participants