From da4878d2647e051724b98cfa9fe5b8bf36cb389e Mon Sep 17 00:00:00 2001 From: Su TT Date: Wed, 2 Jul 2025 16:59:51 -0400 Subject: [PATCH] feat(ui):Add ErrorPanel composable to Comments Section, related UI models, and tests --- .../video/comments/CommentSectionErrorTest.kt | 64 ++++++++++ .../org/schabi/newpipe/error/ErrorUIMapper.kt | 14 +++ .../schabi/newpipe/paging/CommentsSource.kt | 3 + .../schabi/newpipe/ui/UiModel/ErrorUiModel.kt | 41 ++++++ .../ui/components/common/ErrorPanel.kt | 117 ++++++++++++++++++ .../components/common/ServiceColoredButton.kt | 54 ++++++++ .../video/comment/CommentErrorHandler.kt | 64 ++++++++++ .../video/comment/CommentSection.kt | 27 ++-- .../newpipe/viewmodels/CommentsViewModel.kt | 1 + .../schabi/newpipe/error/ErrorUiModelTest.kt | 63 ++++++++++ 10 files changed, 436 insertions(+), 12 deletions(-) create mode 100644 app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt create mode 100644 app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt create mode 100644 app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt create mode 100644 app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt new file mode 100644 index 000000000..bcf9641bd --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/video/comments/CommentSectionErrorTest.kt @@ -0,0 +1,64 @@ +package org.schabi.newpipe.ui.components.video.comments + +import android.net.http.NetworkException +import androidx.paging.LoadState +import androidx.test.ext.junit.rules.ActivityScenarioRule +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.MainActivity +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel +import org.schabi.newpipe.viewmodels.util.Resource + +@RunWith(AndroidJUnit4::class) +class CommentSectionErrorTest { + + @get:Rule + val activityRule = ActivityScenarioRule(MainActivity::class.java) + + /** + * Test Resource.Error state - when initial comment info loading fails + */ + @Test + fun testResourceErrorState_ShowsUnableToLoadCommentsUiModel() { + + val networkException = object : NetworkException("Connection attempt timed out", null) { + override fun getErrorCode(): Int = NetworkException.ERROR_CONNECTION_TIMED_OUT + override fun isImmediatelyRetryable() = true + } + val errorResource = Resource.Error(networkException) + assertEquals("Should contain the network exception", networkException, errorResource.throwable) + + val errorModel = UnableToLoadCommentsUiModel(networkException) + val spec = errorModel.spec + + assertEquals("Should have correct message resource", R.string.error_unable_to_load_comments, spec.messageRes) + assertTrue("Should show retry button", spec.showRetry) + assertTrue("Should show report button", spec.showReport) + assertFalse("Should NOT show open in browser button", spec.showOpenInBrowser) + } + + /** + * Test LoadState.Error state - when paging data loading fails + */ + @Test + fun testLoadStateErrorState_ShowsUnableToLoadCommentsUiModel() { + val pagingException = RuntimeException("Paging data loading failed") + val loadStateError = LoadState.Error(pagingException) + + assertEquals("Should contain the paging exception", pagingException, loadStateError.error) + + val errorModel = UnableToLoadCommentsUiModel(pagingException) + val spec = errorModel.spec + + assertEquals("Should have correct message resource", R.string.error_unable_to_load_comments, spec.messageRes) + assertTrue("Should show retry button", spec.showRetry) + assertTrue("Should show report button", spec.showReport) + assertFalse("Should NOT show open in browser button", spec.showOpenInBrowser) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt new file mode 100644 index 000000000..5df92ddfc --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorUIMapper.kt @@ -0,0 +1,14 @@ +package org.schabi.newpipe.error + +import org.schabi.newpipe.ui.UiModel.ErrorUiModel +import org.schabi.newpipe.ui.UiModel.GenericErrorUiModel +import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel + +fun mapThrowableToErrorUiModel(throwable: Throwable, userAction: UserAction? = null): ErrorUiModel { + if (userAction == UserAction.REQUESTED_COMMENTS) { + + return UnableToLoadCommentsUiModel(rawError = throwable) + } + // Other ErrorInfo logic and throwable + user actions + return GenericErrorUiModel(rawError = throwable) +} diff --git a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt index 669485e66..bec4911df 100644 --- a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt +++ b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt @@ -9,6 +9,7 @@ import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfo import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.ui.components.video.comment.CommentInfo +import java.io.IOException class CommentsSource(private val commentInfo: CommentInfo) : PagingSource() { private val service = NewPipe.getService(commentInfo.serviceId) @@ -16,12 +17,14 @@ class CommentsSource(private val commentInfo: CommentInfo) : PagingSource): LoadResult { // params.key is null the first time the load() function is called, so we need to return the // first batch of already-loaded comments + return LoadResult.Error(IOException("💥 forced test error")) if (params.key == null) { return LoadResult.Page(commentInfo.comments, null, commentInfo.nextPage) } else { val info = withContext(Dispatchers.IO) { CommentsInfo.getMoreItems(service, commentInfo.url, params.key) } + return LoadResult.Page(info.items, null, info.nextPage) } } diff --git a/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt b/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt new file mode 100644 index 000000000..6d91c78cd --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/UiModel/ErrorUiModel.kt @@ -0,0 +1,41 @@ +package org.schabi.newpipe.ui.UiModel + +import androidx.compose.runtime.Immutable +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.components.common.ErrorPanelSpec + +/** + * Each concrete case from this hierarchy represents a different failure state that the UI can render with ErrorPanel + */ +@Immutable +sealed interface ErrorUiModel { + val spec: ErrorPanelSpec + val rawError: Throwable? get() = null +} + +/** + * Concrete cases - Comments unable to load, Comments disabled, No connectivity, DNS failure, timeout etc + */ +@Immutable +data class UnableToLoadCommentsUiModel(override val rawError: Throwable?) : ErrorUiModel { + override val spec: ErrorPanelSpec = + ErrorPanelSpec( + messageRes = R.string.error_unable_to_load_comments, + showRetry = true, + showReport = true, + showOpenInBrowser = false + ) +} + +/** + * A generic ErrorUiModel for unhandled cases + */ +@Immutable +data class GenericErrorUiModel(override val rawError: Throwable?) : ErrorUiModel { + override val spec: ErrorPanelSpec = + ErrorPanelSpec( + messageRes = R.string.general_error, + showRetry = true, + showReport = true, + ) +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt new file mode 100644 index 000000000..5f16941d9 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -0,0 +1,117 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.annotation.StringRes +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.wrapContentWidth +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.tooling.preview.Preview +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingExtraLarge +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingLarge +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall + +@Composable +fun ErrorPanel( + spec: ErrorPanelSpec, + onRetry: () -> Unit, + onReport: () -> Unit, + onOpenInBrowser: () -> Unit, + modifier: Modifier = Modifier + +) { + Column( + horizontalAlignment = Alignment.CenterHorizontally, + modifier = Modifier + .wrapContentWidth() + .padding(horizontal = SpacingLarge, vertical = SpacingMedium) + + ) { + + val message = stringResource(spec.messageRes) + + Text( + text = message, + style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), + textAlign = TextAlign.Center + ) + + spec.serviceInfoRes?.let { infoRes -> + Spacer(Modifier.height(SpacingSmall)) + val serviceInfo = stringResource(infoRes) + Text( + text = serviceInfo, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center + ) + } + + spec.serviceExplanationRes?.let { explanationRes -> + Spacer(Modifier.height(SpacingSmall)) + val serviceExplanation = stringResource(explanationRes) + Text( + text = serviceExplanation, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center + ) + } + Spacer(Modifier.height(SpacingMedium)) + if (spec.showReport) { + ServiceColoredButton(onClick = onReport) { + Text(stringResource(R.string.error_snackbar_action).uppercase()) + } + } + if (spec.showRetry) { + ServiceColoredButton(onClick = onRetry) { + Text(stringResource(R.string.retry).uppercase()) + } + } + if (spec.showOpenInBrowser) { + ServiceColoredButton(onClick = onOpenInBrowser) { + Text(stringResource(R.string.open_in_browser).uppercase()) + } + } + Spacer(Modifier.height(SpacingExtraLarge)) + } +} + +data class ErrorPanelSpec( + @StringRes val messageRes: Int, + @StringRes val serviceInfoRes: Int? = null, + val serviceExplanation: String? = null, + @StringRes val serviceExplanationRes: Int? = null, + val showRetry: Boolean = false, + val showReport: Boolean = false, + val showOpenInBrowser: Boolean = false +) + +@Preview(showBackground = true, widthDp = 360, heightDp = 640) + +@Composable +fun ErrorPanelPreview() { + AppTheme { + ErrorPanel( + spec = ErrorPanelSpec( + messageRes = android.R.string.httpErrorBadUrl, + showRetry = true, + showReport = false, + showOpenInBrowser = false + ), + onRetry = {}, + onReport = {}, + onOpenInBrowser = {}, + modifier = Modifier + ) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt new file mode 100644 index 000000000..8b3c41a8a --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt @@ -0,0 +1,54 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.RowScope +import androidx.compose.foundation.layout.wrapContentWidth +import androidx.compose.material3.Button +import androidx.compose.material3.ButtonDefaults +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.RectangleShape +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall + +@Composable +fun ServiceColoredButton( + onClick: () -> Unit, + modifier: Modifier = Modifier, + content: @Composable() RowScope.() -> Unit, +) { + Button( + onClick = onClick, + modifier = modifier.wrapContentWidth(), + colors = ButtonDefaults.buttonColors( + containerColor = MaterialTheme.colorScheme.error, + contentColor = MaterialTheme.colorScheme.onError + ), + contentPadding = PaddingValues(horizontal = SpacingMedium, vertical = SpacingSmall), + shape = RectangleShape, + elevation = ButtonDefaults.buttonElevation( + defaultElevation = 8.dp, + + ), + ) { + content() + } +} + +@Preview +@Composable +fun ServiceColoredButtonPreview() { + AppTheme { + ServiceColoredButton( + onClick = {}, + content = { + Text("Button") + } + ) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt new file mode 100644 index 000000000..152c032fd --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentErrorHandler.kt @@ -0,0 +1,64 @@ +package org.schabi.newpipe.ui.components.video.comment + +import android.util.Log +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.wrapContentHeight +import androidx.compose.runtime.Composable +import androidx.compose.runtime.SideEffect +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.input.nestedscroll.NestedScrollSource.Companion.SideEffect +import androidx.compose.ui.tooling.preview.Preview +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.error.mapThrowableToErrorUiModel +import org.schabi.newpipe.ui.UiModel.ErrorUiModel +import org.schabi.newpipe.ui.components.common.ErrorPanel +import java.io.IOException + +@Composable +fun CommentErrorHandler( + throwable: Throwable, + userAction: UserAction, + onRetry: () -> Unit, + onReport: (ErrorInfo) -> Unit +) { + SideEffect { + Log.d("CommentErrorHandler", "⛔️ rendered for error: ${throwable.message}") + } + + val uiModel: ErrorUiModel = mapThrowableToErrorUiModel(throwable, userAction) + val errorInfo = ErrorInfo( + throwable = throwable, + userAction = userAction, + request = "" + ) + + Box( + modifier = Modifier.fillMaxSize(), + contentAlignment = Alignment.Center + ) { + ErrorPanel( + spec = uiModel.spec, + onRetry = onRetry, + onReport = { onReport(errorInfo) }, + onOpenInBrowser = {}, + modifier = Modifier + .fillMaxWidth() + .wrapContentHeight() + ) + } +} + +@Preview(showBackground = true) +@Composable +fun PreviewCommentErrorHandler() { + CommentErrorHandler( + throwable = IOException("No network"), + userAction = UserAction.REQUESTED_COMMENTS, + onRetry = {}, + onReport = {} + ) +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index a33ffc0ca..327d30f72 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -13,6 +13,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.rememberNestedScrollInteropConnection import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.tooling.preview.Preview @@ -25,6 +26,8 @@ import androidx.paging.compose.collectAsLazyPagingItems import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorUtil +import org.schabi.newpipe.error.UserAction import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.extractor.stream.Description @@ -50,6 +53,7 @@ private fun CommentSection( val comments = commentsFlow.collectAsLazyPagingItems() val nestedScrollInterop = rememberNestedScrollInteropConnection() val state = rememberLazyListState() + val context = LocalContext.current LazyColumnThemedScrollbar(state = state) { LazyColumn( @@ -98,21 +102,22 @@ private fun CommentSection( ) } } - when (comments.loadState.refresh) { is LoadState.Loading -> { item { LoadingIndicator(modifier = Modifier.padding(top = 8.dp)) } } - is LoadState.Error -> { item { - // TODO use error panel instead - EmptyStateComposable(EmptyStateSpec.ErrorLoadingComments) + CommentErrorHandler( + throwable = (comments.loadState.refresh as LoadState.Error).error, + userAction = UserAction.REQUESTED_COMMENTS, + onRetry = { comments.retry() }, + onReport = { info -> ErrorUtil.openActivity(context, info) }, + ) } } - else -> { items(comments.itemCount) { Comment(comment = comments[it]!!) {} @@ -121,15 +126,13 @@ private fun CommentSection( } } } - is Resource.Error -> { item { - // TODO use error panel instead - EmptyStateComposable( - spec = EmptyStateSpec.ErrorLoadingComments, - modifier = Modifier - .fillMaxWidth() - .heightIn(min = 128.dp) + CommentErrorHandler( + throwable = uiState.throwable, + userAction = UserAction.REQUESTED_COMMENTS, + onRetry = { comments.retry() }, + onReport = { info -> ErrorUtil.openActivity(context, info) }, ) } } diff --git a/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt b/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt index 007292498..58886fc68 100644 --- a/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt +++ b/app/src/main/java/org/schabi/newpipe/viewmodels/CommentsViewModel.kt @@ -23,6 +23,7 @@ import org.schabi.newpipe.viewmodels.util.Resource class CommentsViewModel(savedStateHandle: SavedStateHandle) : ViewModel() { val uiState = savedStateHandle.getStateFlow(KEY_URL, "") .map { + // Resource.Error(RuntimeException("Forced error for testing")) try { Resource.Success(CommentInfo(CommentsInfo.getInfo(it))) } catch (e: Exception) { diff --git a/app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt b/app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt new file mode 100644 index 000000000..641f95966 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/error/ErrorUiModelTest.kt @@ -0,0 +1,63 @@ +package org.schabi.newpipe.error + +import android.net.http.NetworkException +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.schabi.newpipe.R +import org.schabi.newpipe.ui.UiModel.GenericErrorUiModel +import org.schabi.newpipe.ui.UiModel.UnableToLoadCommentsUiModel + +class ErrorUiModelTest { + + /** + * Test that when comments fail to load, the correct error panel is rendered + */ + @Test + fun `mapThrowableToErrorUiModel with REQUESTED_COMMENTS returns UnableToLoadCommentsUiModel`() { + // val throwable = RuntimeException("Comments failed to load") + val networkException = object : NetworkException("Connection attempt timed out", null) { + override fun getErrorCode() = NetworkException.ERROR_CONNECTION_TIMED_OUT + override fun isImmediatelyRetryable() = true + } + val result = mapThrowableToErrorUiModel(networkException, UserAction.REQUESTED_COMMENTS) + assertTrue("Result should be UnableToLoadCommentsUiModel", result is UnableToLoadCommentsUiModel) + assertEquals("Raw error should be preserved for debugging", networkException, result.rawError) + } + + /** + * Test the fallback logic + */ + @Test + fun `mapThrowableToErrorUiModel with null UserAction returns GenericErrorUiModel`() { + val throwable = RuntimeException("Test error") + val result = mapThrowableToErrorUiModel(throwable, null) + + assertTrue("Should return GenericErrorUiModel", result is GenericErrorUiModel) + assertEquals("Should preserve the original throwable", throwable, result.rawError) + } + + /** + * Test that UnableToLoadCommentsUiModel maps to the correct error panel configuration + */ + @Test + fun `UnableToLoadCommentsUiModel has correct ErrorPanelSpec`() { + val throwable = RuntimeException("Test error") + val errorModel = UnableToLoadCommentsUiModel(throwable) + val spec = errorModel.spec + // Assert: Verify the spec has the correct configuration for comment loading errors + assertEquals( + "Error message should be 'Unable to load comments'", + R.string.error_unable_to_load_comments, + spec.messageRes, + ) + + assertTrue("Retry button should be shown for comment loading errors", spec.showRetry) + assertTrue("Report button should be shown for comment loading errors", spec.showReport) + assertFalse("Open in browser should NOT be shown for comment loading errors", spec.showOpenInBrowser) + + // Assert: Verify the raw error is set correctly + assertEquals("Raw error should be preserved for debugging", throwable, errorModel.rawError) + } +}