Skip to content

Convert ErrorActivity to compose#13434

Open
Idadelveloper wants to merge 8 commits intorefactorfrom
error-activity
Open

Convert ErrorActivity to compose#13434
Idadelveloper wants to merge 8 commits intorefactorfrom
error-activity

Conversation

@Idadelveloper
Copy link
Copy Markdown

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

Converted ErrorActivity from Android Views to Jetpack Compose. The XML layout and menu are replaced by a Compose screen with all the same sections: error message, device info, stack trace details, user comment field, and report/share actions. The privacy policy dialog was also converted to Compose so it survives rotation and renders correctly with Material 3 theming. A shared BaseActivity was introduced to provide edge-to-edge display and Compose theme setup, serving as the foundation for future activity migrations.

Before/After Screenshots/Screen Record

  • Before:
e2478d1d-c583-4b1b-9979-add6a532cf38 8484d979-f954-44a5-8197-f5f159fa951c
  • After:
8204f199-5e0f-4443-8d72-8cc07d952fe7 e8a61c4d-685b-4670-9688-b3f7b4ac3fc9

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. You can find more info and a video demonstration on this wiki page.

Due diligence

@github-actions github-actions bot added the size/large PRs with less than 750 changed lines label Apr 18, 2026
Comment thread app/src/main/java/org/schabi/newpipe/error/ErrorActivity.kt Outdated
@ShareASmile ShareASmile added code quality Improvements to the codebase to improve the code quality rewrite Issues and PRs related to rewrite labels Apr 19, 2026
@github-project-automation github-project-automation bot moved this to In Progress in Rewrite Apr 19, 2026
@Idadelveloper Idadelveloper requested a review from Stypox April 19, 2026 12:09
Comment thread app/src/main/java/org/schabi/newpipe/ui/components/common/PrivacyPolicyDialog.kt Outdated
Comment thread app/src/main/java/org/schabi/newpipe/error/ErrorActivity.kt Outdated
Comment thread app/src/main/java/org/schabi/newpipe/ui/components/common/PrivacyPolicyDialog.kt Outdated
Comment thread app/src/main/java/org/schabi/newpipe/BaseActivity.kt Outdated
@github-actions github-actions bot added size/giant PRs with more than 750 changed lines and removed size/large PRs with less than 750 changed lines labels Apr 20, 2026
@Idadelveloper
Copy link
Copy Markdown
Author

Idadelveloper commented Apr 20, 2026

@theimpulson please checkout the new changes with the ComposeActivity. Also introduced an ErrorReportHelper containing those methods that were initially in the ErrorActivity. The settings screens already had an implementation with the NavDisplay so decided to just bring it in as well and get rid of the settings activity since this will serve as the central activity for all compose screens. There's the AboutActivity that needs to be gotten rid of as well but I think I will handle that in another pr.

.value("user_comment", comment)
.end()
.done()
} catch (e: Exception) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
} catch (e: Exception) {
} catch (exception: Exception) {

Please don't use one word variable names

activity?.let {
if (it.isTaskRoot) {
it.startActivity(
Intent(it, MainActivity::class.java).apply {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure but won't this keep looping back and forth in case of crash when ErrorActivity is launched by acra?

Comment on lines +72 to +74
containerColor = MaterialTheme.colorScheme.surface,
titleContentColor = MaterialTheme.colorScheme.onSurface,
textContentColor = MaterialTheme.colorScheme.onSurfaceVariant
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are we specifying these colors manually?

Comment on lines +76 to +77
private const val ACTION_EMAIL = "EMAIL"
private const val ACTION_GITHUB = "GITHUB"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please put variables before functions and classes similar to how we structure things in a class

@theimpulson
Copy link
Copy Markdown
Member

@theimpulson please checkout the new changes with the ComposeActivity. Also introduced an ErrorReportHelper containing those methods that were initially in the ErrorActivity. The settings screens already had an implementation with the NavDisplay so decided to just bring it in as well and get rid of the settings activity since this will serve as the central activity for all compose screens. There's the AboutActivity that needs to be gotten rid of as well but I think I will handle that in another pr.

Thank you, looks good to me other than some minor nitpicks and questions on places where I am unsure about the behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code quality Improvements to the codebase to improve the code quality rewrite Issues and PRs related to rewrite size/giant PRs with more than 750 changed lines

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants