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

Use more generic icon for Quick bookmark feature #4885

Conversation

MarmadileManteater
Copy link
Contributor

@MarmadileManteater MarmadileManteater commented Apr 6, 2024

Use more generic icon for Quick bookmark feature

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Description

The quick bookmark feature currently uses a clock as its icon. This makes sense for if the quick bookmark playlist is set to Watch Later, but it can be changed to different playlists such as Favourites. Because of this, I believe it makes sense for the icon for quick bookmark to be more generic. I believe the bookmark icon is a great candidate because it is already used in reference to the Playlists view.

Screenshots

before after
image image
before after
image image

Desktop

  • OS: Windows 10
  • OS Version: Pro Version 22H2 Installed on ‎4/‎3/‎2022 OS build 19045.4170 Experience Windows Feature Experience Pack 120.2212.4180.0
  • FreeTube version: 8b16bd5

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 6, 2024 03:23
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 6, 2024
@kommunarr
Copy link
Collaborator

suggestion: Another concern that was mentioned was the color not having enough of a contrast with most themes to be visually remarkable. Thoughts on (re-)sprucing it?

@MarmadileManteater
Copy link
Contributor Author

suggestion: Another concern that was mentioned was the color not having enough of a contrast with most themes to be visually remarkable. Thoughts on (re-)sprucing it?

It could be the primary theme colour:
image

image

image

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

Would the secondary theme color confusing?

@MarmadileManteater
Copy link
Contributor Author

Would the secondary theme color confusing?

I don't think it would be confusing. That would make it the same colour as the buttons it is next to on the watch page, except inverted as far as the icon and button are concerned.

image

However, I personally think the primary colour pops a little more next to the other secondary colour buttons.

absidue
absidue previously approved these changes Apr 10, 2024
@PikachuEXE
Copy link
Collaborator

I doubt using primary color is a good idea though
I wonder why all videos in my history were bookmarked already
It's not the primary action (like an OK button on a confirmation prompt) so I see no reason to use a different color on the icon
Also it looks strange in my color scheme at least

image

@MarmadileManteater
Copy link
Contributor Author

I doubt using primary color is a good idea though I wonder why all videos in my history were bookmarked already It's not the primary action (like an OK button on a confirmation prompt) so I see no reason to use a different color on the icon Also it looks strange in my color scheme at least

image

I admit it does look like the video is already bookmarked especially considering the fact that, previously, a lack / presence of colour was used to denote the state of that button.

I like how it looks without colour, but I was trying to make it pop more.

@kommunarr
Copy link
Collaborator

I liked that the old system of theme-specific favorites icon colors ensured a higher contrast with the given theme regardless of primary/secondary color choices. Using the primary/secondary colors for this is tough because it puts it in visual competition with the other primary/secondary-colored controls we have throughout the app. So 1. that, 2. increasing the actual icon size (as done here), and 3. (in the case of the video overlay version) increasing the opacity would resolve the problem with this feature that users are currently having in my opinion.

@MarmadileManteater
Copy link
Contributor Author

I liked that the old system of theme-specific favorites icon colors ensured a higher contrast with the given theme regardless of primary/secondary color choices. Using the primary/secondary colors for this is tough because it puts it in visual competition with the other primary/secondary-colored controls we have throughout the app. So 1. that, 2. increasing the actual icon size (as done here), and 3. (in the case of the video overlay version) increasing the opacity would resolve the problem with this feature that users are currently having in my opinion.

what do you think of this?
image

@PikachuEXE
Copy link
Collaborator

I think there is no need to change the icon size, the opacity change is good enough
image

Also it would introduce a visual issue in playlist
image

@kommunarr
Copy link
Collaborator

It looks really good now! My final suggestions would be 1) to create a new opacity just for this case rather than overriding the value of $thumbnail-overlay-opacity for its other use cases (e.g., in watched styling) and 2) to try out the size increase in watch-video-info too.

Copy link
Member

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great teamwork! :)

@FreeTubeBot FreeTubeBot merged commit 1dde533 into FreeTubeApp:development Apr 11, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 11, 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.

7 participants