Show comment replies #9410

xz-dev · 2022-11-16T17:34:04Z

From #9020, for change branch name

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

Adds a "Show Replies" for the comments with replies for Youtube, just look like youtube.

Before/After Screenshots/Screen Record

Before	After1	After2	After3

Fixes the following issue(s)

Fixes Display comment replies #2277

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

I read the contribution guidelines.

Co-authored-by: krlvm <[email protected]>

* Make "Show Replies" button text a little bigger * Add elevation to the toolbar Co-authored-by: xz-dev <[email protected]>

acrodemocide

I have tested these changes on my Android, and they look good. I'm able to connect replies and navigate back to the comments section. The rest of the app appears to be working correctly.

app/src/main/java/org/schabi/newpipe/util/SerializedUtils.java

Co-authored-by: Isira Seneviratne <[email protected]>

nicoursi · 2022-11-27T19:53:46Z

NewPipe player with this branch has got issues on my phone.

I install the debug apk
I import subscriptions and settings from my previous installation
I longpress on a video and tap "play in background".
It crashes for every video
If I play the video in foreground then it works also for longpress and background.
Videos often become Black and or crash while rotating the phone.
This happens only with this PR.

Maybe b5c0823 should not be in this PR and it is creating problems?

Stypox

Code looks nice and understandable, you did a great job! Thanks for looking into this. Most of the review comments are small things, except for the clone thingy, for which we might want to look into other solutions. Anyway, nothing too big.

I am getting some problems loading the next page of the 488 replies of the first comment on this video. The loading indicator just spins forever. I would not consider this blocking as the comments on which there are so many replies are not so many, but it would be nice if you could fix this, too, before merging. ;-)

app/src/main/res/values/strings.xml

Stypox · 2022-11-27T22:35:53Z

app/src/main/res/layout/fragment_comments_reply.xml

+        <ImageButton
+            android:id="@+id/backButton"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_margin="@dimen/activity_horizontal_margin"
+            android:background="?attr/selectableItemBackgroundBorderless"
+            android:contentDescription="@string/back"
+            android:src="@drawable/ic_arrow_back" />


I think you can just use android:navigationIcon and android:navigationContentDescription instead of adding a button manually

I can not use your way to do that, it will show nothing at here.
There is another way can do that, but it's even more complex.

app/src/main/res/layout/list_comments_mini_item.xml

app/src/main/res/layout/list_comments_item.xml

Stypox · 2022-11-27T22:41:06Z

app/src/main/java/org/schabi/newpipe/fragments/list/comments/CommentsFragment.java

+                    // clone comment object to avoid relatedItems and nextPage actually set null
+                    info = CommentUtils.clone(info);


Mmmh, I don't like this that much. Can you explain further why you need to clone the object is a way that is so low-level?

When you check CommentReplyFragment, you will know.
Because I need create a fake CommentsInfo for the CommentFragment, then I can simply put two CommentFragment together.

Stypox · 2022-11-27T22:42:43Z

app/src/main/java/org/schabi/newpipe/info_list/holder/CommentsMiniInfoItemHolder.java

+            itemContentReplyButton.setOnClickListener(
+                    view -> itemBuilder.getOnCommentsReplyListener().selected(item)
+            );
+            final int replyCount = item.getReplyCount();


Shouldn't you setVisibility(View.GONE) when the reply count is 0?

If no reply, item.getReplies() will return null, so not need to set at here.

Stypox · 2022-11-27T22:48:52Z

app/src/main/res/layout/fragment_comments_reply.xml

+        <org.schabi.newpipe.views.NewPipeTextView
+            android:id="@+id/itemTitleView"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:layout_alignBottom="@+id/backButton"
+            android:layout_marginTop="@dimen/activity_horizontal_margin"
+            android:layout_toEndOf="@+id/backButton"
+            android:ellipsize="marquee"
+            android:fadingEdge="horizontal"
+            android:marqueeRepeatLimit="marquee_forever"
+            android:scrollHorizontally="true"
+            android:singleLine="true"
+            android:text="@string/replies"
+            android:textAppearance="?android:attr/textAppearanceLarge"
+            android:textSize="@dimen/channel_item_detail_title_text_size" />


And you also can do this with just android:title. Also, maybe instead of just showing "Replies" as title, it would be nice if it also contained the number, e.g. "12 replies".

No, you can't, as #9410 (comment) say

Stypox · 2022-11-27T22:50:47Z

app/src/main/java/org/schabi/newpipe/fragments/list/comments/CommentReplyFragment.java

+        this.url = u;
+        this.name = !TextUtils.isEmpty(title) ? title : "";
+        // clone comment object to avoid replies actually set null
+        this.comment = CommentUtils.clone(preComment);


