feat: implementar soporte PWA completo con experiencia offline - #307
Open
shadownrx wants to merge 1 commit into
Open
feat: implementar soporte PWA completo con experiencia offline#307shadownrx wants to merge 1 commit into
shadownrx wants to merge 1 commit into
Conversation
- Agregar manifest.ts con shortcuts para Eventos, Conversaciones, Cursos y Lectura - Crear Service Worker (sw.js) con estrategias Cache-First y Network-First - Agregar PwaProvider para registro del SW, alertas de conexion y actualizaciones - Crear pagina /offline con juego interactivo de trivia para desarrolladores - Agregar widget de instalacion en la sidebar con soporte colapsado/expandido - Generar iconos PWA estandar y maskable (192x512) a partir del logo existente - Agregar script Python para regenerar iconos PWA facilmente
Contributor
WalkthroughAdds full PWA support: a Next.js web app manifest with icons and shortcuts, a Python icon-generation script, a standalone service worker ( ChangesPWA Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant PwaProvider
participant ServiceWorker
participant Cache
participant Toast
rect rgba(59, 130, 246, 0.5)
note over PwaProvider,ServiceWorker: Registration & Update
PwaProvider->>ServiceWorker: register(/sw.js)
ServiceWorker->>Cache: precache assets on install
ServiceWorker-->>PwaProvider: updatefound → statechange installed
PwaProvider->>Toast: show "Actualizar" button
User->>Toast: click Actualizar
Toast->>ServiceWorker: postMessage SKIP_WAITING
ServiceWorker-->>PwaProvider: controllerchange
PwaProvider->>User: location.reload()
end
rect rgba(16, 185, 129, 0.5)
note over User,Cache: Offline Navigation Fallback
User->>ServiceWorker: fetch navigate request
ServiceWorker->>ServiceWorker: network fails
ServiceWorker->>Cache: match cached page
Cache-->>ServiceWorker: miss
ServiceWorker->>User: serve /offline page
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/generate-pwa-icons.py (1)
5-7: ⚡ Quick winResolve the icon paths from the script location.
Right now the script only works when launched from the repo root. Running it from
scripts/or CI with a different cwd will misspublic/logo.webpand write the PNGs to the wrong place.♻️ Proposed fix
-import os +from pathlib import Path from PIL import Image def generate_icons(): - logo_path = 'public/logo.webp' - if not os.path.exists(logo_path): + repo_root = Path(__file__).resolve().parents[1] + logo_path = repo_root / 'public' / 'logo.webp' + if not logo_path.exists(): print(f"Error: No se encontró el logo en {logo_path}") return logo = Image.open(logo_path) @@ - standard_out = f"public/pwa-icon-{size}.png" + standard_out = repo_root / 'public' / f'pwa-icon-{size}.png' standard_img.save(standard_out, "PNG") @@ - maskable_out = f"public/pwa-icon-{size}-maskable.png" + maskable_out = repo_root / 'public' / f'pwa-icon-{size}-maskable.png' maskable_rgb.save(maskable_out, "PNG")Also applies to: 35-37, 56-57
🤖 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 `@scripts/generate-pwa-icons.py` around lines 5 - 7, The script uses hardcoded relative paths like 'public/logo.webp' that only work when executed from the repository root, causing failures when run from different directories or in CI environments. Fix this by resolving all file paths relative to the script's own location using os.path.dirname(__file__) to get the script directory, then construct the paths to logo_path and output directories relative to that location. Apply this fix to all hardcoded paths including the logo_path assignment and the output directory paths at lines 35-37 and 56-57 as well.
🤖 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 `@public/sw.js`:
- Around line 4-13: The ASSETS_TO_CACHE array currently only precaches the
offline page document (OFFLINE_URL) but does not include its required JavaScript
bundles from the _next/static directory. Since the offline route is a client
component with interactivity, it needs its hashed build chunks to function
properly when accessed offline. Add the specific _next/static chunk files
required by the offline route to the ASSETS_TO_CACHE array to ensure they are
precached along with the offline page itself. This applies to both the
ASSETS_TO_CACHE array definition and any other cache precaching logic in the
service worker (around lines 63-79).
In `@src/app/offline/page.tsx`:
- Around line 356-357: The onClick handler at line 356 only resets triviaStarted
to false, but leaves currentIdx and score with their previous completed values.
This causes the game to skip directly to results when "Comenzar juego" is
clicked again. Modify the onClick handler to reset all three state variables:
setTriviaStarted(false), setCurrentIdx to its initial value, and setScore to its
initial value, so the game state is fully reset when returning to the welcome
screen.
In `@src/components/pwa-provider.tsx`:
- Around line 107-119: The cleanup function (the return statement) removes event
listeners for 'online', 'offline', and 'beforeinstallprompt' from the window
object, but fails to remove the 'controllerchange' listener that was registered
on the navigator.serviceWorker object. Add a call to
navigator.serviceWorker.removeEventListener in the cleanup function to remove
the 'controllerchange' listener with the same callback handler that was used to
add it, ensuring that the listener is properly cleaned up on component unmount
and preventing duplicate handlers from accumulating on remounts.
---
Nitpick comments:
In `@scripts/generate-pwa-icons.py`:
- Around line 5-7: The script uses hardcoded relative paths like
'public/logo.webp' that only work when executed from the repository root,
causing failures when run from different directories or in CI environments. Fix
this by resolving all file paths relative to the script's own location using
os.path.dirname(__file__) to get the script directory, then construct the paths
to logo_path and output directories relative to that location. Apply this fix to
all hardcoded paths including the logo_path assignment and the output directory
paths at lines 35-37 and 56-57 as well.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ca28a5df-7c09-45e8-82d5-b5a2cd3b83e7
⛔ Files ignored due to path filters (4)
public/pwa-icon-192-maskable.pngis excluded by!**/*.pngpublic/pwa-icon-192.pngis excluded by!**/*.pngpublic/pwa-icon-512-maskable.pngis excluded by!**/*.pngpublic/pwa-icon-512.pngis excluded by!**/*.png
📒 Files selected for processing (7)
public/sw.jsscripts/generate-pwa-icons.pysrc/app/layout.tsxsrc/app/manifest.tssrc/app/offline/page.tsxsrc/components/pwa-provider.tsxsrc/components/ui/app-sidebar.tsx
Comment on lines
+4
to
+13
| const ASSETS_TO_CACHE = [ | ||
| OFFLINE_URL, | ||
| '/', | ||
| '/favicon.ico', | ||
| '/logo.webp', | ||
| '/pwa-icon-192.png', | ||
| '/pwa-icon-512.png', | ||
| '/pwa-icon-192-maskable.png', | ||
| '/pwa-icon-512-maskable.png', | ||
| ]; |
Contributor
There was a problem hiding this comment.
Precache the offline page’s JS bundles too.
The SW only caches the /offline document here. Because the offline route is a client component with trivia interactivity, a cold offline visit still needs its hashed _next/static chunks; those are not guaranteed to be cached yet, so the fallback can degrade into a static shell.
Either make /offline self-contained/server-rendered, or seed the route’s build assets into the precache set as well. If you want, I can draft the precache wiring.
Also applies to: 63-79
🤖 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 4 - 13, The ASSETS_TO_CACHE array currently only
precaches the offline page document (OFFLINE_URL) but does not include its
required JavaScript bundles from the _next/static directory. Since the offline
route is a client component with interactivity, it needs its hashed build chunks
to function properly when accessed offline. Add the specific _next/static chunk
files required by the offline route to the ASSETS_TO_CACHE array to ensure they
are precached along with the offline page itself. This applies to both the
ASSETS_TO_CACHE array definition and any other cache precaching logic in the
service worker (around lines 63-79).
Comment on lines
+356
to
+357
| onClick={() => setTriviaStarted(false)} | ||
| className="bg-zinc-800 hover:bg-zinc-700 text-zinc-300 border border-zinc-700 h-10 px-6 transition-all active:scale-95" |
Contributor
There was a problem hiding this comment.
Reset quiz state when returning to the welcome screen.
Line 356 only toggles triviaStarted. currentIdx/score remain at completed values, so pressing “Comenzar juego” can jump directly back to results instead of starting over.
Suggested fix
@@
- <Button
- onClick={() => setTriviaStarted(false)}
+ <Button
+ onClick={() => {
+ setCurrentIdx(0);
+ setScore(0);
+ setSelectedOpt(null);
+ setAnswered(false);
+ setTriviaStarted(false);
+ }}
className="bg-zinc-800 hover:bg-zinc-700 text-zinc-300 border border-zinc-700 h-10 px-6 transition-all active:scale-95"
>
Volver
</Button>📝 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.
Suggested change
| onClick={() => setTriviaStarted(false)} | |
| className="bg-zinc-800 hover:bg-zinc-700 text-zinc-300 border border-zinc-700 h-10 px-6 transition-all active:scale-95" | |
| <Button | |
| onClick={() => { | |
| setCurrentIdx(0); | |
| setScore(0); | |
| setSelectedOpt(null); | |
| setAnswered(false); | |
| setTriviaStarted(false); | |
| }} | |
| className="bg-zinc-800 hover:bg-zinc-700 text-zinc-300 border border-zinc-700 h-10 px-6 transition-all active:scale-95" | |
| > | |
| Volver | |
| </Button> |
🤖 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/app/offline/page.tsx` around lines 356 - 357, The onClick handler at line
356 only resets triviaStarted to false, but leaves currentIdx and score with
their previous completed values. This causes the game to skip directly to
results when "Comenzar juego" is clicked again. Modify the onClick handler to
reset all three state variables: setTriviaStarted(false), setCurrentIdx to its
initial value, and setScore to its initial value, so the game state is fully
reset when returning to the welcome screen.
Comment on lines
+107
to
+119
| navigator.serviceWorker.addEventListener('controllerchange', () => { | ||
| if (!refreshing) { | ||
| refreshing = true; | ||
| window.location.reload(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return () => { | ||
| window.removeEventListener('online', handleOnline); | ||
| window.removeEventListener('offline', handleOffline); | ||
| window.removeEventListener('beforeinstallprompt', handleBeforeInstallPrompt); | ||
| }; |
Contributor
There was a problem hiding this comment.
Remove the controllerchange listener during cleanup.
Line 107 registers a navigator.serviceWorker listener, but cleanup at Lines 115-119 only removes window listeners. On remounts, this can accumulate handlers and cause duplicate reload behavior on SW takeover.
Suggested fix
@@
- if (shouldRegisterSw) {
+ let handleControllerChange: (() => void) | null = null;
+ if (shouldRegisterSw) {
@@
- let refreshing = false;
- navigator.serviceWorker.addEventListener('controllerchange', () => {
+ let refreshing = false;
+ handleControllerChange = () => {
if (!refreshing) {
refreshing = true;
window.location.reload();
}
- });
+ };
+ navigator.serviceWorker.addEventListener('controllerchange', handleControllerChange);
}
@@
return () => {
window.removeEventListener('online', handleOnline);
window.removeEventListener('offline', handleOffline);
window.removeEventListener('beforeinstallprompt', handleBeforeInstallPrompt);
+ if (handleControllerChange) {
+ navigator.serviceWorker.removeEventListener('controllerchange', handleControllerChange);
+ }
};🤖 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/components/pwa-provider.tsx` around lines 107 - 119, The cleanup function
(the return statement) removes event listeners for 'online', 'offline', and
'beforeinstallprompt' from the window object, but fails to remove the
'controllerchange' listener that was registered on the navigator.serviceWorker
object. Add a call to navigator.serviceWorker.removeEventListener in the cleanup
function to remove the 'controllerchange' listener with the same callback
handler that was used to add it, ensuring that the listener is properly cleaned
up on component unmount and preventing duplicate handlers from accumulating on
remounts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementación PWA y Soporte Offline
Configurar
manifest.tsCrear Service Worker (
sw.js)Agregar
PwaProviderCrear página
/offlineAgregar widget de instalación en la sidebar
Generar íconos PWA
Summary
Screenshots
Summary by CodeRabbit