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

PWA Support #68

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

PWA Support #68

wants to merge 5 commits into from

Conversation

seanaguinaga
Copy link

https://new-connect-beta.vercel.app/

Add to home screen

turn on airplane mode to see offline fallback - not sure what you exactly want with this?

Assets are all generated from the two logo svgs - run build to see them in the dist folder

Copy link

github-actions bot commented Jul 12, 2024

deployed preview: https://68.connect-d5y.pages.dev

Welcome to new-connect! Make sure to:

  • read the contributing guidelines
  • mark your PR as a draft until it's ready to review
  • post the preview on Discord; feedback from users will speedup the PR review

Mobile

Desktop

@@ -0,0 +1,5 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

not related to PWAs


manifest: {
name: 'connect',
short_name: 'conect',
Copy link
Contributor

Choose a reason for hiding this comment

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

misspelled

@@ -0,0 +1,3 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

also unrelated?

Copy link
Author

Choose a reason for hiding this comment

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

Will remove - was just for my testing on vercel

Comment on lines +24 to +28
"@vite-pwa/assets-generator": "^0.2.4",
"vite-plugin-pwa": "^0.20.0",
"sharp": "0.32.6",
"sharp-ico": "0.1.5",
"workbox-window": "^7.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

what do these 5 new dependencies do?

Copy link
Author

Choose a reason for hiding this comment

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

These are required for the minimal config vite-pwa plugin.

They are used generate the assets like splash screens, icons, etc. at build time and are not included in the applications bundle.

There's an alternative way to use "resolutions" in package.json for the sharp and sharp-ico libs but I couldn't get them to work in this repo.

Copy link
Author

Choose a reason for hiding this comment

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

It's a declarative plugin instead of an imperative config.

@adeebshihadeh
Copy link
Contributor

This PR is +100 lines net, including a bunch of new dependencies and configs. I suspect doing all the PWA stuff ourselves would be less than or equal to this.

@adeebshihadeh adeebshihadeh marked this pull request as draft July 14, 2024 05:41
@seanaguinaga
Copy link
Author

This PR is +100 lines net, including a bunch of new dependencies and configs. I suspect doing all the PWA stuff ourselves would be less than or equal to this.

Adding the splash screen meta tags is 60 lines alone in the html
Listing all the device sizes, names etc. etc.

Maintaining a list of sizes of assets for new devices, configuring service workers, etc. is a lot of grunt work that is not something I think any team should really be focused on maintaining or generating.

Much more than 100 lines unfortunately

@griffinbaker12 griffinbaker12 mentioned this pull request Aug 1, 2024
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