xz-dev · 2022-11-28T06:02:09Z

NewPipe player with this branch has got issues on my phone.

Do you have any log or give me a screen recording to help me understand what happening?

nicoursi · 2022-12-01T15:32:38Z

NewPipe player with this branch has got issues on my phone.

Do you have any log or give me a screen recording to help me understand what happening?

CreenRecord: https://ufile.io/pwdj823r

When it crashes like this it does it with every video. The only way to fix it is to play a video in foreground. After that i can play in background by longpressing again.

Exception

User Action: ui error
Request: ACRA report
Content Country: IT
Content Language: it-IT
App Language: it_IT
Service: none
Version: 0.24.1
OS: Linux Android 11 - 30

Crash log

java.lang.IllegalArgumentException: No view found for id 0x7f0a0199 (org.schabi.newpipe.debug.featcommentreply:id/fragment_container_view) for fragment CommentsFragment{e387d90} (c9dfad59-1f04-42a0-9a9d-ec848df9704e id=0x7f0a0199)
	at androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:513)
	at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:261)
	at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1840)
	at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1758)
	at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1701)
	at androidx.fragment.app.FragmentManager$4.run(FragmentManager.java:488)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:223)
	at android.app.ActivityThread.main(ActivityThread.java:7669)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:594)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

nicoursi · 2022-12-03T18:50:12Z

This screenrecord shows problems while rotating the screen: https://ufile.io/8dfzxp2m

nicoursi · 2022-12-03T20:02:35Z

Another crash with rotation:

Screen record: https://ufile.io/aj35l7bb

Exception

User Action: ui error
Request: ACRA report
Content Country: IT
Content Language: it-IT
App Language: it_IT
Service: none
Version: 0.24.1
OS: Linux Android 11 - 30

Crash log

java.lang.IllegalArgumentException: No view found for id 0x7f0a0199 (org.schabi.newpipe.debug.featcommentreply:id/fragment_container_view) for fragment CommentsFragment{e2fe4bf} (85ea65be-5665-490a-a362-335938b9e29f id=0x7f0a0199)
	at androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:513)
	at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:261)
	at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1840)
	at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1758)
	at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1701)
	at androidx.fragment.app.FragmentManager$4.run(FragmentManager.java:488)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:223)
	at android.app.ActivityThread.main(ActivityThread.java:7669)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:594)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

TobiGr · 2022-12-04T15:51:02Z

I got the following glitch while working on the SoundCloud replies:

bug.webm

I have no idea how i created it. I just opened the stream from the history and scrolled up and down a few times.

Co-authored-by: Stypox <[email protected]>

sonarqubecloud · 2022-12-09T15:17:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

0.0% Coverage
0.0% Duplication

Dragonfly-30 · 2023-03-01T06:05:27Z

Is this still being worked on?

xz-dev · 2023-03-01T06:09:14Z

Is this still being worked on?

I've been busy doing other things lately, I'll probably see later if there's anything else I can do.
If you're able to continue this work, I'd be glad someone could take over and finish it

Stypox · 2023-03-01T07:37:55Z

Yes, this will be merged in 0.26.0, don't worry and stop asking

tsiflimagas · 2023-04-09T07:32:10Z

I've experienced the same crash reported in #9410 (comment) frequently lately, so I wanted to add that to reproduce it you have to be on description tab and rotate the video. I'm able to reproduce it reliably like that.

Stypox · 2023-04-11T20:39:31Z

Closing in favour of #10018, since it is a simpler implementation and should also have less bugs. Thank you nonetheless for your work!

xz-dev and others added 23 commits September 25, 2022 16:48

Show comment replies in DialogFragment

e201b79

Show initial comment in CommentReplyDialog

703876c

Fix wrong background color for Dark theme for BottomSheetDialogFragment

8512198

Fix wrong string

18f3349

Fix comment replies only work at the first time

5cd7807

Clean code for comment reply

a8fcccd

Add click effect to back button

8f8e29c

Co-authored-by: krlvm <[email protected]>

Improve bottom sheets design

648e331

Fix comment replies pagination

a4cd42b

Avoid mocked classes are cached

5603e85

Auto dismiss commit reply dialog

fbd5f24

Add ripple effect to show reply button

b91df5e

Add toolbar to comment reply dialog

8dda380

Clean the code smells which sonarcloud show

3f64dff

Fix comment reply dialog toolbar text color

6eef715

Improve "Show replies" button appearance

3ffcaf8

