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

[Bug]: Inconsistency in color usage on watch page #6472

Open
6 tasks done
efb4f5ff-1298-471a-8973-3d47447115dc opened this issue Dec 29, 2024 · 12 comments
Open
6 tasks done
Labels

Comments

@efb4f5ff-1298-471a-8973-3d47447115dc

Guidelines

  • I have encountered this bug in the latest release of FreeTube.
  • I have encountered this bug in the official downloads of FreeTube.
  • I have searched the issue tracker for open and closed issues that are similar to the bug report I want to file, without success.
  • I have searched the documentation for information that matches the description of the bug I want to file, without success.
  • 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.

VirtualBoxVM_3J6xUJOT6q.mp4
VirtualBoxVM_gfbkIxR7Ry.mp4

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

@kommunarr
Copy link
Collaborator

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

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member Author

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 29, 2024

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

@kommunarr
Copy link
Collaborator

Yes, that theme's title color is bd93f9

@DontBlameMe99
Copy link
Contributor

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

@kommunarr
Copy link
Collaborator

kommunarr commented Dec 29, 2024

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.

@DontBlameMe99
Copy link
Contributor

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?

@kommunarr
Copy link
Collaborator

Separate bug fix PR would be ideal

@DontBlameMe99
Copy link
Contributor

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

@kommunarr
Copy link
Collaborator

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

@DontBlameMe99
Copy link
Contributor

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.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member Author

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

@kommunarr
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: To assign
Development

No branches or pull requests

3 participants