-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat(docs): add pwa with auto update strategy #573
Conversation
✔️ Deploy Preview for vitest-dev ready! 🔨 Explore the source changes: f378a55 🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/6213477dd96d6900082d91f9 😎 Browse the preview: https://deploy-preview-573--vitest-dev.netlify.app |
Run To run locally just If you don't want to test the pwa, just run
I mean, to test pwa with lighthouse, the pwa works on any browser. |
# Conflicts: # docs/package.json # pnpm-lock.yaml
docs/netlify.toml
Outdated
[[headers]] | ||
for = "/manifest.webmanifest" | ||
[headers.values] | ||
Content-Type = "application/manifest+json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a root-level one. Is this neccessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upps, I don't see it, sorry, we can use the top one adding the webmanifest entry.
Should we use 301 instead 302 on new redirection?
/cc @patak-dev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer 302
<link rel="preconnect" crossorigin="anonymous" href="${jsdelivr}"> | ||
<link rel="preconnect" crossorigin="anonymous" href="${github}"> | ||
<link rel="preconnect" crossorigin="anonymous" href="${avatars}"> | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we automate this? I am not so confident of duplicating (It would make future changes much harder)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can include some module with the site data, including all stuff there: avatars, links, contributors, fonts...
I'll be back next Tuesday, we can merge and then change it, but proceed as you wish
# Conflicts: # pnpm-lock.yaml
# Conflicts: # docs/package.json # pnpm-lock.yaml
|
Looks awesome @userquin! The only detail I see is that avatar images are downscaled too much now and they are seen blurry on my monitor: Maybe we can use a bit higher resolution for them? Same for the team avatars, you can also see the diff with our sponsor images |
scripts/update-contributors.ts
Outdated
'content-type': 'application/json', | ||
}, | ||
}) | ||
const data = await res.json() as Contributor[] || [] | ||
collaborators.push(...data.map(i => i.login)) | ||
collaborators.push(...data.map(({ login, avatar_url }) => { | ||
// optimize the avatar size: check avatar and contributors components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The size doesn't matter much, let's keep the codebase simple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
744KB
This comment has been minimized.
This comment has been minimized.
Looks like snapshot needs updating |
I've disabled the PWA, it seems there is some problem with the sw and the images on home page: we only need to change I've changed all avatars (core team members and contributors) and sponsors images to be With these latest changes, the docs will work as the current docs deployed on netlify with some performance added: lazy load images. I think we can merge with the PWA disabled and then fix the images problem: we'll be able also to start adding the new vitepress theme. To test the docs on the preview you will need to remove the service worker previously installed:
|
reverting last commit, pnpm problem when updating, points to wrong scripts when using nvm |
This reverts commit c4520eb.
Since the sponsors images can be changed I revert fixed size: there will be some jank. |
PWA with EDIT: I need to change pwa plugin to include |
# Conflicts: # docs/package.json # docs/src/components/Home.vue # pnpm-lock.yaml
The PWA is disabled until we fix the contributors avarts CORS problem: to enable PWA we only need to change |
# Conflicts: # docs/src/components/FeaturesList.vue
Images are working well, I was seeing them blurry because the old SW was getting in the middle. I think we should merge this one |
This PR includes:
rel="noopener noreferrer"
@vue/theme
or add a custom themenetlify.toml
(also added: we need to review it)localhost-https
added to test pwa on locallocalhost-https
)scripts/build-pwa.ts
module to rebuild the pwa oncevitepress
finish: also addedesno
to run the module => I run it usingesno
insteadesmo
because it never finish, I don't know what's happing with itPending tasks, will not be fixed here, see #580