A couple of visual improvements for comment replies (#3)

77bdf6b

* Make "Show Replies" button text a little bigger * Add elevation to the toolbar Co-authored-by: xz-dev <[email protected]>

FragmentContainerView replace BottomSheetDialogFragment

f707347

Fix crashes when rotate the screen

be3a242

Add reply count

f29c941

Capitalizing the first word of reply

89a1ab5

Update NewPipe Extractor to support handles

b5c0823

Change comment reply toolbar color

298366a

xz-dev mentioned this pull request Nov 16, 2022

Show comment replies #9020

Closed

5 tasks

TobiGr added GUI Issue is related to the graphical user interface feature request Issue is related to a feature in the app labels Nov 16, 2022

acrodemocide reviewed Nov 22, 2022

View reviewed changes

Isira-Seneviratne reviewed Nov 25, 2022

View reviewed changes

app/src/main/java/org/schabi/newpipe/util/SerializedUtils.java Outdated Show resolved Hide resolved

Remove NullPointerException from clone throws list

49600d4

Co-authored-by: Isira Seneviratne <[email protected]>

Stypox requested changes Nov 27, 2022

View reviewed changes

This was referenced Dec 3, 2022

[PeerTube] Support comment replies TeamNewPipe/NewPipeExtractor#941

Merged

[SoundCloud] Add support for comment replies TeamNewPipe/NewPipeExtractor#993

Draft

Lower case reply

d2059db

Co-authored-by: Stypox <[email protected]>

xz-dev force-pushed the feat/comment_reply branch from 0c5433e to d2059db Compare December 9, 2022 14:43

InfinityLoop1308 mentioned this pull request Jan 7, 2023

[HELP NEEDED] Major planned/missing features InfinityLoop1308/PipePipe#51

Closed

6 tasks

Stypox mentioned this pull request Apr 4, 2023

[HELP NEEDED] Major planned/missing features #6448

Open

34 tasks

DUOLabs333 mentioned this pull request Apr 9, 2023

Comment replies #8443

Closed

5 tasks

Stypox closed this Apr 11, 2023

MD77MD mentioned this pull request Apr 19, 2023

Add support for comment replies #10018

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show comment replies #9410

Show comment replies #9410

xz-dev commented Nov 16, 2022 •

edited

Loading

acrodemocide left a comment

nicoursi commented Nov 27, 2022 •

edited

Loading

Stypox left a comment

Stypox Nov 27, 2022

xz-dev Dec 9, 2022

Stypox Nov 27, 2022

xz-dev Dec 9, 2022

Stypox Nov 27, 2022

xz-dev Dec 9, 2022

Stypox Nov 27, 2022

xz-dev Dec 9, 2022

Stypox Nov 27, 2022

xz-dev commented Nov 28, 2022 •

edited by Stypox

Loading

nicoursi commented Dec 1, 2022 •

edited

Loading

nicoursi commented Dec 3, 2022

nicoursi commented Dec 3, 2022

TobiGr commented Dec 4, 2022 •

edited by AudricV

Loading

sonarqubecloud bot commented Dec 9, 2022

Dragonfly-30 commented Mar 1, 2023

xz-dev commented Mar 1, 2023

Stypox commented Mar 1, 2023

tsiflimagas commented Apr 9, 2023

Stypox commented Apr 11, 2023 •

edited

Loading

		// clone comment object to avoid relatedItems and nextPage actually set null
		info = CommentUtils.clone(info);

Show comment replies #9410

Show comment replies #9410

Conversation

xz-dev commented Nov 16, 2022 • edited Loading

What is it?

Description of the changes in your PR

Before/After Screenshots/Screen Record

Fixes the following issue(s)

APK testing

Due diligence

acrodemocide left a comment

Choose a reason for hiding this comment

nicoursi commented Nov 27, 2022 • edited Loading

Stypox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xz-dev commented Nov 28, 2022 • edited by Stypox Loading

nicoursi commented Dec 1, 2022 • edited Loading

Exception

nicoursi commented Dec 3, 2022

nicoursi commented Dec 3, 2022

Exception

TobiGr commented Dec 4, 2022 • edited by AudricV Loading

sonarqubecloud bot commented Dec 9, 2022

Dragonfly-30 commented Mar 1, 2023

xz-dev commented Mar 1, 2023

Stypox commented Mar 1, 2023

tsiflimagas commented Apr 9, 2023

Stypox commented Apr 11, 2023 • edited Loading

xz-dev commented Nov 16, 2022 •

edited

Loading

nicoursi commented Nov 27, 2022 •

edited

Loading

xz-dev commented Nov 28, 2022 •

edited by Stypox

Loading

nicoursi commented Dec 1, 2022 •

edited

Loading

TobiGr commented Dec 4, 2022 •

edited by AudricV

Loading

Stypox commented Apr 11, 2023 •

edited

Loading