Skip to content

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

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

Closed
6 tasks done
efb4f5ff-1298-471a-8973-3d47447115dc opened this issue Dec 29, 2024 · 17 comments · Fixed by #7141
Closed
6 tasks done

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

efb4f5ff-1298-471a-8973-3d47447115dc opened this issue Dec 29, 2024 · 17 comments · Fixed by #7141
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.

Copy link
Contributor

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.

Copy link
Contributor

github-actions bot commented Mar 4, 2025

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.

Copy link
Contributor

github-actions bot commented Apr 2, 2025

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.

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

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?

@DontBlameMe99
Copy link
Contributor

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

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

Successfully merging a pull request may close this issue.

3 participants