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

fix(gui): SubscribeNowPage calls back also when applying manually supplied token. #305

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

CarlosNihelton
Copy link
Contributor

The SubscribeNowPage accepts a callback that must be called when this page causes the current subscription to change, such as when the user purchases a subscription through the store. It turns out that the callback was being called only in such situation, thus the previous name made sense.

But that was a mistake: user manual input also causes the underlying subscription to change, thus that must also call the callback. Hence, the reason for renaming it.

This not only renames it, but also ensures it is called for the manual input case.

for the callback the subscribe now page calls when susbcription changes.

Also, make sure to call it for the manual input token case.

It was previously only used for the purchase through store case.
@CarlosNihelton CarlosNihelton marked this pull request as ready for review October 4, 2023 05:32
@CarlosNihelton CarlosNihelton changed the title maint(gui): onSubscriptionUpdate is a better name for this callback fix(gui): onSubscriptionUpdate is a better name for this callback Oct 4, 2023
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

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

👍

@EduardGomezEscandell
Copy link
Contributor

Not sure the title of the PR reflects this PR, as this is not just a rename.

@CarlosNihelton CarlosNihelton changed the title fix(gui): onSubscriptionUpdate is a better name for this callback fix(gui): SubscribeNowPage calls back also when applying manually supplied token. Oct 4, 2023
@CarlosNihelton
Copy link
Contributor Author

Hard to find a short name :)

@CarlosNihelton CarlosNihelton merged commit 642c5bf into main Oct 4, 2023
33 checks passed
@CarlosNihelton CarlosNihelton deleted the update-subs-on-manual branch October 4, 2023 13:47
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.

2 participants