-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Show comment replies #7244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show comment replies #7244
Changes from all commits
66e999f
ead685e
f6f7994
b11d62f
8d30693
a6f68fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| package org.schabi.newpipe.info_list.holder; | ||
|
|
||
| import android.annotation.SuppressLint; | ||
| import android.view.View; | ||
| import android.view.ViewGroup; | ||
| import android.widget.TextView; | ||
|
|
||
| import androidx.recyclerview.widget.LinearLayoutManager; | ||
| import androidx.recyclerview.widget.RecyclerView; | ||
|
|
||
| import org.schabi.newpipe.extractor.ListExtractor; | ||
| import org.schabi.newpipe.extractor.comments.CommentsInfo; | ||
| import org.schabi.newpipe.extractor.comments.CommentsInfoItem; | ||
| import org.schabi.newpipe.info_list.InfoListAdapter; | ||
| import org.schabi.newpipe.util.ExtractorHelper; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.concurrent.Callable; | ||
|
|
||
| import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; | ||
| import io.reactivex.rxjava3.annotations.NonNull; | ||
| import io.reactivex.rxjava3.core.Single; | ||
| import io.reactivex.rxjava3.core.SingleObserver; | ||
| import io.reactivex.rxjava3.disposables.Disposable; | ||
| import io.reactivex.rxjava3.schedulers.Schedulers; | ||
|
|
||
| public class RepliesHandler { | ||
| private final List<CommentsInfoItem> cachedReplies; | ||
| private final TextView showReplies; | ||
| private final RecyclerView repliesView; | ||
|
|
||
| public RepliesHandler(final TextView showReplies, final RecyclerView recyclerView) { | ||
| this.repliesView = recyclerView; | ||
| repliesView.setAdapter(new InfoListAdapter(repliesView.getContext())); | ||
| repliesView.setLayoutManager(new LinearLayoutManager(repliesView.getContext())); | ||
|
|
||
| this.showReplies = showReplies; | ||
| this.cachedReplies = new ArrayList<>(); | ||
| } | ||
|
|
||
| static class GetMoreItemsCallable implements | ||
| Callable<ListExtractor.InfoItemsPage<CommentsInfoItem>> { | ||
| private CommentsInfo parentCommentInfo; | ||
| private CommentsInfoItem parentInfoItem; | ||
|
|
||
| public void setCallableParameters( | ||
| final CommentsInfo commentInfo, final CommentsInfoItem infoItem) { | ||
| parentCommentInfo = commentInfo; | ||
| parentInfoItem = infoItem; | ||
| } | ||
|
|
||
| @Override | ||
| public ListExtractor.InfoItemsPage<CommentsInfoItem> call() throws Exception { | ||
| return CommentsInfo.getMoreItems(parentCommentInfo, parentInfoItem.getReplies()); | ||
| } | ||
| } | ||
|
|
||
| private Single<ListExtractor.InfoItemsPage<CommentsInfoItem>> | ||
| repliesSingle(final CommentsInfo parentCommentInfo, | ||
| final CommentsInfoItem parentInfoItem) { | ||
|
|
||
| final GetMoreItemsCallable getMoreItems = new GetMoreItemsCallable(); | ||
| getMoreItems.setCallableParameters(parentCommentInfo, parentInfoItem); | ||
| return Single.fromCallable(getMoreItems); | ||
|
|
||
| } | ||
|
|
||
| private SingleObserver<ListExtractor.InfoItemsPage<CommentsInfoItem>> | ||
| repliesObserver(final CommentsInfoItem parentInfoItem) { | ||
|
|
||
| return new SingleObserver<ListExtractor.InfoItemsPage<CommentsInfoItem>>() { | ||
|
|
||
| @SuppressLint("SetTextI18n") | ||
| @Override | ||
| public void onSubscribe(@NonNull final Disposable d) { | ||
| showReplies.setText("Setting up replies"); | ||
| } | ||
|
|
||
| @Override | ||
| public void onSuccess(@NonNull final | ||
| ListExtractor.InfoItemsPage<CommentsInfoItem> | ||
| commentsInfoItemInfoItemsPage) { | ||
|
|
||
| final List<CommentsInfoItem> actualList | ||
| = commentsInfoItemInfoItemsPage.getItems(); | ||
|
|
||
| cachedReplies.addAll(actualList); | ||
| addRepliesToUI(parentInfoItem); | ||
| } | ||
|
|
||
| @SuppressLint("SetTextI18n") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a reason why Android Studio gives you this warning: you shouldn't hardcode strings, put them in the |
||
| @Override | ||
| public void onError(@NonNull final Throwable e) { | ||
| showReplies.setText("Error getting replies"); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| private SingleObserver<CommentsInfo> | ||
| repliesInfoObserver(final CommentsInfoItem parentInfoItem) { | ||
|
|
||
| return new SingleObserver<CommentsInfo>() { | ||
|
|
||
| @SuppressLint("SetTextI18n") | ||
| @Override | ||
| public void onSubscribe(@NonNull final Disposable d) { | ||
| showReplies.setText("Downloading Replies"); | ||
| } | ||
|
|
||
| @Override | ||
| public void onSuccess(@NonNull final CommentsInfo commentsInfo) { | ||
| final Single<ListExtractor.InfoItemsPage<CommentsInfoItem>> | ||
| getRepliesInfoSingle = | ||
| repliesSingle(commentsInfo, parentInfoItem); | ||
|
|
||
| final SingleObserver<ListExtractor.InfoItemsPage<CommentsInfoItem>> | ||
| getRepliesInfoObserver = repliesObserver(parentInfoItem); | ||
|
|
||
| getRepliesInfoSingle | ||
| .subscribeOn(Schedulers.io()) | ||
| .observeOn(AndroidSchedulers.mainThread()) | ||
| .subscribe(getRepliesInfoObserver); | ||
| } | ||
|
|
||
| @SuppressLint("SetTextI18n") | ||
| @Override | ||
| public void onError(@NonNull final Throwable e) { | ||
| showReplies.setText("Error getting replies"); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| public void addRepliesToUI(final CommentsInfoItem parentInfoItem) { | ||
| ((InfoListAdapter) Objects.requireNonNull(repliesView.getAdapter())) | ||
| .setInfoItemList(cachedReplies); | ||
|
|
||
| final ViewGroup.MarginLayoutParams params = | ||
| (ViewGroup.MarginLayoutParams) repliesView.getLayoutParams(); | ||
| params.topMargin = 45; | ||
|
|
||
| repliesView.setMinimumHeight(100); | ||
| repliesView.setHasFixedSize(true); | ||
| parentInfoItem.setRepliesOpen(true); | ||
| showReplies.setVisibility(View.GONE); | ||
| repliesView.setVisibility(View.VISIBLE); | ||
| } | ||
|
|
||
| public void downloadReplies(final CommentsInfoItem parentInfoItem) { | ||
|
|
||
| final Single<CommentsInfo> parentInfoSingle = ExtractorHelper.getCommentsInfo( | ||
| parentInfoItem.getServiceId(), | ||
| parentInfoItem.getUrl(), | ||
| false | ||
| ); | ||
|
|
||
| final SingleObserver<CommentsInfo> singleInfoRepliesInfoObserver | ||
| = repliesInfoObserver(parentInfoItem); | ||
|
|
||
| parentInfoSingle | ||
| .subscribeOn(Schedulers.io()) | ||
| .observeOn(AndroidSchedulers.mainThread()) | ||
| .subscribe(singleInfoRepliesInfoObserver); | ||
| } | ||
|
|
||
| public void addReplies(final CommentsInfoItem parentInfoItem) { | ||
| if (parentInfoItem.getReplies() == null) { | ||
| return; | ||
| } | ||
|
|
||
| if (cachedReplies.isEmpty()) { | ||
| downloadReplies(parentInfoItem); | ||
| } else { | ||
| addRepliesToUI(parentInfoItem); | ||
| } | ||
| } | ||
|
|
||
| public void checkForReplies(final CommentsInfoItem item) { | ||
| if (item.getReplies() == null) { | ||
| repliesView.setVisibility(View.GONE); | ||
| showReplies.setVisibility(View.GONE); | ||
| } else if (item.getRepliesOpen()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said in the extractor, this method should be removed from there and implemented here in the frontend. The usual approach would be to create an item to use in the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @litetex do you have any idea whether this can be achieved in a better way, without storing any separate data in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Stypox At first: Welcome back.
No I don't - currently. I think I would have to look at this in more detail to get an answer to that, however some ideas from my side to make nested commets working:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @golfinq ping ;-)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is what I could guess by looking at the code, but I might be wrong since I couldn't test. But the problem is also the fact that you shouldn't store data about whether the replies menu is open or not in an object coming from the extractor. The extractor shouldn't have anything to do with the UI. |
||
| addReplies(item); | ||
| } else { | ||
| repliesView.setVisibility(View.GONE); | ||
| showReplies.setVisibility(View.VISIBLE); | ||
| showReplies.setOnClickListener(v -> addReplies(item)); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,4 +123,34 @@ | |
| android:textSize="@dimen/video_item_search_upload_date_text_size" | ||
| tools:text="1 year ago" /> | ||
|
|
||
| <TextView | ||
| android:id="@+id/showReplies" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_below="@+id/itemCommentContentView" | ||
| android:layout_alignEnd="@+id/itemCommentContentView" | ||
| android:layout_alignBottom="@+id/itemPublishedTime" | ||
| android:layout_marginStart="21dp" | ||
| android:layout_marginTop="0sp" | ||
| android:layout_marginEnd="0sp" | ||
| android:layout_marginBottom="0sp" | ||
| android:layout_toEndOf="@+id/itemPublishedTime" | ||
| android:text="@string/show_replies" | ||
| android:textAppearance="@style/TextAppearance.AppCompat.Small" | ||
| android:textSize="@dimen/video_item_search_upload_date_text_size" /> | ||
|
|
||
| <androidx.recyclerview.widget.RecyclerView | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understood correctly TeamNewPipe/NewPipeExtractor#703, a reply-comment could have replies itself, but this PR introduces support only for 1 level of reply depth. This would work fine for YouTube, but it would be problematic for other services, so the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was hoping that by reusing the 'InfoListAdapter` the comment views which were created would automatically have the reply handling code inside of it. How are the comment views which are being created here different than the ones which are created in the main comment holding view? |
||
| android:id="@+id/replyRecycleView" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_below="@+id/showReplies" | ||
| android:layout_alignStart="@+id/itemCommentContentView" | ||
| android:layout_alignEnd="@+id/itemCommentContentView" | ||
| android:layout_marginStart="0dp" | ||
| android:layout_marginTop="0dp" | ||
| android:layout_marginEnd="0dp" | ||
| android:layout_marginBottom="0dp"> | ||
|
|
||
| </androidx.recyclerview.widget.RecyclerView> | ||
|
|
||
| </RelativeLayout> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular reason you added this code in
CommentsInfoItemHolderand notCommentsMiniInfoItemHolderlike all other comments code?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was probably left over from something else I was trying to do, should be straight forward to move it to where it is used