Skip to content

Conversation

@ThomasSession
Copy link
Collaborator

@ThomasSession ThomasSession commented Oct 30, 2025

More logic and UI for the pro settings page and their various form factors
Added a loading and error state for the pro settings page screens
Properly linking up to the billing api to get prices and format them for display
Gave CTAs access to the subscription state to determine whichstring to display in some cases.

@ThomasSession ThomasSession marked this pull request as ready for review October 31, 2025 04:34
Comment on lines 28 to 30
// show a toast and go back to pro settings home screen
Toast.makeText(LocalContext.current, R.string.errorGeneric, Toast.LENGTH_LONG).show()
onBack()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this belong to a LaunchedEffect? I think for compose you can not assume the composition to be done only once. Because onBack is an async call under the hood, there's possible another recomposition and you'll end up with two toasts and two onBack calls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's ok since we are calling onBack right after but I will still wrap it in a launchedEffect just in case

Comment on lines 30 to 35
val context = LocalContext.current

LaunchedEffect(Unit) {
Toast.makeText(context, R.string.errorGeneric, Toast.LENGTH_LONG).show()
onBack()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should have LaunchedEffect testing the the state as input instead, and this should happen outside of the when

LaunchedEffect(state) {
  if (state is State.Error) {
     // Error stuff
  }
}

@ThomasSession ThomasSession merged commit 12515ad into dev Nov 3, 2025
4 checks passed
@ThomasSession ThomasSession deleted the feature/more-pro-states branch November 3, 2025 00:48
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.

3 participants