-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat: show play update prompt #357
Conversation
View your CI Pipeline Execution ↗ for commit 5189d64.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pamella014
Thanks for making the changes to isolate the code in a service, I think overall is looking in a good state. See a few minor comments inline that it would be good to tidy up, and after that I will try to get a testing version deployed so we can confirm if behaves as expected
apps/picsa-tools/extension-content/src/app/pages/home/extension-home.component.ts
Outdated
Show resolved
Hide resolved
Thank you @chrismclarke for the feedback. I will review and make the necessary changes. Should i create a new pr once i am done? |
@chrismclarke |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the updates @Pamella014 , and again apologies for the delayed review.
This will be merged and included in the next release (3.53.0), so it won't be until one more release later that we will be able to see if working as expected or not as it will only show when a new version is available after the user has this version installed. But will keep an eye out for it and let you know if any follow-up required.
Description
Created an app update service that can be accessed anywhere in the app and then i imported it into the main extension app entry component.
closes #348