Skip to content

Select default package on Sheet dismissal#2861

Merged
vegaro merged 3 commits into
mainfrom
mon-1541-reset-package-selection-after-closing-the-paywall-sheet
Nov 24, 2025
Merged

Select default package on Sheet dismissal#2861
vegaro merged 3 commits into
mainfrom
mon-1541-reset-package-selection-after-closing-the-paywall-sheet

Conversation

@vegaro

@vegaro vegaro commented Nov 20, 2025

Copy link
Copy Markdown
Member

Equivalent of RevenueCat/purchases-ios#5797

Setting the default package back to default after dismissing Sheet

@vegaro vegaro requested a review from a team as a code owner November 20, 2025 16:29
@vegaro vegaro force-pushed the mon-1541-reset-package-selection-after-closing-the-paywall-sheet branch from 1315085 to 889e48b Compare November 20, 2025 16:31
@vegaro vegaro changed the title Mon 1541 reset package selection after closing the paywall sheet Select default package on Sheet dismissal Nov 20, 2025
@vegaro vegaro requested a review from a team November 20, 2025 16:31
@codecov

codecov Bot commented Nov 20, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.23%. Comparing base (af896fe) to head (8c0e159).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2861   +/-   ##
=======================================
  Coverage   78.23%   78.23%           
=======================================
  Files         323      323           
  Lines       12711    12711           
  Branches     1736     1736           
=======================================
  Hits         9945     9945           
  Misses       2036     2036           
  Partials      730      730           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Just a question... but code looks good to me!

Comment on lines +269 to +271
return initialSelectedPackageOutsideTabs
?: selectedPackageByTab[selectedTabIndex]
?: packages.packagesByTab[selectedTabIndex]?.firstOrNull()?.pkg

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.

I don't have a lot of context, but should this logic be inverted? Like:

Suggested change
return initialSelectedPackageOutsideTabs
?: selectedPackageByTab[selectedTabIndex]
?: packages.packagesByTab[selectedTabIndex]?.firstOrNull()?.pkg
return packages.packagesByTab[selectedTabIndex]?.firstOrNull()?.pkg
?: selectedPackageByTab[selectedTabIndex]
?: initialSelectedPackageOutsideTabs

Just thinking that we should try to keep the previously selected package within the tab after dismissing the sheet? Not sure if this makes a lot of sense though, I think I'm missing context on the issue.

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.

Let me record a video and debug here but if I understand correctly, the tab is the same within the sheet, so selectedPackageByTab​ will be actually the one in the sheet? And we want to as soon as the sheeet is closed, select the default package. Let me record a video and see if I can explain it better. And also debug because I might be wrong

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 I think I get what you are saying now after doing some debugging. You're saying if we should select back what it was selected before opening the sheet.

So we have decided to just select the default package back when closing the sheet. That's why initialSelectedPacakageOutsideTabs​ takes preference. That's what I've done in iOS as well.

One thing though... debugging a bit closely I am noticing that selectedPackageByTab​ is empty and packagesByTab​ as well. I find that strange... but maybe that only exists if there are tabs? In the Paywall I am using for testing I just have a package list and a button that opens the sheet

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.

selectedPackageByTab​ is empty and packagesByTab​ as well

I do think that's normal if the paywall doesn't have tabs. Those are only used in that case.

So we have decided to just select the default package back when closing the sheet.

Ok, we should probably test what happens in case we have tabs with different packages on each tab (to make sure we return to the default tab + default package within that tab. But I think in general the code makes sense!

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.

Can confirm that the *ByTab things are only used if the paywall has tabs.

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.

made a little change and the logic is now:

  • First select default pacakge in tab
  • If null select default package outside of tabs

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

One more question about the behavior, but I think it looks good!

@vegaro vegaro added this pull request to the merge queue Nov 24, 2025
@emerge-tools

emerge-tools Bot commented Nov 24, 2025

Copy link
Copy Markdown

📸 Snapshot Test

570 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
TestPurchasesUIAndroidCompatibility
com.revenuecat.testpurchasesuiandroidcompatibility
0 0 0 0 312 0 N/A
TestPurchasesUIAndroidCompatibility Paparazzi
com.revenuecat.testpurchasesuiandroidcompatibility.paparazzi
0 0 0 0 258 0 N/A

🛸 Powered by Emerge Tools

@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Nov 24, 2025
@vegaro vegaro added this pull request to the merge queue Nov 24, 2025
Merged via the queue into main with commit d695e01 Nov 24, 2025
23 checks passed
@vegaro vegaro deleted the mon-1541-reset-package-selection-after-closing-the-paywall-sheet branch November 24, 2025 18:55
github-merge-queue Bot pushed a commit that referenced this pull request Nov 25, 2025
**This is an automatic release.**

> [!WARNING]  
> If you don't have any login system in your app, please make sure your
one-time purchase products have been correctly configured in the
RevenueCat dashboard as either consumable or non-consumable. If they're
incorrectly configured as consumables, RevenueCat will consume these
purchases. This means that users won't be able to restore them from
version 9.0.0 onward.
> Non-consumables are products that are meant to be bought only once,
for example, lifetime subscriptions.


## RevenueCat SDK
### 🐞 Bugfixes
* Restore Purchases config automatically in CustomerCenter (#2867) via
Facundo Menzella (@facumenzella)
* Handle error reading `errorStream` in some devices (#2865) via Toni
Rico (@tonidero)
* [MON-1122] Revert variable rounding logic to not round up (#2857) via
Pol Piella Abadia (@polpielladev)

## RevenueCatUI SDK
### Paywallv2
#### 🐞 Bugfixes
* Select default package on Sheet dismissal (#2861) via Cesar de la Vega
(@vegaro)
### Customer Center
#### ✨ New Features
* CC-581 | Allow for support ticket creation (#2810) via Rosie Watson
(@RosieWatson)

### 🔄 Other Changes
* Bump fastlane-plugin-revenuecat_internal from `7328ea7` to `efca663`
(#2864) via dependabot[bot] (@dependabot[bot])
* Bump fastlane from 2.228.0 to 2.229.0 (#2863) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane-plugin-revenuecat_internal from `083ced9` to `7328ea7`
(#2862) via dependabot[bot] (@dependabot[bot])
* Runs plugin actions from correct directory (#2858) via JayShortway
(@JayShortway)
* Flush multiple event batches (#2842) via Toni Rico (@tonidero)
* Add file size limit to events tracking files (#2841) via Toni Rico
(@tonidero)
* Make events manager be supported in Android < 24 (#2854) via Toni Rico
(@tonidero)
* Add non paid revenue reporting infra (#2728) via Toni Rico (@tonidero)
* Fix backend integration tests (#2860) via Toni Rico (@tonidero)
* Track `connection_error_reason` property in diagnostics (#2855) via
Toni Rico (@tonidero)
* Uses some git+GitHub lanes from Fastlane plugin (#2856) via
JayShortway (@JayShortway)
* Add client side timeout logic for endpoints that support fallback URLs
(#2807) via Toni Rico (@tonidero)
* [EXTERNAL] Fix deprecation warnings in examples module (#2852)
contributed by @gojoel (#2853) via Toni Rico (@tonidero)
* Bump fastlane-plugin-revenuecat_internal from `9f78bb9` to `083ced9`
(#2848) via dependabot[bot] (@dependabot[bot])

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants