fix(styles): apply appearance changes to live translations - #1981
fix(styles): apply appearance changes to live translations#1981JohnathanT55 wants to merge 4 commits into
Conversation
π¦ Changeset detectedLatest commit: 24ac93e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
contrib-trust:new
Contributor trust score10/100 β New contributor This score estimates contributor familiarity with Outcome
Score breakdown
Signals used
Policy
Updated automatically when the PR changes or when a maintainer reruns the workflow. |
![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: 31537c01ff
βΉοΈ 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".
| const translatedNodes = deepQueryAllSelector(document, isTranslatedContentNode) | ||
| await Promise.all( | ||
| translatedNodes.map((node) => | ||
| decorateTranslationNode(node, newConfig.translate.translationNodeStyle), |
There was a problem hiding this comment.
When a style change occurs while provider requests are still in flight, this one-time scan only updates translation nodes that already exist. The in-flight paths retain the old Config across the request and later call insertTranslatedNodeIntoWrapper with config.translate.translationNodeStyle, so nodes appended after this scan keep the previous appearance indefinitely. Ensure late insertions read the current style or otherwise re-decorate nodes created after the configuration event.
Useful? React with πΒ / π.
There was a problem hiding this comment.
Pull request overview
Updates the host content script so translation appearance settings (preset/custom CSS) are applied to translations already present in the DOM, without re-requesting translation results.
Changes:
- Added a style-change handler that finds existing translated nodes (including in open shadow roots) and re-decorates them with the latest
translationNodeStyle. - Updated the runtime bootstrap to watch persisted config changes and trigger either a restart (mode change) or a re-decoration (style change).
- Added unit tests for the new style-change behavior and included a patch changeset for
@read-frog/extension.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/entrypoints/host.content/translation-control/handle-config-change.ts | Adds handleTranslationStyleChange and changes handleTranslationModeChange to return whether it restarted translation. |
| src/entrypoints/host.content/translation-control/tests/handle-config-change.test.ts | Adds jsdom-based tests validating re-decoration across document + open shadow roots. |
| src/entrypoints/host.content/runtime.ts | Watches config storage updates and triggers either restart or re-decoration accordingly. |
| .changeset/tidy-frogs-style.md | Patch changeset to release the live appearance-update fix. |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| !newConfig || | ||
| !oldConfig || | ||
| dequal(newConfig.translate.translationNodeStyle, oldConfig.translate.translationNodeStyle) | ||
| ) { | ||
| return | ||
| } |
| const didRestart = handleTranslationModeChange(newConfig, oldConfig, manager) | ||
| if (!didRestart) { | ||
| void handleTranslationStyleChange(newConfig, oldConfig) | ||
| } |
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5342c5435
βΉοΈ 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".
| const cleanupTranslationModeShortcut = await bindTranslationModeShortcutKey() | ||
|
|
||
| let currentConfig = initialConfig | ||
| const cleanupConfigListener = storageAdapter.watch<Config>(CONFIG_STORAGE_KEY, (newConfig) => { |
There was a problem hiding this comment.
When the appearance is changed while this tab is frozen or inactive, the storage event can be missed, so this callback never runs and currentConfig remains stale after the tab becomes visible. The repository already accounts for this behavior by reloading config on visibilitychange in configAtom and by reading config on demand in node-translation.ts; without equivalent reconciliation here, existing translations retain their old appearance until another observed config write or a retranslation.
Useful? React with πΒ / π.
| await Promise.all( | ||
| translatedNodes.map((node) => | ||
| decorateTranslationNode(node, newConfig.translate.translationNodeStyle), |
There was a problem hiding this comment.
When custom CSS is active and a root contains multiple translated nodes, this Promise.all makes every decoration call ensureCustomCSS concurrently; on the constructable-stylesheet path those calls invoke replace() on the same cached CSSStyleSheet before its CSS cache is updated. Browsers reject a second replace() while that sheet is already being modified, producing an unhandled rejection on every such refresh, and a rapid subsequent custom-CSS save can consequently leave the earlier CSS installed. Update each root's stylesheet once or serialize replacements rather than starting one replacement per node.
Useful? React with πΒ / π.
mengxi-ream
left a comment
mengxi-ream
left a comment
There was a problem hiding this comment.
Hi,
We are reluctant to add the listener for this, since users can refresh the page to get the new translation style.
Any specific reason you think we should merge this PR?
Hi, thanks for the feedback. The main use case is adjusting translation appearance while actively reading. The preview in the options page cannot fully reproduce a website's own typography and CSS, so users may need to verify colors, font size, and other appearance settings on the actual translated page. Refreshing rebuilds both the page state and the translated DOM, which can be disruptive on long or interactive pages. Applying the new style directly to existing translation nodes provides immediate feedback without restarting the page translation flow. This also seems consistent with the recent same-origin navigation change in #1982, which preserves the live translation session instead of tearing it down and rebuilding it, avoiding translated-content flicker. I understand the concern about adding a broad config listener to every host content script. I can narrow the implementation so that the listener is registered only while page translation is active, disposed of as soon as translation stops, and reacts only when Would that narrower lifecycle-scoped implementation address your concern? |
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24ac93efb9
βΉοΈ 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".
| this.cleanupTranslationStyleListener = registerTranslationStyleListener( | ||
| config.translate.translationNodeStyle, | ||
| ) |
There was a problem hiding this comment.
When appearance settings are saved after start() reads its initial config but before the awaited setAndNotifyPageTranslationStateChangedByManager call returns, no watcher is registered to receive that storage event. This registration then seeds the listener with the stale style and performs no immediate config reread, so a page that remains visible keeps using the old appearance until another style write or visibility transition. Reconcile storage immediately after installing the watcher to close this startup window.
Useful? React with πΒ / π.
| await decorateTranslationNode( | ||
| translatedNode, | ||
| resolveLiveTranslationNodeStyle(translationNodeStyle), | ||
| ) |
There was a problem hiding this comment.
When a provider response is being inserted as a custom stylesheet change arrives, this decoration can call ensureCustomCSS concurrently with the refresh in handleTranslationStyleChange. Although the scan is now sequential, this newly changed insertion path remains outside that queue; on the constructable-stylesheet path, overlapping CSSStyleSheet.replace() calls reject, so the refresh can be logged as failed and leave the previous custom CSS active until another update or insertion. Serialize replacements per root in the style injector rather than only serializing scan nodes.
Useful? React with πΒ / π.
|
Hi, thank you for the update! Could you give me an example website you think it really need instant change of the translation style css? |
Type of Changes
Description
Apply translation appearance changes to translations already present on an actively translated page.
The options preview cannot reproduce every website's typography and CSS. Refreshing the host page to verify an appearance change rebuilds the page state and translated DOM, which can be disruptive on long or interactive pages.
This implementation is lifecycle-scoped:
translationNodeStylechanges;Related Issue
N/A
How Has This Been Tested?
Validation:
Screenshots
N/A β this fixes live application of existing appearance settings rather than adding a new visual preset.
Checklist
Additional Information
@read-frog/extension.main, including fix(translate): stop page-translation flicker on same-origin navigationΒ #1982.