feat: add Phrase In-Context Editor to staging builds#113
Merged
Conversation
Closed
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesThe project adds staging-specific build commands and a Vite staging script. The frontend loads Phrase ICE only for staging builds with cookie or URL opt-in, integrates post-translation decoration and DOM rescanning, and adds tests for initialization and translation behavior. Staging Phrase ICE integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Frontend
participant loadPhraseIce
participant PhraseScript
participant VueI18n
Frontend->>loadPhraseIce: Read staging mode and phraseapp opt-in
loadPhraseIce->>PhraseScript: Inject ICE script
PhraseScript-->>loadPhraseIce: Trigger script onload
loadPhraseIce->>VueI18n: Decorate translated strings
loadPhraseIce->>Frontend: Dispatch DOM rescan events
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🔍 Hex review — Phrase ICE requirements check✅ Looks good — all four requirements met; no blocking issues. One thing worth confirming (Phrase datacenter).
Notes (non-blocking):
Last reviewed: 2026-07-15 11:29 UTC Generated by hex |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/i18n/phrase.ts`:
- Around line 120-134: Update the MutationObserver callback in phrase.ts to
trigger for every childList mutation, including removals, while retaining the
existing characterData behavior. Remove the addedNodes-only condition so DOM
removals also invoke trigger() and rescan stale edit-bubble coordinates.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 91299c9a-0b88-4894-b455-d930542b92c8
📒 Files selected for processing (6)
Makefilefrontend/PHRASE.mdfrontend/package.jsonfrontend/src/i18n/__tests__/phrase.test.tsfrontend/src/i18n/index.tsfrontend/src/i18n/phrase.ts
Embed the Phrase In-Context Editor so translators can edit strings directly on the staging deployment. Since the frontend is compiled into a single binary, ICE is included at build time: make build-staging (vite build --mode staging) compiles it in, while production builds eliminate it entirely. Even on staging, ICE stays dormant until a translator opts in via ?phraseapp=enabled (week-long cookie). Translation keys are decorated ([[__ __]] markers via a vue-i18n postTranslation hook) only after the CDN script has initialised, since markers rendered earlier never get edit bubbles, and a synthetic scroll is dispatched on DOM changes because ICE only re-scans the page on scroll.
deividasma
force-pushed
the
feat/phrase-ice
branch
from
d1c24fb to
9713844
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/i18n/__tests__/phrase.test.ts (1)
67-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
?phraseapp=disabled.The suite covers cookie absence and
?phraseapp=enabled, but not the required disable override. Start with an enabled cookie, load a URL containing?phraseapp=disabled, and assert that no ICE configuration or script is installed.🤖 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 `@frontend/src/i18n/__tests__/phrase.test.ts` around lines 67 - 121, Add a test in the loadPhraseIce suite that starts with documentStub.cookie set to phraseapp=enabled, sets locationStub.href with ?phraseapp=disabled, and invokes loadPhraseIce. Assert that window.PHRASEAPP_CONFIG remains undefined and appendedScripts remains empty, covering the disable override behavior.
🤖 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 `@frontend/src/i18n/__tests__/phrase.test.ts`:
- Around line 142-147: Update the non-string test around phrasePostTranslation
to initialize the ICE state by enabling ICE and advancing readiness before
invoking the handler. Ensure the test exercises the typeof translated guard
while retaining the assertion that the original non-string message is returned
unchanged.
---
Nitpick comments:
In `@frontend/src/i18n/__tests__/phrase.test.ts`:
- Around line 67-121: Add a test in the loadPhraseIce suite that starts with
documentStub.cookie set to phraseapp=enabled, sets locationStub.href with
?phraseapp=disabled, and invokes loadPhraseIce. Assert that
window.PHRASEAPP_CONFIG remains undefined and appendedScripts remains empty,
covering the disable override behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a43420e-4786-4e51-ae50-aa13170f168e
📒 Files selected for processing (5)
Makefilefrontend/package.jsonfrontend/src/i18n/__tests__/phrase.test.tsfrontend/src/i18n/index.tsfrontend/src/i18n/phrase.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/package.json
- Makefile
- frontend/src/i18n/phrase.ts
Removals shift layout just like additions, but ICE only cleans up the removed node's own overlay — the remaining edit bubbles kept stale coordinates until the next scroll. Also strengthen the tests: exercise the non-string pass-through with ICE ready (previously it short- circuited on the readiness flag) and cover ?phraseapp=disabled.
laurynasgadl
approved these changes
Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Embeds the Phrase In-Context Editor (ICE) so translators can edit strings in place on the staging deployment.
The frontend compiles into a single binary, so ICE is included or excluded at build time:
make buildmake build-staging(new)Even in the staging build, ICE stays dormant until opted in per browser via
?phraseapp=enabled(week-long cookie;?phraseapp=disabledclears it). Translation keys are decorated via a vue-i18npostTranslationhook only after the ICE script has initialised (markers rendered earlier never get edit bubbles), and a synthetic scroll is dispatched on DOM changes so ICE repositions its edit overlays.Verified: the production bundle contains zero references to Phrase; the staging bundle includes the integration; unit tests cover the module.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests