Repository navigation
Publish Dayshaper: branding, PWA, geography, and sharing - #5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
beejsbj
left a comment
beejsbj
left a comment
There was a problem hiding this comment.
Independent review by GPT-6-Sol
Reviewed SHA: 09cd19b18ae91a5814b0e8d883b7ffb87b13d2a2 (origin/master...HEAD).
Verdict: Changes requested before release for the offline geography behavior. Branding, canonical metadata, install flow, and local matching otherwise align with the requested result.
Findings:
- Offline first use loses physical geography.
sw.js:5-12precaches the shell but omitsdata/geography.json.src/main.js:101-116downloads and caches that file only during a location lookup. A user can install the app online, go offline, and then grant/use location for the first time: the app shows coordinates but no physical feature. This falls short of the requested offline local geography. The existing offline E2E attest/e2e/smoke.mjs:494-510first populates the map cache online, so it does not cover this case. - A failed lookup can leave the previous place attached to new coordinates. On a new geolocation,
src/main.js:909-914changesprefs.locbut does not clear or recomputeplacebeforerefreshGeography(). If the map fetch/cache lookup fails (src/main.js:107-116),syncMenu()atsrc/main.js:885-888can show the old feature beside the new coordinates. This matters when a user moves or changes location while offline.
Verified: the geography and weather unit tests passed (11/11); git diff --check passed. src/weather.js:86-96 uses rounded coordinates for the consented Open-Meteo request; src/main.js:105-114 requests same-origin map data without coordinates. index.html:6-37, manifest.webmanifest:2-18, and sw.js:4-15 contain the canonical SEO, branding, and install shell changes. The social image is 1200×630 and matches the portfolio copy byte for byte.
Limitations: I did not run the full browser smoke suite; the main release agent is running the full app tests. This review does not assess live deployment or DNS behavior.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6f17d1c8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
beejsbj
left a comment
beejsbj
left a comment
There was a problem hiding this comment.
Independent follow-up by GPT-6-Sol (low effort) — reviewed 40f701d, including 09cd19b..HEAD.
Verdict: previous findings resolved; no new substantive regression found in the changed paths.
• First offline location: data/geography.json is now in the service worker SHELL addAll list, so a completed install caches the map before the first location lookup. refreshGeography also falls back to caches.match if fetch fails. The new browser test waits for service worker control, goes offline before first location, and checks Toronto resolves to Near Lake Ontario.
• Location changes: locate now immediately recalculates place from geographyData (or clears it while the map has yet to load); refreshGeography reuses the loaded map and matches against current prefs.loc. The new test checks travel to Sahara and then a featureless coordinate, where the label returns to Your location.
• Installed state: .item[hidden] now overrides .item display:flex, and the browser test opens the menu before asserting that the install button is hidden.
Independent inspection covered the changed code, matching and install logic, and git diff --check (clean). The main agent is running the full unit/browser suite; I did not duplicate it.
beejsbj
left a comment
beejsbj
left a comment
There was a problem hiding this comment.
Independent final delta review by GPT-6-Sol (low effort, fresh context)
Reviewed 40f701d..dc0214e and essential location/map call sites. Clean: the shared in-flight geography promise prevents concurrent startup/geolocation requests from downloading the map twice. It clears on success, null response, or rejection, allowing a later location update to retry. After loading, each caller matches against the current prefs.loc, so an older request cannot restore a stale place. The new browser case forces weather and checks one delayed map download plus the resulting Sahara location. Package version is 2.2.0.
No material findings. The full browser run was handled by the parent reviewer; I did not duplicate it here.
Publish Dayshaper at https://dayshaper.burooj.dev with consistent naming, logo and social cards, a concise README, search metadata, and an installable offline app. Location describes nearby physical features using bundled Natural Earth data instead of latitude bands; coordinates stay on-device for that lookup.
Includes installation guidance, offline schedule/share-link support, first-use offline geography, and reproducible artwork/map generation. Preserves existing schedules and retains the former Vercel URL for transferring them by share link. The map covers major features and adds 1.9 MB before compression to the offline shell.
Validation: 55 unit tests and 34 Chromium browser checks pass; GitHub CI is green on the final head. Independent GPT-6-Sol reviews and automated review findings are addressed. GitHub's sharing image, profile pin, repository rename, and custom-domain HTTPS have been verified.