Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add text in default language to feedbackSurveyCompleted callback #4625

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vegaro
Copy link
Contributor

@vegaro vegaro commented Dec 31, 2024

With the changes we did to the localization UI, this callback is kinda broken since the ids are auto-generated, and mean nothing to the developer.

After this PR we'll be sending the English translation of the option as well so it's easier to understand what each option represent. It requires backend changes to send the English/default language translation to the callback as well.

@RevenueCat-Danger-Bot
Copy link

1 Error
🚫 Label the PR using one of the change type labels. If you are not sure which label to use, choose pr:other.
Label Description
pr:feat A new feature. Use along with pr:breaking to force a major release.
pr:fix A bug fix. Use along with pr:force_minor to force a minor release.
pr:other Other changes. Catch-all for anything that doesn't fit the above categories. Releases that only contain this label will not be released. Use along with pr:force_patch, or pr:force_minor to force a patch or minor release.
pr:RevenueCatUI Use along any other tag to mark a PR that only contains RevenueCatUI changes
pr:next_release Preparing a new release
pr:dependencies Updating a dependency
pr:phc_dependencies Updating purchases-hybrid-common dependency

Generated by 🚫 Danger

@vegaro vegaro force-pushed the cc-224-send-english-text-in-survey-option-callback branch from ef87340 to 9e4df29 Compare January 17, 2025 17:44
@vegaro vegaro force-pushed the cc-224-send-english-text-in-survey-option-callback branch from 9e4df29 to 6f14273 Compare January 21, 2025 11:46
Copy link

emerge-tools bot commented Jan 21, 2025

1 build decreased size

Name Version Download Change Install Change Approval
Paywalls
com.revenuecat.PaywallsTester
1.0 (1) 10.6 MB ⬇️ 104.7 kB (-0.98%) 40.0 MB ⬇️ 437.9 kB (-1.09%) N/A

Paywalls 1.0 (1)
com.revenuecat.PaywallsTester

⚖️ Compare build
📦 Install build
⏱️ Analyze build performance

Total install size change: ⬇️ 437.9 kB (-1.09%)
Total download size change: ⬇️ 104.7 kB (-0.98%)

Largest size changes

Item Install Size Change
DYLD.String Table ⬇️ -190.0 kB
Code Signature ⬇️ -11.5 kB
DYLD.Exports ⬇️ -9.0 kB
🗑 RevenueCatUI.PurchaseHistoryViewModel ⬇️ -6.2 kB
Strings.Unmapped ⬇️ -1.7 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

case feedbackSurveyCompleted(_ feedbackSurveyOptionId: String)
/// An option of the feedback survey has been selected
/// - Parameter option: The ``CustomerCenterConfigData.HelpPath.FeedbackSurvey.Option`` that was selected
case feedbackSurveyCompletedWithOption(_ option: CustomerCenterConfigData.HelpPath.FeedbackSurvey.Option)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up changing the name of the case because I coudln't find a way where the Swift compiler wouldn't get confused.

I tried adding another parameter and keeping the name and got this:

Screenshot 2025-01-21 at 12 42 55

I also tried returning the whole option (keeping one parameter only) and got this:
Screenshot 2025-01-21 at 12 58 59

I am also concerned because Switch must be exhaustive, meaning this is a breaking change:
Screenshot 2025-01-21 at 13 03 52

@vegaro vegaro changed the title Add defaultLanguageText to feedbackSurveyCompleted callback Add text in default language to feedbackSurveyCompleted callback Jan 21, 2025
@vegaro vegaro added the pr:fix A bug fix label Jan 21, 2025
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.

2 participants