Codex/add agent skill#1962
Conversation
…project-45f8bv
…ions-for-chatgpt-app feat(examples): make Spanish Speaking Gym installable on mobile
![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: 3f9c38bcf1
ℹ️ 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".
|
|
||
| function ensureStateShape(rawState) { | ||
| const hydrated = rawState || {}; | ||
| hydrated.phrases ||= starterPhrases; |
There was a problem hiding this comment.
When no saved state exists, including immediately after the Reset local data button removes localStorage, this assigns state.phrases to the module-level starterPhrases array. Practice actions mutate those phrase objects in place, so marking a phrase Easy/Hard and then resetting leaves the recall counts/status visible until a full page reload, and any subsequent save writes the mutated starters back. Initialize from fresh copies instead of reusing the starter objects.
Useful? React with 👍 / 👎.
|
|
||
| function card(html) { | ||
| const node = document.getElementById('card-template').content.firstElementChild.cloneNode(); | ||
| node.innerHTML = html; |
There was a problem hiding this comment.
This helper writes every caller's template directly to innerHTML, but some callers interpolate arbitrary values saved from FormData/localStorage such as weak phrases and benchmark notes. If a learner pastes text containing markup with an event handler, visiting the saved entry executes it in the app; render user-controlled fields with textContent or escape them before passing them into card().
Useful? React with 👍 / 👎.
| function todayISO() { | ||
| return new Date().toISOString().slice(0, 10); |
There was a problem hiding this comment.
For learners outside UTC near midnight, this uses the UTC calendar date for sessions, challenges, streaks, and benchmark prompts. For example, a US Pacific learner practicing on Saturday evening can be moved to Sunday's daily state and benchmark day early, which corrupts the daily/streak tracking the app is presenting.
Useful? React with 👍 / 👎.
| const done = state.sessionLog[dateKey].completedBlocks; | ||
| if (el.checked && !done.includes(block)) done.push(block); | ||
| if (!el.checked) state.sessionLog[dateKey].completedBlocks = done.filter((x) => x !== block); | ||
| awardXP(2); |
There was a problem hiding this comment.
When a learner unchecks a completed session block, or toggles an already-recorded block back and forth, this still awards +2 XP on every change. Because done is only updated conditionally above but XP is unconditional, the daily dashboard can inflate progress without additional practice; award points only when a block transitions from incomplete to complete.
Useful? React with 👍 / 👎.
| python3 -m http.server "${PORT:-4173}" --bind "${HOST:-0.0.0.0}" | ||
| ``` | ||
|
|
||
| Then open `http://<your-computer-lan-ip>:4173/` on your phone, go to **Settings**, copy/share the mobile connection URL if needed, and choose **Add to Home Screen** / **Install App** from the mobile browser. |
There was a problem hiding this comment.
In the documented mobile flow, the phone opens a plain http://<lan-ip> URL, which is not a secure context, so browsers will not expose/register the service worker used for the offline cache. The app may load, but the promised mobile offline support will not work from this URL; document an HTTPS/tunnel option or avoid promising offline install for the LAN HTTP path.
Useful? React with 👍 / 👎.
| event.waitUntil( | ||
| caches | ||
| .keys() | ||
| .then((keys) => Promise.all(keys.filter((key) => key !== CACHE_NAME).map((key) => caches.delete(key)))), |
There was a problem hiding this comment.
When this example is served from a shared origin, such as a repo-root localhost server or a site that hosts other PWAs under sibling paths, the service worker activate step enumerates all Cache Storage keys for the origin and deletes every cache whose name is not this one. That can wipe offline caches belonging to other apps outside this service worker's scope; filter by a cache-name prefix owned by this app before deleting.
Useful? React with 👍 / 👎.
1 participant
Changes being requested
Additional context & links