[WIP] add Web-App Manifest (fixes #274) #317
Conversation
src/hub.html
Outdated
| @@ -294,9 +318,9 @@ | |||
| hand-controls2="right" | |||
| tracked-controls | |||
| teleport-controls=" | |||
| cameraRig: #player-rig; | |||
| @@ -17,6 +19,28 @@ | |||
| <% } else { %> | |||
| <script src="https://cdn.rawgit.com/brianpeiris/aframe/845825ae694449524c185c44a314d361eead4680/dist/aframe-master.js"></script> | |||
| <% } %> | |||
There was a problem hiding this comment.
We actually inject these tags server-side, since the contents are dynamic (right now, includes the name of the room) see: Hubs-Foundation/reticulum#23
We can update the reticulum code for any missing tags you've included here. Eventually some of these (like the thumbnail) will be informed by the current state of the room, such as the environment choice and number of CCU.
There was a problem hiding this comment.
Also worth noting that we plan on ripping out almost everything from this file and having it be rendered client side with react, at which point this will be just a basic stub and reticulum will have a proper templated server side view for the whole page (to inject these tags, point to the proper HEAD deployed JS/CSS asset, etc.)
There was a problem hiding this comment.
oh thanks! I was wondering where; this makes more sense!
it's useful to have the HTML document's skeleton (and ideally critical-path assets, like inlined CSS and minimal JS to make the page interactive).
what about server-side rendering with React? just curious about the desire to move more to the client.
There was a problem hiding this comment.
I'm not sure what's involved to do that with Phoenix, my guess is that it is probably prohibitive since this is running on the BEAM VM. I agree that would be a good approach if we were running on a JS VM though.
There was a problem hiding this comment.
Oh, and the desire to move it all into the client is basically just to decouple our reticulum build/deploy process from the hubs project's build process. Once this is done there's a straight shot to being able to a) remove the coupling, so builds and deploys are faster and b) it cleans up the self-hosting story, since reticulum right now is coupled with the "burned in" asset URLs in the client code at package build time, so we can't just tell people to run our Habitat package to get things working.
This is being tracked here: #347
webpack.config.js
Outdated
| @@ -206,6 +206,18 @@ const config = { | |||
| to: "favicon.ico" | |||
| } | |||
| ]), | |||
| new CopyWebpackPlugin([ | |||
There was a problem hiding this comment.
We'll need to whitelist these in reticulum here so they will be served statically from origin: https://github.com/mozilla/reticulum/blob/master/lib/ret_web/endpoint.ex#L16
If we want these CDN'ed, we'll need to digest them and presumably add some absolute URLs somewhere to point the browser to them (I'm too unfamililar with the spec to know how to do this offhand but I'm guessing it's possible?)
There was a problem hiding this comment.
the URLs for any src value in the manifest (e.g., for icons and screenshots) can be local or external. local URLs are relative to the manifest URL (just FYI).
I'll follow up with this. I think I'll try this small webpack-pwa-manifest Webpack plugin, and see how that works out. thanks for the helpful comments!
| @@ -4,9 +4,31 @@ | |||
| <head> | |||
| <meta charset="utf-8"> | |||
| <meta name="viewport" content="width=device-width, initial-scale=1"> | |||
| <link rel="shortcut icon" type="image/png" href="/favicon.ico"/> | |||
There was a problem hiding this comment.
Totally forgot to add these! Good thinking. (These don't require dynamic injection like the one listed above, obviously.)
| "short_name": "Hubs", | ||
| "icons": [ | ||
| { | ||
| "src": "https://assets-prod.reticulum.io/assets/images/icon.png", |
There was a problem hiding this comment.
Ideally this would be digested, so it will get busted out of CDN if we change it.
There was a problem hiding this comment.
(Since this is a slowly changing asset, and it might be quite complex to convince webpack to inject it properly here, I'm not opposed to us punting and just manually busting the CDN if it changes. Worth doing if its easy though.)
There was a problem hiding this comment.
most definitely. I'm going to try webpack-pwa-manifest, so I don't reinvent the wheel with this.
(slightly dependent on issue #316)