Skip to content

Conversation

@Jaspann
Copy link
Contributor

@Jaspann Jaspann commented Dec 26, 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

Removes a blank sub-channel icon if you watch a Peertube video and then a non-PeerTube video after.

  • Does not remove the icon if you watch a PeerTube video with a channel that does not have a sub-channel icon, to avoid confusion

Before/After Screenshots/Screen Record

Fixes the following issue(s)

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 AudricV added bug Issue is related to a bug GUI Issue is related to the graphical user interface peertube Service, https://joinpeertube.org/ labels Dec 26, 2021
Copy link
Member

@litetex litetex 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 for the PR.
However this doesn't fix the problem completely:

if (!isEmpty(info.getSubChannelName())) {
displayBothUploaderAndSubChannel(info);
} else if (!isEmpty(info.getUploaderName())) {
displayUploaderAsSubChannel(info);
} else {
binding.detailUploaderTextView.setVisibility(View.GONE);
binding.detailUploaderThumbnailView.setVisibility(View.GONE);
}

if we encounter the else-Block the view will still be visible.

I would suggest simply putting binding.detailSubChannelThumbnailView.setVisibility(View.GONE); before the above code block (line 1523 or so) to fix the problem.

@Jaspann
Copy link
Contributor Author

Jaspann commented Dec 27, 2021

@litetex Yeah, I was unsure what to think of that else statement, as it seams like we would have already reached an error if it got to the point where there is no uploader name. Inside the else we also don't remove the binding.detailSubChannelTextView view, so I was not sure if it was accurate to remove binding.detailSubChannelThumbnailView or not.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Jaspann Jaspann requested a review from litetex December 28, 2021 21:15
Copy link
Member

@litetex litetex left a comment

Choose a reason for hiding this comment

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

LGTM :)

@litetex litetex merged commit 6b490ee into TeamNewPipe:dev Dec 31, 2021
This was referenced Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue is related to a bug GUI Issue is related to the graphical user interface peertube Service, https://joinpeertube.org/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Peertube channel sub-icon persists in non-Peertube videos

3 participants