feat: implement custom mobile PWA install prompt with dual-lock devic…#85
Conversation
|
@the-madhankumar is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thank you for your Pull Request! We're thrilled to have your contribution to FreshScan AI. Before we review, please make sure you have:
A maintainer will review your code as soon as possible! |
|
Frontend lint and build test are failing fix that. |
|
The lock file is missing three packages that are declared in your package.json: @emnapi/core@1.10.0 Problem: The npm ci command failed because package.json and package-lock.json are out of sync. The lock file is missing these packages: @emnapi/core@1.10.0 |
|
Their are multiple issues:
|
|
Hi Sir, Thank you for the detailed review and feedback. I have addressed the items mentioned in the review:
While running I have attached the lint output/screenshots for reference. Could you please let me know if this approach is acceptable, or if you would prefer a different implementation for the I am really sorry, Sir, for the number of issues you found. I am still learning and trying to improve with every review and piece of feedback. I apologize if I am not the best developer yet, but I am doing my best to learn from these mistakes and become better. Thank you for your patience, guidance, and support. 
After page initialization logic fix 
|
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Install Prompt Component Implementation src/components/InstallPrompt.tsx |
New React component detects iOS (via navigator.standalone) and Android (via beforeinstallprompt event), manages prompt state and visibility, defers the browser install event, triggers the native install UI on user confirmation, and renders a dismissible bottom banner with platform-specific messaging and buttons. |
App Integration & PWA Configuration src/App.tsx, index.html, public/manifest.json |
InstallPrompt component is imported and rendered at root App level outside Routes. Manifest icon entries are updated from GIF to PNG format (192x192 and 512x512). HTML head structure is reformatted while preserving all manifest, preconnect, and font resources. |
Auth Flow Query Parameter Handling
| Layer / File(s) | Summary |
|---|---|
Query Parameter-Based State Initialization src/pages/AuthPage.tsx |
AuthPage now derives initial status and errorMsg from URL error and access_token query parameters on first render. In the OAuth callback effect, the error parameter branch now clears the query string by replacing the URL with /auth instead of explicitly updating React state. Minor formatting changes in Google login error handling. |
🎯 3 (Moderate) | ⏱️ ~20 minutes
🐰 A prompt to install, how keen!
iOS, Android, all seen.
Query params take the lead,
Auth flows faster, indeed!
Fresh Scan hops to the screen! 🚀
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately reflects the main change: implementing a custom mobile PWA install prompt. It is specific, concise, and directly relates to the primary feature being added across multiple modified files. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/AuthPage.tsx (1)
50-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
posthogfrom dependency array.The effect body does not reference
posthog, so including it in the dependency array can trigger unnecessary re-executions and is misleading to readers.- }, [navigate, posthog]); + }, [navigate]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AuthPage.tsx` at line 50, In the AuthPage component's useEffect, remove the unused posthog identifier from the dependency array so the effect depends only on navigate; locate the useEffect (references to useEffect, navigate and posthog) and change the dependency array from [navigate, posthog] to [navigate] to prevent unnecessary re-runs and reflect actual dependencies.
🧹 Nitpick comments (1)
public/sw.js (1)
1-5: Consider caching bundled assets for full offline support.The current cache only includes
/and/index.html, which means JS/CSS bundles and other assets loaded byindex.htmlwill fail when offline. For a complete PWA offline experience, you should expandASSETS_TO_CACHEto include all critical assets emitted by your build (e.g., the hashed JS/CSS bundles in/assets/*).One approach: during the build step, generate a list of build artifacts and inject them into
sw.js, or use a tool like Workbox to automate this.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/sw.js` around lines 1 - 5, The service worker currently only caches "/" and "/index.html" via CACHE_NAME and ASSETS_TO_CACHE, so JS/CSS bundles and other build artifacts won't be available offline; update the ASSETS_TO_CACHE array to include all critical emitted assets (e.g., hashed bundles under /assets/*) or modify your build to inject a generated asset manifest into the sw (or switch to Workbox) and use that manifest to populate ASSETS_TO_CACHE at install time so the service worker precaches the full set of runtime assets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@index.html`:
- Line 1: The document is missing the HTML5 DOCTYPE which can trigger quirks
mode; add the declaration <!DOCTYPE html> as the very first line before the
existing <html lang="en"> tag in index.html so browsers render in standards mode
and avoid inconsistent layout.
- Line 5: Update the favicon link in index.html so the tab icon matches the PWA
manifest icons: replace the current <link rel="icon" ... href="/fish.gif">
reference with a link to one of the manifest assets (e.g., /image_192.png or
/image_512.png) or add multiple rel="icon" links for different sizes to mirror
manifest.json entries; ensure the hrefs exactly match the filenames used in
manifest.json (image_192.png, image_512.png) so the browser tab icon and
installed PWA icons stay in sync.
---
Outside diff comments:
In `@src/pages/AuthPage.tsx`:
- Line 50: In the AuthPage component's useEffect, remove the unused posthog
identifier from the dependency array so the effect depends only on navigate;
locate the useEffect (references to useEffect, navigate and posthog) and change
the dependency array from [navigate, posthog] to [navigate] to prevent
unnecessary re-runs and reflect actual dependencies.
---
Nitpick comments:
In `@public/sw.js`:
- Around line 1-5: The service worker currently only caches "/" and
"/index.html" via CACHE_NAME and ASSETS_TO_CACHE, so JS/CSS bundles and other
build artifacts won't be available offline; update the ASSETS_TO_CACHE array to
include all critical emitted assets (e.g., hashed bundles under /assets/*) or
modify your build to inject a generated asset manifest into the sw (or switch to
Workbox) and use that manifest to populate ASSETS_TO_CACHE at install time so
the service worker precaches the full set of runtime assets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e9266e5-e3ec-4186-9afc-1fca3691bc2f
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/image_192.pngis excluded by!**/*.pngpublic/image_512.pngis excluded by!**/*.png
📒 Files selected for processing (7)
index.htmlpublic/manifest.jsonpublic/sw.jssrc/App.tsxsrc/components/InstallPrompt.tsxsrc/main.tsxsrc/pages/AuthPage.tsx
| @@ -1,19 +1,24 @@ | |||
| <!doctype html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
Add the missing DOCTYPE declaration.
HTML5 documents should begin with <!DOCTYPE html>. Without it, browsers may enter quirks mode, leading to inconsistent rendering behavior across platforms.
🔧 Proposed fix
+<!DOCTYPE html>
<html lang="en">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <html lang="en"> | |
| <!DOCTYPE html> | |
| <html lang="en"> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@index.html` at line 1, The document is missing the HTML5 DOCTYPE which can
trigger quirks mode; add the declaration <!DOCTYPE html> as the very first line
before the existing <html lang="en"> tag in index.html so browsers render in
standards mode and avoid inconsistent layout.
|
|
||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/gif" href="/fish.gif" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the new PNG icon files exist in the public directory
fd -t f '^(image_192\.png|image_512\.png|fish\.gif)$' public/Repository: jpdevhub/FreshScanAi
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find manifest file referenced by index.html
echo "== index.html manifest references =="
rg -n --hidden --no-ignore -S '<link[^>]+rel=["'\'']manifest["'\'']' -g'index.html' . || true
# Locate manifest file(s)
echo
echo "== candidate manifest files =="
fd -t f -e json -e webmanifest -e manifest -d 4 'manifest*.{json,webmanifest}' . || true
fd -t f -d 4 -e json -e webmanifest '*manifest*.{json,webmanifest}' . || true
# Print the manifest icon entries (paths only)
echo
echo "== manifest icon references (paths) =="
for f in $(fd -t f 'manifest*.json' . -0 2>/dev/null | tr '\0' '\n' || true); do
echo "--- $f ---"
# extract icon src fields and sizes
rg -n '"icons"\s*:' "$f" || true
rg -n '"src"\s*:' "$f" || true
done
for f in $(fd -t f 'manifest*.webmanifest' . -0 2>/dev/null | tr '\0' '\n' || true); do
echo "--- $f ---"
rg -n '"src"\s*:' "$f" || true
done
# Show the favicon line from index.html
echo
echo "== favicon line in index.html =="
rg -n --hidden --no-ignore -S '<link[^>]+rel=["'\'']icon["'\'']' index.html || trueRepository: jpdevhub/FreshScanAi
Length of output: 1097
Fix favicon to match PWA manifest icon assets.
index.html still sets the tab favicon to /fish.gif, while public/manifest.json references /image_192.png and /image_512.png for the installed PWA icons. This leaves the browser-tab icon out of sync with the PWA icon set.
🔧 Recommended fix to align favicon with manifest
- <link rel="icon" type="image/gif" href="/fish.gif" />
+ <link rel="icon" type="image/png" href="/image_192.png" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <link rel="icon" type="image/gif" href="/fish.gif" /> | |
| <link rel="icon" type="image/png" href="/image_192.png" /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@index.html` at line 5, Update the favicon link in index.html so the tab icon
matches the PWA manifest icons: replace the current <link rel="icon" ...
href="/fish.gif"> reference with a link to one of the manifest assets (e.g.,
/image_192.png or /image_512.png) or add multiple rel="icon" links for different
sizes to mirror manifest.json entries; ensure the hrefs exactly match the
filenames used in manifest.json (image_192.png, image_512.png) so the browser
tab icon and installed PWA icons stay in sync.
|
Please remove public/sw.js and the manual registration in main.tsx. We use vite-plugin-pwa to auto-generate our Service Worker via Workbox in vite.config.ts, and your manual file breaks the ONNX model caching. Also, please revert the unnecessary typeof window === 'undefined' checks in AuthPage.tsx (this is a Vite SPA, not Next.js) and revert the package-lock.json bloat. still some issues |
jpdevhub
left a comment
jpdevhub
left a comment
There was a problem hiding this comment.
Now its perfect and fine. Thanks for your contribution.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thank you for the guidance sir |
2 participants
Description [Reference Issue: #74]
This Pull Request implements a custom installation prompt for mobile and tablet devices. The solution addresses the requirements for FreshScan AI by prompting mobile users to install the application to their home screen while ensuring that desktop users do not see the pop-up, even if their browser windows are resized or compressed.
This update also introduces a functional Service Worker to ensure Progressive Web App compliance.
Changes Made
1. Service Worker Setup and Compliance
public/sw.js: Created a service worker file that handles basic install, activate, and fetch events. It uses a network-first approach with an offline fallback. This ensures that the application structure remains accessible if a user loses internet connectivity in a market.src/main.tsxto register the service worker when the page loads.2. Device and Hardware Detection
beforeinstallpromptevent listener.src/components/InstallPrompt.tsxto ensure the banner only displays on true mobile devices:3. Custom Installation User Interface
4. Development Environment Configuration
vite.config.tsto allow network access to the local development server. This allowed testing the setup directly on a physical mobile device using local network port forwarding.Installation Dialog UI
Testing and Verification
The entire workflow was verified on a physical mobile phone using the following steps:
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style