Add PWA manifest and iOS meta tags to index.html - #136
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces PWA support by adding a web manifest and corresponding meta tags to the application. Feedback suggests using media queries for the theme-color meta tag to support both light and dark modes and adding id, description, and lang properties to the manifest for improved identity and accessibility.
| { | ||
| "name": "成績分析平台(中大壢中)", | ||
| "short_name": "成績分析", | ||
| "start_url": "/", | ||
| "scope": "/", | ||
| "display": "standalone", | ||
| "background_color": "#0f172a", | ||
| "theme_color": "#4f46e5", | ||
| "icons": [ | ||
| { | ||
| "src": "/icon-192.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png" | ||
| }, | ||
| { | ||
| "src": "/icon-512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png" | ||
| }, | ||
| { | ||
| "src": "/icon-192-maskable.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png", | ||
| "purpose": "maskable" | ||
| }, | ||
| { | ||
| "src": "/icon-512-maskable.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png", | ||
| "purpose": "maskable" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
To improve the PWA's metadata and identity, consider adding the id, description, and lang properties. The id property provides a stable identifier for the application, while description and lang enhance the installation experience and accessibility.
{
"id": "/",
"name": "成績分析平台(中大壢中)",
"short_name": "成績分析",
"description": "中大壢中成績分析平台,提供學生查詢段考成績並進行落點分析。",
"lang": "zh-TW",
"start_url": "/",
"scope": "/",
"display": "standalone",
"background_color": "#0f172a",
"theme_color": "#4f46e5",
"icons": [
{
"src": "/icon-192.png",
"sizes": "192x192",
"type": "image/png"
},
{
"src": "/icon-512.png",
"sizes": "512x512",
"type": "image/png"
},
{
"src": "/icon-192-maskable.png",
"sizes": "192x192",
"type": "image/png",
"purpose": "maskable"
},
{
"src": "/icon-512-maskable.png",
"sizes": "512x512",
"type": "image/png",
"purpose": "maskable"
}
]
}
There was a problem hiding this comment.
Pull request overview
This PR adds Progressive Web App (PWA) metadata so the app can be installable and display proper branding on Android/iOS via a web manifest and Apple-specific meta tags.
Changes:
- Added
public/manifest.webmanifestdefining app metadata and icon entries (including maskable variants). - Updated
public/index.htmlto reference the manifest, settheme-color, and include iOS standalone/app icon meta tags.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
public/manifest.webmanifest |
Introduces the PWA manifest with app metadata and icon declarations. |
public/index.html |
Adds <link rel="manifest">, theme-color, and Apple PWA meta tags for install/standalone support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "icons": [ | ||
| { | ||
| "src": "/icon-192.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png" | ||
| }, | ||
| { | ||
| "src": "/icon-512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png" | ||
| }, | ||
| { | ||
| "src": "/icon-192-maskable.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png", | ||
| "purpose": "maskable" | ||
| }, | ||
| { | ||
| "src": "/icon-512-maskable.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png", | ||
| "purpose": "maskable" | ||
| } | ||
| ] |
There was a problem hiding this comment.
The manifest references icon files (/icon-192.png, /icon-512.png, and maskable variants), but these files are not present under public/ in the repo, so the URLs will 404 and installability/icon rendering will break. Add these icon assets to public/ (or update the src paths to match existing assets).
| "icons": [ | |
| { | |
| "src": "/icon-192.png", | |
| "sizes": "192x192", | |
| "type": "image/png" | |
| }, | |
| { | |
| "src": "/icon-512.png", | |
| "sizes": "512x512", | |
| "type": "image/png" | |
| }, | |
| { | |
| "src": "/icon-192-maskable.png", | |
| "sizes": "192x192", | |
| "type": "image/png", | |
| "purpose": "maskable" | |
| }, | |
| { | |
| "src": "/icon-512-maskable.png", | |
| "sizes": "512x512", | |
| "type": "image/png", | |
| "purpose": "maskable" | |
| } | |
| ] | |
| "icons": [] |
Updated favicon and manifest links with new versions and added apple-touch-icon.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <link rel="shortcut icon" href="/favicon.ico?v=20260402" /> | ||
| <link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon.png?v=20260402" /> | ||
| <meta name="apple-mobile-web-app-title" content="成績分析" /> | ||
| <link rel="manifest" href="/site.webmanifest?v=20260402" /> |
There was a problem hiding this comment.
/site.webmanifest will be served via the Flask /<path:filename> static route, but app/routes/system.py's ALLOWED_STATIC_EXT does not include .webmanifest, so this request will currently return 403 and the manifest won’t load. Fix by either adding .webmanifest to the allowlist (and ensuring it’s served with an appropriate manifest content-type) or renaming the manifest to an allowed extension (e.g., .json) and updating this link accordingly.
| <link rel="manifest" href="/site.webmanifest?v=20260402" /> | |
| <link rel="manifest" href="/site.json?v=20260402" /> |
| <link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon.png?v=20260402" /> | ||
| <meta name="apple-mobile-web-app-title" content="成績分析" /> | ||
| <link rel="manifest" href="/site.webmanifest?v=20260402" /> |
There was a problem hiding this comment.
The PR description/title mention adding theme-color, apple-mobile-web-app-capable, and apple-mobile-web-app-status-bar-style, but they are not present in the <head> after this change. Add these meta tags (and keep them consistent with the manifest theme_color) so Android/iOS standalone theming and install UX work as intended.
| <link rel="icon" type="image/x-icon" href="/favicon.ico"> | ||
| <link rel="icon" type="image/png" href="/favicon-96x96.png?v=20260402" sizes="96x96" /> | ||
| <link rel="icon" type="image/svg+xml" href="/favicon.svg?v=20260402" /> | ||
| <link rel="shortcut icon" href="/favicon.ico?v=20260402" /> |
There was a problem hiding this comment.
There are now two favicon ICO declarations: the existing <link rel="icon" ... href="/favicon.ico"> and the newly added <link rel="shortcut icon" href="/favicon.ico?v=...">. This is redundant and can cause inconsistent icon selection/caching; keep a single canonical ICO link (and apply the same cache-busting strategy consistently if you really need it).
| <link rel="icon" type="image/x-icon" href="/favicon.ico"> | |
| <link rel="icon" type="image/png" href="/favicon-96x96.png?v=20260402" sizes="96x96" /> | |
| <link rel="icon" type="image/svg+xml" href="/favicon.svg?v=20260402" /> | |
| <link rel="shortcut icon" href="/favicon.ico?v=20260402" /> | |
| <link rel="icon" type="image/x-icon" href="/favicon.ico?v=20260402"> | |
| <link rel="icon" type="image/png" href="/favicon-96x96.png?v=20260402" sizes="96x96" /> | |
| <link rel="icon" type="image/svg+xml" href="/favicon.svg?v=20260402" /> |
| { | ||
| "name": "成績分析平台", | ||
| "short_name": "成績分析", | ||
| "icons": [ | ||
| { | ||
| "src": "/web-app-manifest-192x192.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png", | ||
| "purpose": "maskable" | ||
| }, | ||
| { | ||
| "src": "/web-app-manifest-512x512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png", | ||
| "purpose": "maskable" | ||
| } | ||
| ], | ||
| "theme_color": "#ffffff", | ||
| "background_color": "#ffffff", | ||
| "display": "standalone" |
There was a problem hiding this comment.
This manifest is missing fields called out in the PR description (start_url, scope, and (optionally) id). Without these, install behavior and navigation scope rely on browser defaults and can be surprising (especially if the app is hosted under a subpath). Add start_url/scope aligned to the deployment path.
| "purpose": "maskable" | ||
| }, | ||
| { | ||
| "src": "/web-app-manifest-512x512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png", | ||
| "purpose": "maskable" |
There was a problem hiding this comment.
Both icons declare "purpose": "maskable" only. Some platforms expect an any icon for general usage; with maskable-only, the icon may be ignored or rendered unexpectedly. Consider using "purpose": "any maskable" (or provide separate entries for any and maskable).
| "purpose": "maskable" | |
| }, | |
| { | |
| "src": "/web-app-manifest-512x512.png", | |
| "sizes": "512x512", | |
| "type": "image/png", | |
| "purpose": "maskable" | |
| "purpose": "any maskable" | |
| }, | |
| { | |
| "src": "/web-app-manifest-512x512.png", | |
| "sizes": "512x512", | |
| "type": "image/png", | |
| "purpose": "any maskable" |
Motivation
Description
public/manifest.webmanifestfile withname,short_name,start_url,scope,display,background_color,theme_color, and icon entries including maskable variants.public/index.htmlto include a<link rel="manifest" href="/manifest.webmanifest">and ametatheme-colortag.public/index.html:apple-touch-icon,apple-mobile-web-app-capable,apple-mobile-web-app-status-bar-style, andapple-mobile-web-app-title.Testing
Codex Task