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

Show video list buttons on hover or focus #3954

Conversation

kommunarr
Copy link
Collaborator

@kommunarr kommunarr commented Aug 27, 2023

Show video list buttons on hover or focus

Co-authored by @rolandoangulo and @saideepesh000

Pull Request Type

  • Feature Implementation

Related issue

closes #1746
related to #999

Description

With this change, the Settings, Favorite, and External Player icons on a given video in an ft-list-video only display on hover (if the given device supports hovering).

Screenshots

Screenshot of this feature on a device with hover:
Screenshot_20230826_235237

Screenshot of this feature with the Device Mode devtool emulating an iPhone XR:
localhost_9080_(iPhone XR)

Testing

Tested with keyboard navigation, the Device Mode devtool, and regular use on multiple views.

Desktop

  • OS: OpenSUSE Tumbleweed
  • OS Version: 2023xxxx
  • FreeTube version: 0.19.0

…r hover/focus

As a way of cleaning up the look of videos, only shows the video icons on hover or focus. This is for all purposes a very 'free' change that I would opine makes FreeTube appear much cleaner.
Same idea as for the favorites & external player icons; much cleaner look without any sizable 'cost' to boot.
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) August 27, 2023 05:12
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 27, 2023
PikachuEXE
PikachuEXE previously approved these changes Aug 28, 2023
@PikachuEXE
Copy link
Collaborator

Wait for #3957 merged and rebase first?
Don't wanna see this merged before #3957 which seems weird

@kommunarr
Copy link
Collaborator Author

Wait for #3957 merged and rebase first? Don't wanna see this merged before #3957 which seems weird

I removed the extraneous additions from this PR three hours back, so it doesn't matter now either way.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

One thing i noticed that is not related to this PR is that i cant navigate the menu with tab or arrow keys. In this video im pressing very hard on arrow keys and when that didnt work i pressed tab but that jumped me to the next element in line

VirtualBoxVM_w9I5sKqAeU.mp4

@FreeTubeBot FreeTubeBot merged commit c32b84c into FreeTubeApp:development Aug 28, 2023
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 Aug 28, 2023
@kommunarr
Copy link
Collaborator Author

LGTM

One thing i noticed that is not related to this PR is that i cant navigate the menu with tab or arrow keys. In this video im pressing very hard on arrow keys and when that didnt work i pressed tab but that jumped me to the next element in line
VirtualBoxVM_w9I5sKqAeU.mp4

I noticed this as well a few days ago, but I thought it was because this hadn't been added yet. No idea what's causing it, blegh.

@ChunkyProgrammer
Copy link
Member

ChunkyProgrammer commented Aug 28, 2023

Arrow keys being overridden by something else, looks like the chapter navigation

PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Aug 29, 2023
* development: (65 commits)
  Make constants imported from @fortawesome/free-solid-svg-icons ordered by name again (FreeTubeApp#3958)
  Replace labeler workflow with GH labeler (FreeTubeApp#3966)
  Fix HTML styling (e.g., hashtag links) showing up as raw HTML in descriptions (FreeTubeApp#3946)
  Make certain controls non-selectable / non-draggable - Part II (FreeTubeApp#3957)
  Translated using Weblate (German)
  Show video list buttons on hover or focus (FreeTubeApp#3954)
  Bump @babel/eslint-parser from 7.22.10 to 7.22.11 (FreeTubeApp#3962)
  Bump youtubei.js from 6.0.0 to 6.1.0 (FreeTubeApp#3965)
  Bump eslint-plugin-n from 16.0.1 to 16.0.2 (FreeTubeApp#3964)
  Bump eslint from 8.47.0 to 8.48.0 (FreeTubeApp#3963)
  Bump marked from 7.0.4 to 7.0.5 (FreeTubeApp#3961)
  Bump @babel/core from 7.22.10 to 7.22.11 (FreeTubeApp#3959)
  Make certain controls non-selectable / non-draggable (FreeTubeApp#3947)
  Add updated video resolution to auto selector (FreeTubeApp#3935)
  Fix Save icon blocking issue (FreeTubeApp#3951)
  * Update URL parser to recognize youtube.com/live/xxxxxxx (FreeTubeApp#3930)
  Translated using Weblate (Czech)
  Translated using Weblate (Portuguese)
  Translated using Weblate (Polish)
  Translated using Weblate (Arabic)
  ...

# Conflicts:
#	src/renderer/main.js
#	src/renderer/scss-partials/_ft-list-item.scss
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Aug 29, 2023
* feature/playlist-2023-05: (77 commits)
  * Update add to playlist prompt to add sort options
  ! Fix user playlist view incorrectly sorted playlists by latest updated first when filtered
  ! Fix add to play prompt max width
  * Update sorting options labels
  Make constants imported from @fortawesome/free-solid-svg-icons ordered by name again (FreeTubeApp#3958)
  Replace labeler workflow with GH labeler (FreeTubeApp#3966)
  ! Fix unable to visit previous video when playing first video in a playlist
  * Update upcoming video to allow saving in playlist
  * Show filtering input & sorting element when no. of playlist > 1
  Fix HTML styling (e.g., hashtag links) showing up as raw HTML in descriptions (FreeTubeApp#3946)
  Make certain controls non-selectable / non-draggable - Part II (FreeTubeApp#3957)
  Translated using Weblate (German)
  Show video list buttons on hover or focus (FreeTubeApp#3954)
  Bump @babel/eslint-parser from 7.22.10 to 7.22.11 (FreeTubeApp#3962)
  Bump youtubei.js from 6.0.0 to 6.1.0 (FreeTubeApp#3965)
  Bump eslint-plugin-n from 16.0.1 to 16.0.2 (FreeTubeApp#3964)
  Bump eslint from 8.47.0 to 8.48.0 (FreeTubeApp#3963)
  Bump marked from 7.0.4 to 7.0.5 (FreeTubeApp#3961)
  Bump @babel/core from 7.22.10 to 7.22.11 (FreeTubeApp#3959)
  Make certain controls non-selectable / non-draggable (FreeTubeApp#3947)
  ...
@Gorrrg
Copy link

Gorrrg commented Sep 8, 2023

I think this is downgrading the user experience. It makes it very hard to see at a glance in the subscription view what videos I have previously added to the playlist. I can't possibly remember them the next day so I basically have to hover over each individual video to check whether they have been added to the playlist or not. Honestly that makes it so bothersome that I reverted back to build 3320.

As an alternative I suggest, that on non-hover you do not show the hamburger button and also not the grey star icon (i.e. if the video is not in the playlist) like before. But if a video is saved to the playlist and the cursor is not hovering over the video, then only the yellow star icon is shown. So all non-saved videos get the clean look, and the saved videos only the yellow star shows up but nothing else.

If you hover over a video, of course everything, the hamburger icon and the star icon in both states, is shown.

And of course in the playlist view it makes sense to not show the yellow star icon, because here all videos are starred by default, hence it looks messy. And if you want to remove a video from the playlist you'd hover over it anyway so the star icon is shown when you need it to.

@kommunarr
Copy link
Collaborator Author

@Gorrrg I think this is a fair concern that I can try to get to this weekend. I hope you don't hold it against me for resolving the issues as requested, because no one thought to share this feedback until now! But I think that's a reasonable proposal.

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

efb4f5ff-1298-471a-8973-3d47447115dc commented Sep 8, 2023

@Gorrrg this star save icon is going to phased out anyway when real playlist feature is going to be implemented. There will just be an add to playlist icon. Then can select multiple playlists and put the same video in those playlists if u want.

So reverting or implementing the changes u requested doesnt make sense IMO

@Gorrrg
Copy link

Gorrrg commented Sep 8, 2023

Thank you. Yeah I was a bit surprised that nobody was bothered by it to open an issue about it.

I'm not proposing to revert it, I think depending how long it will take for the new playlist system to be here for the time being, only showing the yellow star for saved videos in the subscriptions would be a good compromise.

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.

Show save icon in thumbnail on hover
6 participants