NewPipe icon indicating copy to clipboard operation
NewPipe copied to clipboard

Add long press action on hashtags and web links in descriptions

Open AudricV opened this issue 3 years ago • 12 comments

What is it?

  • [x] Feature (user facing)

Description of the changes in your PR

This PR adds a long press action on web links and hashtags which copy them to clipboard.

To do so:

  • Some changes in our TextView class have been required and metadata items are now using a NewPipeTextView instead of a standard TextView.

  • Three new classes have been added: a custom LinkMovementMethod class (adapted from a StackOverflow answer), a custom ClickableSpan class have been added in order to set a long press event and a class to avoid code duplication in CommentTextOnTouchListener, TouchUtils.

Before/After Screenshots/Screen Record

  • Before:

https://user-images.githubusercontent.com/74829229/151714340-aa46635c-998a-48ee-9b87-13ba5e83a771.mp4

  • After:

https://user-images.githubusercontent.com/74829229/151714470-88379f1f-9fb0-4da3-95f4-950847867725.mp4

Fixes the following issue(s)

  • Fixes #3253

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

AudricV avatar Jan 30 '22 17:01 AudricV

  • The videos are unplayable (can't view them in Firefox or Chromium-based browsers)

I had to compress them, unfortunately, but it seems Video Transcoder converted them in an unplayable format for browsers (you can play them in VLC I think)

AudricV avatar Jan 30 '22 17:01 AudricV

@litetex You may download the video with Video Download Helper. I eventually had to download, the video is still loading on my browser.

@TiA4f8R I included some suggestions in #7097. I think, it'll be more suitable for copying links.

SameenAhnaf avatar Jan 30 '22 17:01 SameenAhnaf

Converted the videos from mpeg4 (mp4v) to h264 (avc1) (according to ffmpeg). Now they are also 8x smaller. However the resolution with 221x480 is not very good, but at least it's viewable now 😄

litetex avatar Jan 30 '22 17:01 litetex

So what's the best way to proceed when long pressing links and hashtags?

  • copy them directly to clipboard
  • show an alert dialog to allow opening/searching them and copying them to clipboard

I am not all a UX designer so feedback is appreciated.

AudricV avatar Jan 30 '22 18:01 AudricV

@TiA4f8R The usual behaviour for Android apps is to take the specific action when directly tapping, and to copy to clipboard (or something else) when long-pressing.

opusforlife2 avatar Jan 30 '22 18:01 opusforlife2

Ok, I will do this.

Edit: done

AudricV avatar Jan 30 '22 18:01 AudricV

@TiA4f8R Can you please add this functionality to timestamps in the description? (Example video with timestamps: https://www.youtube.com/watch?v=mOSirVeP5lo)

Thanks!

ktprograms avatar Jan 31 '22 12:01 ktprograms

Note: even if it seems I forgot this pull request, I didn't forget it and I will continue to work on this, maybe once my work is done on other features/bug fixes I have on NewPipe projects.

AudricV avatar Jul 19 '22 20:07 AudricV

@TiA4f8R Can you please add this functionality to timestamps in the description? (Example video with timestamps: https://www.youtube.com/watch?v=mOSirVeP5lo)

Thanks!

@ktprograms Done (in the app side) on services on which timestamps supported (otherwise, a fallback to copy timestamp raw text is made).

AudricV avatar Sep 07 '22 20:09 AudricV

Note that the Sonar bug is a false positive, as nullity is checked by Utils.isBlank method of the extractor as the first instruction of the method and that the TODO that I added, is of course, not feasible in this PR.

AudricV avatar Sep 07 '22 20:09 AudricV

  • The videos are unplayable (can't view them in Firefox or Chromium-based browsers) - @ litetex at https://github.com/TeamNewPipe/NewPipe/pull/7725#pullrequestreview-867148494

kinda strange that yeah, it doesn't play on desktop browser ( Vivaldi | 5.5.2805.48 (Stable channel) (64-bit) ; Chrome/106.0.0.0), but is working fine on android browser (Vivaldi 5.6.2868.29)

goyalyashpal avatar Jan 22 '23 21:01 goyalyashpal

but i still don't understand the premise of this PR.

like, selecting tags and links was available before this pr as well:

  • "Enable selecting text in description"
  • press and hold on links to select and copy them
  • the only problem was that clicking them caused them to open

This pr shows that press and holding them automatically copies them - one tap saved.

But what happens in this pr when u single tap on links/tags? Did this problem was an effort to solve this single tap problem?

Following is a screenrecord from Newpipe v0.23.3 i.e. back from 2022.08'

https://user-images.githubusercontent.com/19423063/213942070-13371bb7-5819-40d5-a9d6-8518613c1218.mp4

goyalyashpal avatar Jan 22 '23 21:01 goyalyashpal

The videos are unplayable

I don't understand what you are referring to.

Did this problem was an effort to solve this single tap problem?

What do you mean by "single tap problem"? For me single taps work normally. Also, this PR was also needed because soon we will have comments with copiable links and hashtags.

Stypox avatar Jan 23 '23 09:01 Stypox

I don't understand what you are referring to.

sorry, have updated with link. : https://github.com/TeamNewPipe/NewPipe/pull/7725#pullrequestreview-867148494

single tap problem"? For me single taps work normall

this is shown in the screen recording - i meant that on single tapping, the links were opening. i was asking that was that considered the problem?

Also, this PR was also needed because soon we will have comments with copiable links and hashtags.

yeah, that is understandable. the thing that confused me was the following quoted part: "reopening as we are not able to copy full links" which we definitely were.

Reopening as we are not able to copy full links on YouTube. I will open a PR which will add this ability soon. - @ AudricV at https://github.com/TeamNewPipe/NewPipe/issues/3253#issuecomment-1024970127

goyalyashpal avatar Jan 24 '23 12:01 goyalyashpal

which we definitely were

Oh ok, I see what you meant. Yeah, in the description you were already able to copy YouTube links, since those are always shown in full. I am not sure about Peertube though, where Markdown is employed, and [link](https://example.org) gets transformed into the literal "link". Unless Android is able to copy rich text, I doubt you would be able to copy links from there. So the YouTube description was just a special case among other services' descriptions and also comments.

Stypox avatar Jan 28 '23 20:01 Stypox

about Peertube though, where Markdown is employed, Unless Android is able to copy rich text,

ohw ohkayh, yeah, makes sense then :+1:

goyalyashpal avatar Jan 29 '23 04:01 goyalyashpal