Skip to content

Display parent channel details#3430

Merged
TobiGr merged 7 commits intoTeamNewPipe:devfrom
Royosef:DisplayParentChannelDetails
May 8, 2020
Merged

Display parent channel details#3430
TobiGr merged 7 commits intoTeamNewPipe:devfrom
Royosef:DisplayParentChannelDetails

Conversation

@Royosef
Copy link
Contributor

@Royosef Royosef commented Apr 13, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Add sub-channel details to channel fragment & video detail fragment

Relies on the following changes

TeamNewPipe/NewPipeExtractor#313

Testing apk

NewPipe_displayparentchanneldetails-debug.zip

Agreement

Copy link
Member

@B0pol B0pol left a comment

Choose a reason for hiding this comment

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

.

Copy link
Member

@B0pol B0pol left a comment

Choose a reason for hiding this comment

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

There is one thing I've noticed while testing: I'm not a fan of the possibility to go to parent channel page directly in video page. The room is already tiny, and it's now divided by two, on my large phone it's hard to go on the one you want, on my small phone it's simply impossible to choose.

Words are better than images: red is the separation between channel (up) and parent (down)
.

Copy link
Contributor

@wb9688 wb9688 left a comment

Choose a reason for hiding this comment

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

You broke the ability to open streams from a channel.

@Royosef
Copy link
Contributor Author

Royosef commented Apr 14, 2020

There is one thing I've noticed while testing: I'm not a fan of the possibility to go to parent channel page directly in video page. The room is already tiny, and it's now divided by two, on my large phone it's hard to go on the one you want, on my small phone it's simply impossible to choose.

Words are better than images: red is the separation between channel (up) and parent (down)
.

@B0pol So, will the whole section navigate to the channel page? (Les vidéos de Framasoft in this case)

@wb9688 wb9688 added this to the 0.20.0 milestone Apr 15, 2020
@Royosef Royosef force-pushed the DisplayParentChannelDetails branch 2 times, most recently from cd1196c to 5581eb9 Compare April 17, 2020 08:13
@B0pol
Copy link
Member

B0pol commented Apr 19, 2020

There is one thing I've noticed while testing: I'm not a fan of the possibility to go to parent channel page directly in video page. The room is already tiny, and it's now divided by two, on my large phone it's hard to go on the one you want, on my small phone it's simply impossible to choose.

Words are better than images: red is the separation between channel (up) and parent (down)
.

@B0pol So, will the whole section navigate to the channel page? (Les vidéos de Framasoft in this case)

@wb9688 asked me to reply but we've already talked about that via IRC, I just make it public:

Yes, the whole section will go to Les vidéos de Framasoft in this case. Unless someone find a good way to access seamlessly to both, it should go to the (sub)channel page.

But don't remove this part from the extractor because it's not only used by NewPipe, or someone in another PR will success to integrate it seamlessly…

@wb9688
Copy link
Contributor

wb9688 commented Apr 19, 2020

Unless someone find a good way to access seamlessly to both, it should go to the (sub)channel page.

Maybe use long press for parent channel while using a normal tap for (sub)channel.

@Royosef
Copy link
Contributor Author

Royosef commented Apr 19, 2020

Unless someone find a good way to access seamlessly to both, it should go to the (sub)channel page.

Maybe use long press for parent channel while using a normal tap for (sub)channel.

Honestly, I don't know how much intuitive it will be for the users.

@wb9688
Copy link
Contributor

wb9688 commented Apr 19, 2020

@Royosef: It's better than nothing, and we have long presses on other buttons on that page as well.

@TobiGr: What do you think?

@TobiGr
Copy link
Contributor

TobiGr commented Apr 19, 2020

I agree with @wb9688. Long-press is better than nothing.

@opusforlife2
Copy link
Collaborator

There is already a setting to show a tip for background and popup buttons. It could be modified to also show one for this so that users get to know of the change.

@wb9688
Copy link
Contributor

wb9688 commented Apr 19, 2020

@opusforlife2: I don't think that's needed as it's currently only a PeerTube thing, and we don't have a tip for the download button either.

@opusforlife2
Copy link
Collaborator

Alright. If it's a Peertube specific feature and not app wide, I can imagine it would be tougher to implement.

Yeah, I got to know of the download button from your reddit comments. There really should be a tool tip for it.

@wb9688 wb9688 modified the milestones: 0.20.0, 0.19.4 Apr 19, 2020
Royosef added 4 commits May 7, 2020 20:39
Make all of the uploader section on stream page navigate to the channel page
Extract hard coded strings
Remove redundant spaces
Fix open streams from a channel
Rename "ParentChannel" to "SubChannel"
Config royosef:NewPipeExtractor in app/build.gradle
@wb9688 wb9688 force-pushed the DisplayParentChannelDetails branch from 6d55024 to a43fa65 Compare May 7, 2020 18:52
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

A few questions, but in general this is good to go :-D

Stypox
Stypox previously approved these changes May 8, 2020
@wb9688 wb9688 force-pushed the DisplayParentChannelDetails branch from d00ca18 to 1096ec1 Compare May 8, 2020 13:52
@wb9688 wb9688 force-pushed the DisplayParentChannelDetails branch from aa71230 to ae437b1 Compare May 8, 2020 16:08
@TobiGr TobiGr merged commit 9cf76a9 into TeamNewPipe:dev May 8, 2020
@B0pol B0pol added the feature request Issue is related to a feature in the app label May 17, 2020
This was referenced May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request Issue is related to a feature in the app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants