FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

[Bug]: Inconsistency in color usage on watch page

Open efb4f5ff-1298-471a-8973-3d47447115dc opened this issue 1 year ago • 14 comments

Guidelines

  • [X] I have encountered this bug in the latest release of FreeTube.
  • [X] I have encountered this bug in the official downloads of FreeTube.
  • [X] I have searched the issue tracker for open and closed issues that are similar to the bug report I want to file, without success.
  • [X] I have searched the documentation for information that matches the description of the bug I want to file, without success.
  • [X] This issue contains only one bug.

Describe the bug

There are multiple colors being used for Show more, Show less, Click to view comments and View replies.

@efb4f5ff-1298-471a-8973-3d47447115dc The page links are the --link-color variable which is just the --accent-color (Secondary Theme Color) - not sure why we have this one-time-use --link-color variable, but oh well. The comments button is --title-color which is base theme-dependent. For some reason, changing the secondary theme color is changing the rule for the comments button, which shouldn't be happening. You can inspect this more closely with the Dev Tools to see which CSS rules it's applying to figure out where that bug is coming from.

https://github.com/user-attachments/assets/befb4925-834a-41e0-82c4-a5edd9156bba

https://github.com/user-attachments/assets/965c6e62-6e41-4d6c-b7b5-a32ae627aa7f

Expected Behavior

Dont use secondary color theme for Show more, Show less, Click to view comments and View replies.

Issue Labels

inconsistent behavior, visual bug

FreeTube Version

https://github.com/FreeTubeApp/FreeTube/actions/runs/12535178475

Operating System Version

Win10 22H2

Installation Method

.exe

Primary API used

Local API

Last Known Working FreeTube Version (If Any)

No response

Additional Information

No response

Nightly Build

It looks like this is due to both of the Catppucchin themes setting their title-color to accent-color. CC: @DontBlameMe99

kommunarr avatar Dec 29 '24 16:12 kommunarr

On Dracula it looks a bit purple-ish is that expected?

Yes, that theme's title color is bd93f9

kommunarr avatar Dec 29 '24 17:12 kommunarr

Is this something that should be reverted? If yes to which colors? I would be able to do this once #6468 is merged?

DontBlameMe99 avatar Dec 29 '24 18:12 DontBlameMe99

The gist is to use what best color a link/URL/<a> would be in that theme. Look at how the Light, Dark, and Dracula themes handle that for three examples of the approaches you could take.

kommunarr avatar Dec 29 '24 19:12 kommunarr

The gist is to use what best color a link/URL/<a> would be in that theme. Look at how the Light, Dark, and Dracula themes handle that for three examples of the approaches you could take.

That makes sense. Should I change this in #6468?

DontBlameMe99 avatar Dec 29 '24 20:12 DontBlameMe99

Separate bug fix PR would be ideal

kommunarr avatar Dec 29 '24 20:12 kommunarr

After it gets merged ig? Then I'll make a PR which fixes both of them?

DontBlameMe99 avatar Dec 29 '24 21:12 DontBlameMe99

If that's what you prefer, sure! I tend to open multiple PRs and just resolve any merge conflicts based on when they're merged

kommunarr avatar Dec 29 '24 21:12 kommunarr

If that's what you prefer, sure! I tend to open multiple PRs and just resolve any merge conflicts based on when they're merged

I could also do this I guess. I thought one PR doing this after it has been merged would be more elegant. But if you think otherwise I can do this as well.

DontBlameMe99 avatar Dec 29 '24 21:12 DontBlameMe99

tbh you can do whatever you prefer. Small PR's changes like this one will probably get merged faster than a whole theme

Related issue that we should switch to the link-color for the About page links in the same effort due to it being the only apparent instance of mixing the theme colors with a bg-color background, thus failing color contrast issues. Such a fix should check that this succeeds in achieving a sufficient color contrast in this case, otherwise another solution will have to be pursued. More information here.

kommunarr avatar Dec 30 '24 16:12 kommunarr

This issue is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jan 28 '25 01:01 github-actions[bot]

This issue is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Mar 04 '25 02:03 github-actions[bot]

This issue is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Apr 02 '25 02:04 github-actions[bot]

Hi @DontBlameMe99 i forgot to follow up on this after your most recent PR was merged. Would it be possible for you to take a look at this issue?

Hi @DontBlameMe99 i forgot to follow up on this after your most recent PR was merged. Would it be possible for you to take a look at this issue?

Yes, see #7141

DontBlameMe99 avatar Apr 03 '25 10:04 DontBlameMe99