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

Added Button to Show Original Title and Thumbnail for DeArrow #6164

Open
wants to merge 5 commits into
base: development
Choose a base branch
from

Conversation

JL0000
Copy link

@JL0000 JL0000 commented Nov 14, 2024

Added Button to Show Original Title and Thumbnail for DeArrow

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

closes #3900

Description

If DeArrow is enabled for title or/and thumbnail, when hovering over the video info area, a button is added next to the video title when the video is part of a list. This button switches the title or/and thumbnails back to its original version and vice versa.

  • The smaller button suggests that the title content did not change (case insensitive).

Video

after.mov

Testing

  • Button doesn't show up when DeArrow is not enabled
  • The button cannot be pinned if DeArrow did not change anything.

Desktop

  • OS: MacOS
  • OS Version: 15.1
  • FreeTube version: v0.22.0 Beta

Additional context

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 14, 2024 00:12
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 14, 2024
@PikachuEXE
Copy link
Collaborator

Even though it feels kind of working for desktop
Reducing padding for buttons is bad for mobile users (touch screen users more precisely)
Functionality works but UI needs to be improved but I have no idea what would be good

@absidue
Copy link
Member

absidue commented Nov 14, 2024

I agree with @PikachuEXE here, maybe it would be better suited inside the dropdown?

@JL0000
Copy link
Author

JL0000 commented Nov 14, 2024

Didn't know it was available on mobile. What's the best way to test on mobile?

@JL0000
Copy link
Author

JL0000 commented Nov 14, 2024

@absidue I think putting it in the dropdown menu would make it too cumbersome to use this feature.

@absidue
Copy link
Member

absidue commented Nov 14, 2024

If you don't want to move it to the dropdown where the rest of the options are then you'll have to stick with your current solution but undo the change to shrink the buttons.

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

I think that we shouldn't put this in the dropdown and keep this as is.

Some differences im seeing compared to #3900 (comment)

title moves around on hover

VirtualBoxVM_uhd6mYOPQa.mp4

button has different sizes

VirtualBoxVM_48KHtSzYNe.mp4

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Nov 15, 2024
auto-merge was automatically disabled November 15, 2024 11:22

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 15, 2024 11:22
@JL0000
Copy link
Author

JL0000 commented Nov 15, 2024

@efb4f5ff-1298-471a-8973-3d47447115dc I see the title also moving on hover in #3900 (comment)? About the button size, in the DeArrow extension there are two different button sizes.

brave.mov

@JL0000
Copy link
Author

JL0000 commented Nov 15, 2024

I have changed back the button padding to its original size.

Thinking of adding a new setting in the DeArrow setting to enable this toggle to see original title/thumbnail.

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

I think https://fontawesome.com/icons/circle-dot?f=classic&s=solid is a better representation of the DeArrow button that is used on the YT side.

Dot enabled color should be based on the secondary color theme
Dot disabled should be grey

firefox_s0pK2XtSBu.mp4

Title moves around on hover

VirtualBoxVM_GJQZKbc0BW.mp4

Doenst hover here

firefox_w2XRWfIjM2.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Reveal original title when using DeArrow
4 participants