Implement PWA for mobile experience #282
Conversation
* chore: add serviceworker-rails gem to Gemfile * chore: add .ruby-version file with version 3.2.3 * chore: add favicon.svg, android-chrome-192x192.png, android-chrome-512x512.png, apple-touch-icon.png, and manifest.json files * chore: update application_helper.rb with meta_tag, charset_tag, and viewport_tag methods * chore: update application/_head.html.erb with charset_tag and viewport_tag * chore: update application/head/_meta_tags.html.erb with meta tags * chore: update application/head/_favicons.html.erb with favicon links * chore: update layouts/application.html.erb with title tag * chore: update layouts/settings.html.erb with title tag
…ML-encoded entities to display and add tests for verification.
…de_proc implementation * fix(redcarpet/custom_html_renderer.rb): remove unused block_code method
… block_code * fix(messages_helper.rb): remove unnecessary empty line * fix(custom_html_renderer.rb): remove unused initialize method * fix(custom_html_renderer.rb): remove unnecessary empty line
Co-authored-by: Jason Schulz <jason@schulz.studio>
Co-authored-by: Jason Schulz <jason@schulz.studio>
…ML-encoded entities to display and add tests for verification.
… block_code * fix(messages_helper.rb): remove unnecessary empty line * fix(custom_html_renderer.rb): remove unused initialize method * fix(custom_html_renderer.rb): remove unnecessary empty line
… block_code * fix(messages_helper.rb): remove unnecessary empty line * fix(custom_html_renderer.rb): remove unused initialize method * fix(custom_html_renderer.rb): remove unnecessary empty line
… block_code * fix(messages_helper.rb): remove unnecessary empty line * fix(custom_html_renderer.rb): remove unused initialize method * fix(custom_html_renderer.rb): remove unnecessary empty line
… block_code * fix(messages_helper.rb): remove unnecessary empty line * fix(custom_html_renderer.rb): remove unused initialize method * fix(custom_html_renderer.rb): remove unnecessary empty line
|
@krschacht what can you tell me about the |
|
It’s a flaky test that has to do with parallel test runners. Just rerun CI
a couple times and it should pass.
In my branch I have another attempted fix to increase reliability of it so
no need to investigate.
…On Fri, Apr 19, 2024 at 7:12 PM Jason Schulz ***@***.***> wrote:
@krschacht <https://github.com/krschacht> what can you tell me about the The
section #messages was able to move down so it was not at the bottom.
error?
—
Reply to this email directly, view it on GitHub
<#282 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAIR5M6VGUIGQHTHJWLY2DY6HFKXAVCNFSM6AAAAABGPN5DWKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXGUYTIOBZHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
app/javascript/serviceworker.js
Outdated
| // Perform install steps | ||
| event.waitUntil( | ||
| caches.open('static').then(function (cache) { | ||
| return cache.addAll(['/', '/index.html', '/css/style.css', '/js/app.js']) |
There was a problem hiding this comment.
@jasonpaulso I'm not super familiar with service workers, but it looks like this is caching some specific files. Will these files be properly reloaded when new versions are pushed? I'm noticing those don't have the usual rails asset pipeline cache busting stuff within the filenames.
There was a problem hiding this comment.
Apologies for the confusion with that file—the service worker stuff is somewhat new to me as well, in particular with how it works in Rails. I got a bit overzealous and ultimately forgot to revisit that part when before I submitted the pull request. My mistake! It is safe to disregard that part for now. It's not strictly necessary (and isn't really doing anything yet). I should have time to properly flesh that out tomorrow.
There was a problem hiding this comment.
@jasonpaulso I pulled this branch down and did a bunch of testing on my local machine. It's working really well, nice work on this. I basically think we should deploy it as-is and I used this opportunity to document a bunch of further improvements we can make to the mobile version of the app but that's stuff we should look at later.
However, take a look at a few comments I left just to make sure these things are okay.
| @@ -0,0 +1,3 @@ | |||
| <%= favicon_link_tag asset_path("/favicon.svg"), rel: 'icon', type: 'image/svg+xml' %> | |||
| <%= favicon_link_tag asset_path("/favicon.png"), rel: 'icon', type: 'image/png' %> | |||
There was a problem hiding this comment.
We were previously serving only the favicon.png but it was being done from the asset_path. I added that helper back in and moved PNG into that directory. Is that okay? I assume so.
I also updated the SVG so it would have a transparent background in order to match the PNG. But I did not know how to do this quickly from the ICO file so I simply deleted it. Do you think we need to have an ICO file as well?
There was a problem hiding this comment.
I was encountering a minor issue with regard to the favicon in the asset_path. When using Safari, if you click the "share" button and then look at the terminal output you should see routing errors like ActionController::RoutingError (No route matches [GET] "/favicon.png") and ActionController::RoutingError (No route matches [GET] "/favicon.svg") despite the files existing in the asset path.
I do not think we need the ico file. I used a figma plugin to generate the icons and it was included, so I dropped it in there.
|
@jasonpaulso I really appreciate how knowledgable you are about this PWA stuff! I just caught up on your comments and it sounds like you have an additional change to make to the serviceworker? I won't merge this in quite yet. I'm about to update the asset_path() change that I made simply to undo it and clean up the old files. I'll push that small commit to this branch shortly. Aside from those two things, there are two items on my "Future PWA improvements" list that I want to call your attention to. What do you think of these two? The first on that list is about the overall icon seeming a bit tight (favicon is good, I just mean app icon). What's your opinion on that? This is admittedly a bit nitpicky so maybe it's just me, and plus, I don't know how easy that is to fix. The second is probably a bug with the ios-pwa-splash unpkg link? Again, I don't know how hard that might be to fix. I'm okay leaving both those issues in this PR and we can fix going forward, but chime in and let me know what you think because if either is easy it may be worth fixing now. |
|
I'd say we can just remove the service worker file and gem for now. There isn't much use for it yet, considering the app has no push notifications and doesn't really work without network connection. As far as the rest, I cannot repro the iPad issue—on my iPad and simulator the splash screen dismisses too quickly to get a look at it. There are other options we can look into instead of ios-pwa-splash though. Regarding the icon, I agree that it's not so comfortable on the home screen or dock. I can make some quick changes to it and we can see how it feels. |
* chore(Gemfile.lock): remove 'serviceworker-rails' dependency * chore(serviceworker.js): remove serviceworker.js file * asset(apple-touch-icon.png): replacing icons with more padding
|
Merging in! I love that it's actually usable on mobile now. :) It was pretty painful to use the app before with the browser toolbar present. |
Closes #205 (hopefully...)
<% content_for :title ... %>to home and settings views