Add Universe PWA branding#172
Conversation
dosubot
Bot
added
the
size:M
📝 WalkthroughWalkthroughThis PR establishes PWA branding for the BAWES Universe play app by creating a web app manifest with Universe identity metadata and updating the HTML entry point with mobile meta tags and manifest link configuration. ChangesPWA Manifest and Mobile Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| { | ||
| "src": "/static/universe-icon-512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png", | ||
| "purpose": "maskable any" | ||
| } |
There was a problem hiding this comment.
Bug: The PWA icon combines maskable and any purposes, causing it to appear shrunken in non-maskable contexts like home screen shortcuts.
Severity: LOW
Suggested Fix
Separate the icon definitions in manifest.webmanifest. Provide one icon entry with "purpose": "maskable" and another identical icon entry with "purpose": "any". This ensures the appropriate icon is used in each context, displaying correctly on home screens and in other UI surfaces.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: play/public/manifest.webmanifest#L17-L22
Potential issue: The 512px PWA icon in `manifest.webmanifest` specifies `"purpose":
"maskable any"`. This is an anti-pattern because browsers will use this single, padded
icon for both maskable and non-maskable (`any`) contexts. As a result, when a user adds
the PWA to their home screen, the icon will appear shrunken due to the unnecessary
safe-zone padding intended for masking, leading to inconsistent and poor branding
presentation.
Did we get this right? 👍 / 👎 to inform future reviews.
![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
🤖 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 `@play/index.html`:
- Line 23: The two meta tags are inconsistent: <meta
name="msapplication-TileColor" content="{{ themeColor }}" /> and <meta
name="theme-color" content="`#000000`" />; pick one approach and make them match.
Either hardcode both values to "`#000000`" (change msapplication-TileColor to
"`#000000`") or make both use the template variable (change theme-color to "{{
themeColor }}"); update the tags accordingly so msapplication-TileColor and
theme-color are consistent.
- Line 49: The index now points to a static /manifest.webmanifest which breaks
per-deployment dynamic manifests; either revert play/index.html to reference the
dynamic endpoint (/static/images/favicons/manifest.json) or make the dynamic
generator serve at /manifest.webmanifest instead; locate play/index.html and
change the <link rel="manifest"> href accordingly, or update FrontController.ts
so the existing displayManifestJson(...) handler (the function
displayManifestJson and its route in FrontController) is mounted at
/manifest.webmanifest and returns the same dynamic content and headers as the
old path to restore backward compatibility.
🪄 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
Run ID: 84aefb0f-129e-48e4-a9f1-d43700eaec56
⛔ Files ignored due to path filters (2)
play/public/static/universe-icon-192.pngis excluded by!**/*.pngplay/public/static/universe-icon-512.pngis excluded by!**/*.png
📒 Files selected for processing (2)
play/index.htmlplay/public/manifest.webmanifest
| @@ -22,7 +22,9 @@ | |||
| <!-- App Design --> | |||
| <meta name="msapplication-TileColor" content="{{ themeColor }}" /> | |||
There was a problem hiding this comment.
Inconsistent theme-color handling: hardcoded vs template variable.
Line 23 uses the template variable {{ themeColor }} for msapplication-TileColor, but line 25 hardcodes #000000 for theme-color. Both serve similar purposes (Windows tile color vs browser theme color) and should be consistent. All other dynamic meta tags (title, description, url, cardImage, etc.) still use template variables.
If theme-color must be hardcoded to #000000 for Universe branding, then msapplication-TileColor should also be hardcoded to match. Alternatively, both should use {{ themeColor }} to allow per-deployment customization.
🔧 Proposed fixes
Option A: Hardcode both for consistency (if Universe branding requires fixed black theme)
- <meta name="msapplication-TileColor" content="{{ themeColor }}" />
+ <meta name="msapplication-TileColor" content="`#000000`" />
<meta name="theme-color" content="`#000000`" />Option B: Use template variable for both (if per-deployment customization is needed)
<meta name="msapplication-TileColor" content="{{ themeColor }}" />
- <meta name="theme-color" content="`#000000`" />
+ <meta name="theme-color" content="{{ themeColor }}" />Also applies to: 25-25
🤖 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 `@play/index.html` at line 23, The two meta tags are inconsistent: <meta
name="msapplication-TileColor" content="{{ themeColor }}" /> and <meta
name="theme-color" content="`#000000`" />; pick one approach and make them match.
Either hardcode both values to "`#000000`" (change msapplication-TileColor to
"`#000000`") or make both use the template variable (change theme-color to "{{
themeColor }}"); update the tags accordingly so msapplication-TileColor and
theme-color are consistent.
| <link rel="{{ rel }}" type="image/png" sizes="{{ sizes }}" href="{{ src }}" /> | ||
| {{/favIcons}} | ||
| <link rel="manifest" href="/static/images/favicons/manifest.json?url={{ url }}" /> | ||
| <link rel="manifest" href="/manifest.webmanifest" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for references to the old manifest endpoint that may need updating
echo "=== Checking for hardcoded references to old manifest endpoint ==="
rg -n 'static/images/favicons/manifest.json' --type html --type ts --type js
echo -e "\n=== Checking if FrontController manifest handler can be safely removed ==="
rg -n 'displayManifestJson|static/images/favicons/manifest.json' play/src/pusher/controllers/FrontController.ts
echo -e "\n=== Verifying new manifest is being served correctly ==="
rg -n 'manifest.webmanifest' play/Repository: BAWES-Universe/workadventure-universe
Length of output: 767
Breaking change: play/index.html now loads a static manifest, bypassing dynamic per-deployment metadata
play/index.html(line 49) now links to/manifest.webmanifest, so browsers will load the static file instead of the dynamically generated manifest.- The old dynamic endpoint
/static/images/favicons/manifest.jsonis still implemented inplay/src/pusher/controllers/FrontController.tsand callsdisplayManifestJson(..., query.url), but it’s no longer used via the default manifest link. - Deployments that previously depended on admin/map-based manifest customization will lose that behavior in the normal browser flow; either migrate/replace the dynamic behavior for the manifest link or provide a backward-compatible path for existing deployments.
🤖 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 `@play/index.html` at line 49, The index now points to a static
/manifest.webmanifest which breaks per-deployment dynamic manifests; either
revert play/index.html to reference the dynamic endpoint
(/static/images/favicons/manifest.json) or make the dynamic generator serve at
/manifest.webmanifest instead; locate play/index.html and change the <link
rel="manifest"> href accordingly, or update FrontController.ts so the existing
displayManifestJson(...) handler (the function displayManifestJson and its route
in FrontController) is mounted at /manifest.webmanifest and returns the same
dynamic content and headers as the old path to restore backward compatibility.
1 participant
Summary
/manifest.webmanifestwith BAWES Universe app identityplay/public/static/Fixes #3
/claim #1
Verification
python3 -m json.tool play/public/manifest.webmanifestgit diff --checkSummary by CodeRabbit
New Features
Chores