Skip to content

Show stream uploader icon on comments they have replied to #11991

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: refactor
Choose a base branch
from
Open

Show stream uploader icon on comments they have replied to #11991

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd222c4
Show stream uploader icon on comments they have replied to
Isira-Seneviratne Feb 1, 2025
b768b38
Force-load stream data
Isira-Seneviratne Feb 2, 2025
a1090f9
Merge branch 'refactor' into Uploader-reply-indicator
Isira-Seneviratne Feb 22, 2025
fdd6caf
Merge branch 'refactor' into Uploader-reply-indicator
Isira-Seneviratne Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 36 additions & 15 deletions app/src/main/java/org/schabi/newpipe/ui/components/video/comment/Comment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ import org.schabi.newpipe.util.image.ImageStrategy

@OptIn(ExperimentalFoundationApi::class)
@Composable
fun Comment(comment: CommentsInfoItem, onCommentAuthorOpened: () -> Unit) {
fun Comment(
comment: CommentsInfoItem,
uploaderAvatarUrl: String? = null,
onCommentAuthorOpened: (() -> Unit)? = null,
) {
val context = LocalContext.current
var isExpanded by rememberSaveable { mutableStateOf(false) }
var showReplies by rememberSaveable { mutableStateOf(false) }
Expand All @@ -82,7 +86,7 @@ fun Comment(comment: CommentsInfoItem, onCommentAuthorOpened: () -> Unit) {
.clip(CircleShape)
.clickable {
NavigationHelper.openCommentAuthorIfPresent(context, comment)
onCommentAuthorOpened()
onCommentAuthorOpened?.invoke()
}
)

Expand Down Expand Up @@ -159,17 +163,31 @@ fun Comment(comment: CommentsInfoItem, onCommentAuthorOpened: () -> Unit) {
}

if (comment.replies != null) {
// reduce LocalMinimumInteractiveComponentSize from 48dp to 44dp to slightly
// reduce the button margin (which is still clickable but not visible)
CompositionLocalProvider(LocalMinimumInteractiveComponentSize provides 44.dp) {
TextButton(
onClick = { showReplies = true },
modifier = Modifier.padding(end = 2.dp)
) {
val text = pluralStringResource(
R.plurals.replies, comment.replyCount, comment.replyCount.toString()
Row(verticalAlignment = Alignment.CenterVertically) {
if (comment.hasCreatorReply()) {
AsyncImage(
model = uploaderAvatarUrl,
contentDescription = null,
placeholder = painterResource(R.drawable.placeholder_person),
error = painterResource(R.drawable.placeholder_person),
modifier = Modifier
.size(30.dp)
.clip(CircleShape)
)
Text(text = text)
}

// reduce LocalMinimumInteractiveComponentSize from 48dp to 44dp to slightly
// reduce the button margin (which is still clickable but not visible)
CompositionLocalProvider(LocalMinimumInteractiveComponentSize provides 44.dp) {
TextButton(
onClick = { showReplies = true },
modifier = Modifier.padding(end = 2.dp)
) {
val text = pluralStringResource(
R.plurals.replies, comment.replyCount, comment.replyCount.toString()
)
Text(text = text)
}
}
}
}
Expand Down Expand Up @@ -198,6 +216,7 @@ fun CommentsInfoItem(
isPinned: Boolean = false,
replies: Page? = null,
replyCount: Int = 0,
hasCreatorReply: Boolean = false,
) = CommentsInfoItem(serviceId, url, name).apply {
this.commentText = commentText
this.uploaderName = uploaderName
Expand All @@ -207,6 +226,7 @@ fun CommentsInfoItem(
this.isPinned = isPinned
this.replies = replies
this.replyCount = replyCount
setCreatorReply(hasCreatorReply)
}

private class CommentPreviewProvider : CollectionPreviewParameterProvider<CommentsInfoItem>(
Expand Down Expand Up @@ -245,7 +265,8 @@ private class CommentPreviewProvider : CollectionPreviewParameterProvider<Commen
isPinned = false,
isHeartedByUploader = false,
replies = Page(""),
replyCount = 4283
replyCount = 4283,
hasCreatorReply = true,
),
)
)
Expand All @@ -258,7 +279,7 @@ private fun CommentPreview(
) {
AppTheme {
Surface {
Comment(commentsInfoItem) {}
Comment(commentsInfoItem)
}
}
}
Expand All @@ -270,7 +291,7 @@ private fun CommentListPreview() {
Surface {
Column {
for (comment in CommentPreviewProvider().values) {
Comment(comment) {}
Comment(comment)
}
}
}
Expand Down
13 changes: 5 additions & 8 deletions app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentRepliesDialog.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import org.schabi.newpipe.ui.theme.AppTheme
fun CommentRepliesDialog(
parentComment: CommentsInfoItem,
onDismissRequest: () -> Unit,
onCommentAuthorOpened: () -> Unit,
onCommentAuthorOpened: (() -> Unit)?,
) {
val coroutineScope = rememberCoroutineScope()
val commentsFlow = remember {
Expand All @@ -64,7 +64,7 @@ private fun CommentRepliesDialog(
parentComment: CommentsInfoItem,
commentsFlow: Flow<PagingData<CommentsInfoItem>>,
onDismissRequest: () -> Unit,
onCommentAuthorOpened: () -> Unit,
onCommentAuthorOpened: (() -> Unit)? = null,
) {
val comments = commentsFlow.collectAsLazyPagingItems()
val nestedScrollInterop = rememberNestedScrollInteropConnection()
Expand All @@ -74,7 +74,7 @@ private fun CommentRepliesDialog(
val sheetState = rememberModalBottomSheetState()
val nestedOnCommentAuthorOpened: () -> Unit = {
// also partialExpand any parent dialog
onCommentAuthorOpened()
onCommentAuthorOpened?.invoke()
coroutineScope.launch {
sheetState.partialExpand()
}
Expand Down Expand Up @@ -138,10 +138,7 @@ private fun CommentRepliesDialog(
}
} else {
items(comments.itemCount) {
Comment(
comment = comments[it]!!,
onCommentAuthorOpened = nestedOnCommentAuthorOpened,
)
Comment(comments[it]!!, onCommentAuthorOpened = nestedOnCommentAuthorOpened)
}
}
}
Expand Down Expand Up @@ -172,6 +169,6 @@ private fun CommentRepliesDialogPreview() {
val flow = flowOf(PagingData.from(replies))

AppTheme {
CommentRepliesDialog(comment, flow, onDismissRequest = {}, onCommentAuthorOpened = {})
CommentRepliesDialog(comment, flow, onDismissRequest = {})
}
}
17 changes: 12 additions & 5 deletions app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,25 @@ import org.schabi.newpipe.ui.components.common.LoadingIndicator
import org.schabi.newpipe.ui.emptystate.EmptyStateComposable
import org.schabi.newpipe.ui.emptystate.EmptyStateSpec
import org.schabi.newpipe.ui.theme.AppTheme
import org.schabi.newpipe.util.image.ImageStrategy
import org.schabi.newpipe.viewmodels.CommentsViewModel
import org.schabi.newpipe.viewmodels.util.Resource

@Composable
fun CommentSection(commentsViewModel: CommentsViewModel = viewModel()) {
val state by commentsViewModel.uiState.collectAsStateWithLifecycle()
CommentSection(state, commentsViewModel.comments)
val streamState by commentsViewModel.streamState.collectAsStateWithLifecycle()
val commentState by commentsViewModel.commentState.collectAsStateWithLifecycle()

val avatars = (streamState as? Resource.Success)?.data?.uploaderAvatars.orEmpty()
val uploaderAvatarUrl = ImageStrategy.choosePreferredImage(avatars)

CommentSection(commentState, uploaderAvatarUrl, commentsViewModel.comments)
}

@Composable
private fun CommentSection(
uiState: Resource<CommentInfo>,
uploaderAvatarUrl: String? = null,
commentsFlow: Flow<PagingData<CommentsInfoItem>>
) {
val comments = commentsFlow.collectAsLazyPagingItems()
Expand Down Expand Up @@ -110,7 +117,7 @@ private fun CommentSection(

else -> {
items(comments.itemCount) {
Comment(comment = comments[it]!!) {}
Comment(comments[it]!!, uploaderAvatarUrl)
}
}
}
Expand Down Expand Up @@ -140,7 +147,7 @@ private fun CommentSection(
private fun CommentSectionLoadingPreview() {
AppTheme {
Surface {
CommentSection(uiState = Resource.Loading, commentsFlow = flowOf())
CommentSection(Resource.Loading, commentsFlow = flowOf())
}
}
}
Expand Down Expand Up @@ -187,7 +194,7 @@ private fun CommentSectionSuccessPreview() {
private fun CommentSectionErrorPreview() {
AppTheme {
Surface {
CommentSection(uiState = Resource.Error(RuntimeException()), commentsFlow = flowOf())
CommentSection(Resource.Error(RuntimeException()), commentsFlow = flowOf())
}
}
}
22 changes: 20 additions & 2 deletions app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,32 @@ import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.rx3.await
import org.schabi.newpipe.extractor.comments.CommentsInfo
import org.schabi.newpipe.paging.CommentsSource
import org.schabi.newpipe.ui.components.video.comment.CommentInfo
import org.schabi.newpipe.util.ExtractorHelper
import org.schabi.newpipe.util.KEY_SERVICE_ID
import org.schabi.newpipe.util.KEY_URL
import org.schabi.newpipe.util.NO_SERVICE_ID
import org.schabi.newpipe.viewmodels.util.Resource

class CommentsViewModel(savedStateHandle: SavedStateHandle) : ViewModel() {
val uiState = savedStateHandle.getStateFlow(KEY_URL, "")
private val url = savedStateHandle.getStateFlow(KEY_URL, "")
private val serviceId = savedStateHandle[KEY_SERVICE_ID] ?: NO_SERVICE_ID

val streamState = url
.map {
try {
Resource.Success(ExtractorHelper.getStreamInfo(serviceId, it, true).await())
} catch (e: Exception) {
Resource.Error(e)
}
}
.flowOn(Dispatchers.IO)
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), Resource.Loading)
Comment on lines +31 to +40
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this result in loading twice StreamInfo when VideoDetailFragment is loaded, as loading calls for player and comments would be concurrent?

If so, then this approach must be not be used due to the time and resources used to fetch info (especially for YouTube streams).

Copy link
Member Author

Choose a reason for hiding this comment

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

Couldn't the cache value be used instead, by setting the boolean to false?

Copy link
Member

Choose a reason for hiding this comment

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

No, the solution to this is to have the VideoDetailFragment pass the channel avatar URL to the comment section (null at the beginning, then set once video info load). This is probably hard to do now, but will be easy once the VDF is also written in Compose.


val commentState = url
.map {
try {
Resource.Success(CommentInfo(CommentsInfo.getInfo(it)))
Expand All @@ -33,7 +51,7 @@ class CommentsViewModel(savedStateHandle: SavedStateHandle) : ViewModel() {
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), Resource.Loading)

@OptIn(ExperimentalCoroutinesApi::class)
val comments = uiState
val comments = commentState
.filterIsInstance<Resource.Success<CommentInfo>>()
.flatMapLatest {
Pager(PagingConfig(pageSize = 20, enablePlaceholders = false)) {
Expand Down