Add Universe PWA branding#6
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR configures Progressive Web App (PWA) support for the BAWES Universe application by updating the HTML head with PWA meta tags (hardcoded theme color, Apple mobile app support) and manifest link, and adding a new web app manifest file that defines app identity, display modes, colors, and icon assets. ChangesPWA Configuration Setup
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
play/public/manifest.webmanifest (1)
5-6: ⚡ Quick winAdd an explicit manifest
idfor stable web app identity.Browsers historically use
start_urlas the unique identifier for installed PWAs, meaning any changes tostart_urlrisk creating duplicate installations. Adding an explicitiddecouples the app's identity from volatile fields, ensuring the browser recognizes updates as updates rather than new installations.Suggested change
"start_url": "/", + "id": "/", "scope": "/",🤖 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/public/manifest.webmanifest` around lines 5 - 6, Add an explicit "id" field to the web manifest to decouple app identity from "start_url" and prevent duplicate installations; update the manifest (around the existing "start_url" and "scope" entries) to include a stable, unique id string (e.g., a canonical URL or app identifier) as the "id" field so browsers use that as the PWA identity.
🤖 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 25: The meta tag providing theme color is hardcoded to "#000000" which
can diverge from the template variable used elsewhere; update the theme-color
meta tag to use the same template variable ({{ themeColor }}) so the browser UI
color is sourced consistently from the theme variable used elsewhere in this
file (replace the hardcoded meta name="theme-color" content value with the {{
themeColor }} template variable).
- Line 49: The manifest link in play/index.html currently points to a fixed
"/manifest.webmanifest" which can be cached; update the href on the <link
rel="manifest"> element to include a cache-busting fingerprint (e.g.,
"/manifest.webmanifest?v={BUILD_HASH}" or a timestamp) so each deploy changes
the URL; ensure the build system or server injects the {BUILD_HASH} (or
timestamp) value when rendering the page and update any asset pipeline that
references manifest.webmanifest accordingly so installed PWAs pick up new
metadata.
---
Nitpick comments:
In `@play/public/manifest.webmanifest`:
- Around line 5-6: Add an explicit "id" field to the web manifest to decouple
app identity from "start_url" and prevent duplicate installations; update the
manifest (around the existing "start_url" and "scope" entries) to include a
stable, unique id string (e.g., a canonical URL or app identifier) as the "id"
field so browsers use that as the PWA identity.
🪄 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: 423dc3dd-02ca-42f7-9493-52ad21b9f5ab
⛔ 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
| <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.
Add cache-busting to manifest URL to avoid stale installed-app metadata.
Using a fixed /manifest.webmanifest path can leave users on cached manifest/icon data after releases (especially on mobile PWAs). Keep a versioned query/fingerprint on this URL.
Suggested change
- <link rel="manifest" href="/manifest.webmanifest" />
+ <link rel="manifest" href="/manifest.webmanifest?v={{ version }}" />📝 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="manifest" href="/manifest.webmanifest" /> | |
| <link rel="manifest" href="/manifest.webmanifest?v={{ version }}" /> |
🤖 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 manifest link in play/index.html currently
points to a fixed "/manifest.webmanifest" which can be cached; update the href
on the <link rel="manifest"> element to include a cache-busting fingerprint
(e.g., "/manifest.webmanifest?v={BUILD_HASH}" or a timestamp) so each deploy
changes the URL; ensure the build system or server injects the {BUILD_HASH} (or
timestamp) value when rendering the page and update any asset pipeline that
references manifest.webmanifest accordingly so installed PWAs pick up new
metadata.
souf92i
force-pushed
the
feat/mobile-pwa-branding
branch
from
8f7d210 to
c35ba7e
Compare
|
Follow-up update: I pushed Changes made:
Re-verified locally:
CodeRabbit is currently rate-limited for a re-review, but the PR remains mergeable and scoped to #3 only. |
1 participant
Summary
play/public/manifest.webmanifestbranded for BAWES Universe with fullscreen display, landscape orientation, and 192/512 icon entriesplay/public/static/universe-icon-192.pngandplay/public/static/universe-icon-512.png/claim #1
Verification
python3 -m json.tool play/public/manifest.webmanifestsips -g pixelWidth -g pixelHeight play/public/static/universe-icon-192.png play/public/static/universe-icon-512.pngfile play/public/static/universe-icon-192.png play/public/static/universe-icon-512.pnggit diff --checkNotes
This stays within the #3 path ownership:
play/public/and the existingplay/index.htmlentry metadata only. It does not touchmobile/,android/, orios/.Summary by CodeRabbit