ubershmekel / redditp

Convert any reddit page to a presentation or slide show
https://redditp.com
MIT License
261 stars 102 forks source link

added webmanifest, service worker and necessary icons #116

Open smarek opened 4 years ago

smarek commented 4 years ago

This PR should provide a browser cache for all application resources, service worker is implemented in fetch-first-then-cache manner, so it provides cache for everything that is requested, but currently unavailable, everything that is requested and available will be cached

service worker will not cache Reddit API responses or other external resources, only resources within redditp.com domain (but this can be modified)

smarek commented 4 years ago

Also, since there is no svg form of favicon, i've used fullscreen icon svg to provide application icon

Because this PR allows the redditp to be installed as PWA/extension in browser, or as desktop icon (installed web-app) on Android and iOS devices. If you provide another icon, or if any designer will want to provide one, we can easily switch to new, by altering the images/icon-192.png and images/icon-512.png files

Deployment can be later verified by Lighthouse reporting, specifically the PWA section, see https://googlechrome.github.io/lighthouse/viewer/?psiurl=https%3A%2F%2Fwww.redditp.com

smarek commented 4 years ago

ok, i squashed commits in the branch, sorry for the noise, but currently the site looks the same way, and all the checks in lighthouse PWA section are green, should be good to merge https://googlechrome.github.io/lighthouse/viewer/?psiurl=https://deploy-preview-116--redditp.netlify.app/

ubershmekel commented 4 years ago

Wow, thank you for this work! I'm under water with work on https://github.com/2020PB/police-brutality but I hope to get to reviewing this PR this week.

ubershmekel commented 4 years ago

@smarek I've had some bad experiences with service workers. Eg google analytics registering hits on the first fetch and not on the next, and some assets going stale and not updating. It can be confusing especially when you're working locally and things aren't changing. Here are a few questinos:

Everything else seems great like the mobile friendliness tweaks.

smarek commented 4 years ago

@ubershmekel cheers, well, the general user experience should remain the same, google analytics will not register fetches, that are already cached, but since GA analytics of usage of static assets is not important metric, i'm not sure if that matters

ubershmekel commented 4 years ago

@smarek I would approve if there was no caching at all.

An always fetch policy, or no fetch listener. Any caching logic is just asking for trouble on a site that won't work at all when any sort of connectivity failure arises.

Prefetching is a good idea btw, but we should do that with a <link rel="preload"> https://css-tricks.com/prefetching-preloading-prebrowsing/

smarek commented 4 years ago

Well, you cannot correctly have service-worker without it being able to respond on behalf of the unavailable service server. Without it, you're just using browser caching features.

ubershmekel commented 4 years ago

Sites that have a build step can flush the cache by adding a content hash to resource names. While redditp doesn't have a build system, I'm fearful of introducing caching mechanisms. I've had to uninstall service workers (which means installing a different service worker that uninstalls) and I've gotten confused in various ways when changes I made did/didn't work with service-worker sites. Including situations where I work on site A that has service workers, then site B gets broken because the service worker persists.

You said yourself

added value is for me installability, ie. i can install the redditp in my browser or Android device, without need to open the browser and type the address in

So I'm saying let's make redditp a PWA for your use-case.

If that means to drop the service worker entirely - I would prefer that. Is that OK? Do you think the cache and performance potential gains are worth the risks?

smarek commented 4 years ago

Ok, let's make the redditp PWA, however that requires to have service worker up and running, or at least that is my understanding of https://web.dev/what-are-pwas/