Skip to content

Paywalls: Small optimization purchase button#1375

Closed
tonidero wants to merge 1 commit into
paywallsfrom
small-optimization-purchase-button
Closed

Paywalls: Small optimization purchase button#1375
tonidero wants to merge 1 commit into
paywallsfrom
small-optimization-purchase-button

Conversation

@tonidero

Copy link
Copy Markdown
Contributor

Description

This changes the animations for the purchase button to use AnimatedVisibility which actually removes the composable when the animation ends, making sure the progress indicator is not rendered without need, causing CPU usage

@tonidero tonidero marked this pull request as ready for review October 19, 2023 20:30
@tonidero tonidero requested a review from a team October 19, 2023 20:30
colors: TemplateConfiguration.Colors,
) {
val localization = selectedPackage.localization
AnimatedVisibility(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

<3 this is so much better

@tonidero tonidero enabled auto-merge (squash) October 19, 2023 20:35
@NachoSoto

Copy link
Copy Markdown
Contributor

This is a huge find, thank you @tonidero! 🙏🏻

@NachoSoto NachoSoto disabled auto-merge October 19, 2023 20:41

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually I think we need to use the existing implementation

I was in the process of changing ConsistentPackageContentView to use AnimatedVisibility as well, but I realized that it removes elements from the layout. It's important that we don't do that because we need them to be hidden and contribute to the overall layout so that package transitions are consistent and don't move content around.

@tonidero

Copy link
Copy Markdown
Contributor Author

Oh hmm then maybe we can use it only for the progress indicator, which was causing the issue? Will confirm tomorrow, thanks for catching that?

@NachoSoto

Copy link
Copy Markdown
Contributor

I noticed that the purchase button was changing height now because of that as well.
I'm trying to find another solution for this 🙏🏻 no worries though, we'll figure this out! Thanks for finding the source of the performance issue.

NachoSoto added a commit that referenced this pull request Oct 19, 2023
Extracted part of #1375. This uses `AnimatedVisibility` on the `CircularProgressIndicator` so that when it's not visible it's actually removed and the animation doesn't take CPU time.
It uses `Modifier.matchParentSize` to ensure that when it's presented it doesn't contribute to the layout and abruptly resize the `PurchaseButton`.
@NachoSoto

Copy link
Copy Markdown
Contributor

@tonidero new approach: #1376

@tonidero

Copy link
Copy Markdown
Contributor Author

Closing in favor of #1376

@tonidero tonidero closed this Oct 20, 2023
@tonidero tonidero deleted the small-optimization-purchase-button branch October 20, 2023 07:13
@vegaro vegaro added pr:other and removed pr:perf labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants