Skip to content

Disable close button when action is in progress#1528

Merged
vegaro merged 5 commits into
mainfrom
cesar/pwl-428-paywalls-android-disable-the-close-button-when-an-action-is
Dec 20, 2023
Merged

Disable close button when action is in progress#1528
vegaro merged 5 commits into
mainfrom
cesar/pwl-428-paywalls-android-disable-the-close-button-when-an-action-is

Conversation

@vegaro

@vegaro vegaro commented Dec 18, 2023

Copy link
Copy Markdown
Member

@vegaro vegaro added the pr:fix A bug fix label Dec 18, 2023
validatedPaywallData = displayablePaywall,
template = template,
shouldDisplayDismissButton = options.shouldDisplayDismissButton,
actionInProgress = actionInProgress.value,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this here, then realized that we need to update the state in verifyNoActionInProgressOrStartAction(), where actionInProgress is updated. I am not sure there's a more efficient way of doing it. The alternative would be to update the button wherever the PaywallListener is implemented

What do you think @tonidero

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.

Hmm we're already exposing actionInProgress from the PaywallViewModel as a different State. Couldn't we just use that in the InternalPaywall directly? I believe in that case this wiring won't be necessary? Do lmk if I'm missing something!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh that's right, I missed we were exposing that as a state already

@codecov

codecov Bot commented Dec 18, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9d096d7) 84.14% compared to head (c4ac62d) 84.14%.

❗ Current head c4ac62d differs from pull request most recent head a714409. Consider uploading reports for the commit a714409 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1528   +/-   ##
=======================================
  Coverage   84.14%   84.14%           
=======================================
  Files         218      218           
  Lines        7196     7196           
  Branches     1007     1007           
=======================================
  Hits         6055     6055           
  Misses        750      750           
  Partials      391      391           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@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.

Let's release this?

)
}
CloseButton(state.shouldDisplayDismissButton, onDismiss)
CloseButton(state.shouldDisplayDismissButton, viewModel.actionInProgress.value, onDismiss)

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.

Maybe add labels to the parameters here and in InternalPaywall?

IconButton(
onClick = onClick,
modifier = Modifier.align(Alignment.TopStart),
enabled = actionInProgress.not(),

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.

So nice

@vegaro vegaro marked this pull request as ready for review December 20, 2023 09:37
@vegaro vegaro requested a review from tonidero December 20, 2023 09:37

@tonidero tonidero 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.

Looks great!

@vegaro vegaro enabled auto-merge (squash) December 20, 2023 09:48
@vegaro vegaro merged commit 9b6c4ca into main Dec 20, 2023
@vegaro vegaro deleted the cesar/pwl-428-paywalls-android-disable-the-close-button-when-an-action-is branch December 20, 2023 09:55
tonidero pushed a commit that referenced this pull request Dec 21, 2023
**This is an automatic release.**

### RevenueCatUI
* Paywalls: fix empty description when using custom package type and
Offer Period (#1519) via Andy Boedo (@aboedo)
### Bugfixes
* Disable close button when action is in progress (#1528) via Cesar de
la Vega (@vegaro)
### Dependency Updates
* Bump danger from 9.4.1 to 9.4.2 (#1527) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Add revenuecatui docs to reference docs (#1526) via Toni Rico
(@tonidero)

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants