NewPipe icon indicating copy to clipboard operation
NewPipe copied to clipboard

Update placeholder variants

Open HighGr0und opened this issue 2 years ago • 6 comments

What is it?

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

Description of the changes in your PR

  • Add two new placeholder thumbnail (hourglass_thumbnail and close_thumbnail).
  • Replace placeholder_thumbnail_video with placeholder_thumbnail_hourglass.
  • Show placeholder_thumbnail_off when cover URL is empty

Before/After Screenshots/Screen Record

Before After hourglass After close
Before After After

Fixes the following issue(s)

  • Fixes #9044

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

HighGr0und avatar Oct 27 '22 14:10 HighGr0und

They ignored the template section for linking the fixed issue. @HighGr0und You're not supposed to ignore provided templates.

opusforlife2 avatar Oct 27 '22 16:10 opusforlife2

I don't know whether this is a good addition. The new placeholders look very old-fashioned. We could try to use the three loading dots for the loading placeholders, but I'd prefer to keep the old placeholder in case of a failure.

TobiGr avatar Oct 27 '22 19:10 TobiGr

I have changed the style of hourglass, should I use this or three loading dots Screenshot 2022-10-28 152628

HighGr0und avatar Oct 28 '22 04:10 HighGr0und

Please explain why you think we need multiple types of placeholder images. And explain use-cases for the various situations.

Stypox avatar Oct 28 '22 08:10 Stypox

Please explain why you think we need multiple types of placeholder images. And explain use-cases for the various situations. I apologise for missing the issue that this PR relates to, it is specific to this issue: https://github.com/TeamNewPipe/NewPipe/issues/9044

HighGr0und avatar Oct 28 '22 10:10 HighGr0und

See https://github.com/TeamNewPipe/NewPipe/issues/9044#issuecomment-1336504293. Also, I don't think the images proposed here fit into the app design and I don't think it's worth to spend time on this niche feature. Sorry for that and thank you for the contribution anyway!

Stypox avatar Dec 04 '22 20:12 Stypox