🔨[refactor] [Configure Vanilla.js app to be a PWA.] #383
Conversation
|
fix issue #85 |
taniashiba
changed the title
taniashiba
changed the title
taniashiba
changed the title
There was a problem hiding this comment.
Heyy @Ankitv003 !! Thanks for the contribution. Just a few merge conflicts to resolve so we can get this approved and merged. Thank you!!
|
|
||
| test('should navigate to Home', async ({ page }) => { | ||
| await page.goto('/'); |
There was a problem hiding this comment.
changed "/" to "http://localhost:5173"
| @@ -101,9 +101,9 @@ export default defineConfig({ | |||
| * Use the preview server on CI for more realistic testing. | |||
| * Playwright will re-use the local server if there is already a dev-server running. | |||
| */ | |||
| command: 'pnpm run dev', | |||
There was a problem hiding this comment.
changed to npm because i am using npm and the testcases were failing
| "tailwindcss": "^3.4.13", | ||
| "vite": "^5.4.1", | ||
| "vitest": "^2.1.2" | ||
| }, | ||
| "packageManager": "[email protected]+sha512.22721b3a11f81661ae1ec68ce1a7b879425a1ca5b991c975b074ac220b187ce56c708fe5db69f4c962c989452eee76c82877f4ee80f474cebd61ee13461b6228" |
There was a problem hiding this comment.
added serwist/next dependency
|
|
||
| // Initial call to router to render the correct component on page load | ||
| document.addEventListener('DOMContentLoaded', router); |
There was a problem hiding this comment.
registered service worker
| @@ -11,7 +11,10 @@ | |||
| name="description" | |||
| content="A Decentralized Web Application template" | |||
| /> | |||
| <link rel="stylesheet" href="dist/output.css" /> | |||
There was a problem hiding this comment.
this line is missing only
|
hey @blackgirlbytes , i have added some comments and i think the only main change required is in index.html file , everything else , either has some extra space or double inverted commas( " ") instead of single(' '). |
|
HI @Ankitv003 . Thanks -- Adding comments wont resolve the conflicts though. Here's some information on merge conflicts:
I cannot change your code for you because it's on your branch. Thank you! |
|
i mean the main change is for index.html , i can change it and then add it for review again , the rest of the files are correct , so you can merge it from your end, right? @blackgirlbytes |
|
@Ankitv003 if you want, you should come to our office hours in Discord..so we could help you. It's saying that you have merge conflicts in the index.html, package.json, main.js, and the main.spec.js. I think you didn't pull the latest from main before you made changes, so you need to make the changes on all those files and then i can review and again and merge on my end :D |
|
hey @blackgirlbytes discord link is not working , what are the office hours ? 👀 I will try to do resolve this by myself today , lets see! |
|
@Ankitv003 hmm okay try discord.gg/tbd and then go to events. You will see our office hours. It's set to happen on Oct 30 at 1pm ET. If you join the discord, you will see it in your timezone |
Ankitv003
requested review from
acekyd,
nitro-neal,
michaelneale,
mistermoe,
jiyoonie9,
finn-block,
angiejones and
EbonyLouis
as code owners
|
made some changes @blackgirlbytes , please check, if it is not ok, i will simply open a new branch,and add a new PR, been on it for most part of the day on a crappy laptop, my head will explode now. |
|
Hey @Ankitv003!!! While I appreciate the updates, I been running the code and it doesn't look like it's working. I'm just seeing a blank page |
|
i think service worker code is missing here , let me check @blackgirlbytes |
|
yeah , service worker registeration in main.js was missing and |
What type of PR is this? (check all applicable)
Description
This PR adds PWA to Vanilla.js App
Related Tickets & Documents
Resolves #85
Mobile & Desktop Screenshots/Recordings
Added tests?
No tests? Add a note
Added to documentation?
No docs? Add a note
added comments in code
[optional] Are there any post-deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?