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

Clear upgrade subscribed interval after declining #1857

Merged
merged 5 commits into from
Aug 9, 2018

Conversation

doniyor2109
Copy link
Contributor

Closes #1747

@tzarebczan
Copy link
Contributor

@doniyor2109 thanks for for this PR, please see https://lbry.io/faq/tips if you are interested in a tip for your contribution!

Just looking at this by inspection, it may prevent new upgrade modals from popping up if the user clicked decline, but I don't think it prevents multiple upgrade modals from stacking on each other if a user has the app running in the background and an update occurs.

This might do the trick, needs to be tested.
@tzarebczan tzarebczan requested a review from neb-b August 7, 2018 13:33
@lbry-bot lbry-bot assigned neb-b and unassigned neb-b Aug 7, 2018
@doniyor2109
Copy link
Contributor Author

@tzarebczan How can I create new action for app actions?

@tzarebczan
Copy link
Contributor

@doniyor2109 i just submitted a commit which should fix this issue, but haven't tested it yet. I'll let @seanyesmunt review first.

@doniyor2109
Copy link
Contributor Author

@tzarebczan Ok

Create action for clearing upgrade timer
@doniyor2109
Copy link
Contributor Author

doniyor2109 commented Aug 7, 2018

@tzarebczan Can you merge lbryio/lbry-redux#66 and upgrade lbry-redux?

@alyssaoc alyssaoc added this to the Aug 20th (Protocol) milestone Aug 7, 2018
@neb-b
Copy link

neb-b commented Aug 9, 2018

@doniyor2109 Looks good. I merged the lbry-redux PR and bumped the version in this PR. I'll give it a better test tomorrow before merging. Thanks for the contribution!

@neb-b neb-b merged commit f62181d into lbryio:master Aug 9, 2018
@tzarebczan
Copy link
Contributor

Thanks again @doniyor2109, was able to test and it worked!

@doniyor2109
Copy link
Contributor Author

@tzarebczan I am glad to help 👌

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.

4 participants