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

Download option removed from channels/playlists #6385

Merged
merged 1 commit into from
May 28, 2021

Conversation

sauravrao637
Copy link
Contributor

@sauravrao637 sauravrao637 commented May 27, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • The Download button should not be included in option action choices when linkType is CHANNEL or PLAYLIST (as it will just give unsupported url ) , thus done the needed changes

Fixes the following issue(s)

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

@sauravrao637
Copy link
Contributor Author

From what I can understand, for linkType CHANNEL download is completely redundant and thus should not be shown as an option and till "bulk Download" is pending we should rely on this and when it's available we can put the option back for linkType PLAYLIST (I'll myself add back as soon as the feature is available)
And by "'Show info' option only for playlists" ,I think this should be an option for CHANNELS as well as for any STREAM, as user may only want to see say "Description" of a video
If I've misinterpreted , I would like to be further assisted with the issue :)

@SameenAhnaf
Copy link
Collaborator

From what I can understand, for linkType CHANNEL download is completely redundant and thus should not be shown as an option

Your points are quite valid and logical. So, this PR still makes sense.

and till "bulk Download" is pending we should rely on this and when it's available we can put the option back for linkType PLAYLIST (I'll myself add back as soon as the feature is available)

Thank you so much for your help in advance.

And by "'Show info' option only for playlists" ,I think this should be an option for CHANNELS as well as for any STREAM, as user may only want to see say "Description" of a video

Unfortunately, 'Show info' is still useless for videos. Enabling auto-play makes the video play. Disabling it doesn't do so as expected. Rather, shared videos should not be dependent on such a setting.

Show info: Don't auto-play videos
Video player: Play videos on main player

Could you please add this feature on some other PR? It would be really nice of you.

If I've misinterpreted , I would like to be further assisted with the issue :)

It's me who should have apologized. Pardon me for my vague explanation at first. Also, thanks once again for your time and efforts.

Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Thank you

@TobiGr TobiGr merged commit f134e2d into TeamNewPipe:dev May 28, 2021
@sauravrao637
Copy link
Contributor Author

@SameenAhnaf
Thanks for being patient while explaining and also for reporting the issue
Also feel free to open an Issue for any feature you feel can be useful

@sauravrao637 sauravrao637 deleted the 6371 branch May 28, 2021 14:53
@opusforlife2 opusforlife2 changed the title Option for download is redundant and thus removed when linkType is CH… Download option removed from channels/playlists May 28, 2021
This was referenced Jun 5, 2021
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.

"Preferred 'Open' Action" setting should be applicable for videos only
3 participants