Skip to content

Handle TopicUiState.Error without crashing TopicScreen#2117

Open
saadkhalidkhan wants to merge 2 commits into
android:mainfrom
saadkhalidkhan:fix/topic-screen-error-state
Open

Handle TopicUiState.Error without crashing TopicScreen#2117
saadkhalidkhan wants to merge 2 commits into
android:mainfrom
saadkhalidkhan:fix/topic-screen-error-state

Conversation

@saadkhalidkhan
Copy link
Copy Markdown

@saadkhalidkhan saadkhalidkhan commented May 24, 2026

Summary

  • Replace TODO() in TopicScreen with error UI when topic loading fails
  • Show a user-visible message for news loading errors instead of a hardcoded placeholder
  • Add a compose test covering TopicUiState.Error
  • Fix scrollbar item count to include toolbar and header for news loading/error states

Fixes #2108

Test plan

  • ./gradlew assembleDemoDebug passes
  • ./gradlew :feature:topic:impl:compileDemoDebugAndroidTestKotlin passes
  • Run TopicScreenTest.topicError_whenTopicIsError_isShown on device/emulator

@saadkhalidkhan saadkhalidkhan requested a review from dturner as a code owner May 24, 2026 11:40
@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 24, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Replace TODO() with error UI when topic or news loading fails, and add a compose test for the error state.

Fixes android#2108
@saadkhalidkhan saadkhalidkhan force-pushed the fix/topic-screen-error-state branch from 2ba0cdb to 4ed4992 Compare May 24, 2026 11:42
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements the error state for the topic screen by adding dedicated UI components, localized error strings, and a corresponding UI test. The feedback highlights a potential issue in the topicItemsSize function where the success state logic may not accurately account for all items, which could lead to incorrect scrollbar behavior.

Account for TopicToolbar and TopicHeader when computing itemsAvailable for TopicUiState.Success.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TopicScreen crashes when TopicUiState.Error is emitted (TODO() in production)

1 participant