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

#168 Service worker #680

Closed
wants to merge 4 commits into from
Closed

#168 Service worker #680

wants to merge 4 commits into from

Conversation

M4ttoF
Copy link
Collaborator

@M4ttoF M4ttoF commented Aug 9, 2018

Included some stuff for firebase which will be used for the push notifications later. It is not completely setup but the service worker still saves caches properly and has no issues.

@M4ttoF M4ttoF force-pushed the 168 branch 2 times, most recently from f7295f7 to 13d658c Compare August 9, 2018 19:56

var cacheFiles = [];

self.addEventListener('install', function(e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is "self" defined?

@@ -0,0 +1,6 @@
importScripts('https://www.gstatic.com/firebasejs/4.8.1/firebase-app.js');
Copy link
Collaborator

@joshi1983 joshi1983 Aug 9, 2018

Choose a reason for hiding this comment

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

I'm not sure where this is coming from but there are some JavaScript errors when visiting the home page of the site. The errors lead to the Google Map not working.

I did not install firebase on app.accesslocator.com. Would that be the cause?

Here's a screenshot of them:
image

I'm going to undeploy the changes now since you ran the audit and made a screenshot.

<script>
// Initialize Firebase
var config = {
apiKey: "AIzaSyBX6l4geba-CjXE_WlQwOX6pxCIrVl-tjk",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know you're still working on this but after you get everything working to your satisfaction, API keys like this one should be moved to .env and be taken from there like we do with the Google Maps API key.

@joshi1983
Copy link
Collaborator

joshi1983 commented Aug 12, 2018

Can you make it so the service worker is active only when the application is run as a PWA?

The start_url in the manifest.json is "/?using_pwa=1". Maybe that can help you differentiate between when it is run as a progressive web application and not.

@M4ttoF
Copy link
Collaborator Author

M4ttoF commented Aug 13, 2018

@joshi1983 Updated the PR

@joshi1983
Copy link
Collaborator

I'm closing this because it has some caching problems and pull request #688 is taking over what this was intended to do.

@joshi1983 joshi1983 closed this Aug 22, 2018
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