Feat/shortcut keys to configure browser split screen#1413
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…browser-splitScreen
|
Contributor trust score14/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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0a6e3d0b4
ℹ️ 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".
| source: "extension-user-action", | ||
| windowId: browser.windows.WINDOW_ID_CURRENT, | ||
| })) |
There was a problem hiding this comment.
Resolve a real window ID before toggling from popup
Passing browser.windows.WINDOW_ID_CURRENT here sends -2, but the background toggle logic stores open state in createSidePanelWindowState keyed by the provided windowId. Because -2 is reused for every window, state becomes cross-window and the first toggle in another window can be misclassified as already open (leading to a close/no-op/unsupported path instead of opening). Use the concrete current window id (or normalize -2 in background) before state tracking and side panel calls.
Useful? React with 👍 / 👎.
| () => { | ||
| void sendMessage("toggleSidePanel", undefined).catch((error) => { | ||
| logger.error("Failed to toggle split translator from shortcut", error) |
There was a problem hiding this comment.
Handle unsuccessful toggle responses in shortcut callback
This callback only logs rejected promises, but toggleSidePanel can resolve with { ok: false, reason: ... }. In Firefox, background returns requires-extension-user-action for content-script sourced requests, so the split-translator hotkey becomes a silent no-op with no user feedback or fallback. Check the resolved result and handle ok: false explicitly (or send an allowed source for this user gesture path).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a136529e4
ℹ️ 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 HAN_SCRIPT_RE = /[\u3400-\u9FFF\uF900-\uFAFF]/u | ||
|
|
||
| function detectSplitTranslatorShortTextLanguage(text: string): LangCodeISO6393 | null { | ||
| const trimmedText = text.trim() | ||
| return trimmedText.length < MIN_LENGTH_FOR_SKIP_LLM_DETECTION && HAN_SCRIPT_RE.test(trimmedText) ? "cmn" : null |
There was a problem hiding this comment.
Restrict short-text Chinese override to Chinese-only inputs
The new short-text fast path marks any string containing a Han ideograph as cmn, which also matches Japanese kanji (and hanja), so short non-Chinese inputs like 日本語 are forced to Chinese and skip normal detection. In split-translator flows where source is auto, this can trigger incorrect same-language warnings and produce lower-quality or incorrect translations for short Japanese/Korean text because the detection signal is now biased before fallback detection runs.
Useful? React with 👍 / 👎.
…browser-splitScreen
|
This PR has been inactive for 14 days and is now marked as stale. |
Type of Changes
Description
This PR adds a split translator workflow that can be opened from the extension UI and used directly in the side panel.
It includes:
Overall, these changes make the split translator easier to access, easier to configure, and more reliable during real usage.
Related Issue
Closes #1196
How Has This Been Tested?
Screenshots
Suggested screenshots to attach if needed:
Checklist
Additional Information
Most of the changes in this PR are part of the same split translator delivery thread, so the branch includes feature work, follow-up fixes, i18n updates, and test coverage improvements around the same user flow.