Skip to content

fix(web): preserve preview focus on window return - #11444

Merged
maria-rcks merged 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/preserve-preview-focus-on-window-return
Sep 12, 2026
Merged

fix(web): preserve preview focus on window return#11444
maria-rcks merged 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/preserve-preview-focus-on-window-return

Conversation

@Lucenx9

@Lucenx9 Lucenx9 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

What changed

Leave focused iframe and Electron webview previews alone when the window-return handler checks whether to refocus the composer. This adds two tag checks to the existing predicate and covers both with regression tests.

Why

Returning to T3 by clicking an input in an embedded preview can move focus straight back to the composer. Window activation schedules a refocus check, the click focuses the frame, and the check then treats that frame as a place from which it can take focus. This is a timing-dependent race, not a failure on every window return.

The parent document sees IFRAME or WEBVIEW, not the input inside it. Treating those elements as input owners preserves the user's click without changing the refocus scheduling.

flowchart LR
    A[Return by clicking a preview input] --> B[Before: queued check refocuses composer]
    B --> C[Typing goes to the composer]
    A --> D[After: preserve frame focus]
    D --> E[Typing stays in the preview]
Loading

Scheduling and the existing button, field, dialog and terminal behavior are unchanged. The shared web/desktop code changes; native mobile and mobile viewports do not use this handler.

This concerns the user's return to the window, not the agent keyboard dispatch corrected by #11354.

Verification

  • Both new regression cases fail on the base commit. All 145 tests in ChatView.logic.test.ts pass with the fix.
  • From apps/web: vp test run --project unit src/components/ChatView.logic.test.ts --maxWorkers=1 --no-file-parallelism.
  • Targeted lint and formatting checks pass for the two changed files.
  • tsc --noEmit -p apps/web/tsconfig.json passes, with existing Effect suggestions in unrelated files.
  • Fallow review of apps/web, with one parser thread, reports low risk. No dependencies, exports or diagnostic suppressions are added.
  • In Chromium, the preview retains focus after the fix, including three repeat activation-click checks. Returning from an ordinary button still refocuses the composer.

UI changes

Captured in the running T3 web client with an isolated thread and HTML fixture. Start with the composer focused and the terminal drawer closed, activate another window, then return by clicking the preview input and type without clicking again.

Before After
Typing lands in the composer Typing stays in the preview

23-second interaction video after the fix. PNGs are 2880 × 1800 at 2× density.

Verification limits: Chromium used native X11 window activation/clicks with Playwright focus emulation disabled, not dispatched DOM focus events. The local asset response omitted Content-Type, so the browser test supplied text/html for this one fixture response. The app and focus handler were otherwise unchanged. Plain tab return did not reproduce the bug. Electron's WEBVIEW exception has predicate coverage, not an end-to-end desktop verification.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Model: GPT-5 family. Harness: Codex.

Summary by CodeRabbit

  • Bug Fixes
    • Preserved focus in embedded frames and web views when the application window regains focus, preventing unintended composer refocusing.
    • Improved focus behavior for users interacting with embedded content.

Copilot AI lite review requested due to automatic review settings September 12, 2026 18:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-12T19:01:13.606329Z 93c283b PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 12, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 93c283b

Macroscope's review found this PR approvable — The patch makes a narrowly scoped focus-predicate correction for embedded iframe and Electron webview previews, with regression tests and no changes to other focus scheduling or application workflows. Its runtime impact is limited to preserving deliberate preview focus when the window regains focus.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: b562b001-ab21-44b4-a14e-408844c60993

📥 Commits

Reviewing files that changed from the base of the PR and between bbedad0 and 93c283b.

📒 Files selected for processing (2)
  • apps/web/src/components/ChatView.logic.test.ts
  • apps/web/src/components/ChatView.logic.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The window-focus handler now preserves focus on IFRAME and WEBVIEW elements. A parameterized test verifies both element types.

Changes

Focus preservation

Layer / File(s) Summary
Window-focus guard and coverage
apps/web/src/components/ChatView.logic.ts, apps/web/src/components/ChatView.logic.test.ts
The handler skips composer refocusing for focused IFRAME and WEBVIEW elements. Tests verify both cases.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 93c28

This change preserves focus in embedded previews without altering other focus behavior, and verification is passing. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving preview focus when the window returns.
Description check ✅ Passed The description explains what changed, why it changed, verification results, UI evidence, and checklist completion. It satisfies the repository template and remains focused.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@maria-rcks
maria-rcks merged commit cfeaca4 into pingdotgg:main Sep 12, 2026
21 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 13, 2026
## What's Changed
* fix(mobile): render photo library picks to a bounded JPEG off the JS thread by @Nelglor in pingdotgg/t3code#11440
* fix(desktop): keep the native preview User-Agent so Turnstile passes by @akriaueno in pingdotgg/t3code#7110
* fix(chat): keep user input outside collapsed work by @maria-rcks in pingdotgg/t3code#11363
* fix(web): preserve preview focus on window return by @Lucenx9 in pingdotgg/t3code#11444
* fix(web): complete thread status icons and keep input threads prominent by @maria-rcks in pingdotgg/t3code#11461
* feat(web): tint image chips with their average color by @maria-rcks in pingdotgg/t3code#11468
* fix(web): move viewer controls outside media and restore arrow navigation by @maria-rcks in pingdotgg/t3code#11470
* fix(web): tighten sidebar search and footer spacing by @maria-rcks in pingdotgg/t3code#11466
* feat(web): subagent spawns render as an expandable work row by @maria-rcks in pingdotgg/t3code#11433
* fix(web): keep subagent rows visible under folded turns by @maria-rcks in pingdotgg/t3code#11474
* fix(usage): make unavailable account limits more visible by @dominic-r in pingdotgg/t3code#10601
* fix(desktop): bound backend shutdown wait during quit by @ishaanko in pingdotgg/t3code#7599
* feat(web): choose the default diff file state by @maria-rcks in pingdotgg/t3code#11484
* feat(composer): fold large pastes into text attachments by @chrisdeeming in pingdotgg/t3code#11442
* feat(web): expose each chat message as a heading for screen readers by @Leos-Khai in pingdotgg/t3code#11199
* fix(usage): respect provider account homes by @maria-rcks in pingdotgg/t3code#11485

## New Contributors
* @Nelglor made their first contribution in pingdotgg/t3code#11440
* @akriaueno made their first contribution in pingdotgg/t3code#7110
* @Leos-Khai made their first contribution in pingdotgg/t3code#11199

**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260912.1612...v0.0.41-nightly.20260913.1625

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260913.1625
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 13, 2026
Merges `upstream/main` at `0c5771d60` into the fork, from merge base
`e81606494` — 32 upstream commits.

The range is mostly client polish, plus two structural changes that
mattered
here: upstream extracted the sidebar header into a new component
(`SidebarThreadHeader.tsx`, pingdotgg#11315), which is where two fork gates had
to be
re-homed, and upstream added a `context` field to orchestration messages
at the
exact anchor the fork's `origin` field sits on, which is four of the
eight
conflicts.

## Merge stats

- Landed (`HEAD^1..HEAD`): 489 files, 36117+/5930−
- Upstream range (base..`HEAD^2`): 484 files, 35784+/5824−
- Fork delta (`HEAD^2..HEAD`): 767 files, 78724+/2528−

The two file lists reconcile exactly. The 5 extra landed files are all
fork-owned and none of them is upstream work:
`apps/web/src/fork/SidebarThreadFilter.tsx`
(one className, described below), `docs/fork/inventory.json`,
`docs/fork/gaps.md`, `docs/fork/upstream-merge-log.md`, and
`.agents/skills/fork-upstream-merge/scripts/unsupported-methods.mjs`.
Nothing in
the upstream range failed to land.

## Conflicts

All 8 were resolved by the verdict `preflight.mjs` printed. No `decide`
conflict
was left unresolved.

- `projector.ts`, `orchestration.ts`, `threadReducer.ts`,
`MessagesTimeline.tsx`
— `converged — message-origin-upstream-files`, and all the same
conflict:
upstream appended where the fork already appends. Both sides kept, twice
per
file for the first three. `duplicate-adds.mjs` confirms no line was
taken
  twice.
- `Sidebar.tsx` — `converged — thread-visibility-upstream-files`. Took
upstream
whole; its `SidebarThreadFilter` import was left unused by the
extraction and
  was removed.
- `SettingsSidebarNav.tsx` — unlisted. Kept the fork's
`settingsPathEnabled`
filter over the personal nav items and took upstream's new active-state
rule
(`/settings/general` stays active on `/settings/open-source-licenses`).
- `ChatComposer.tsx` — unlisted, so `decide, then add an entry`. Both
fork deltas
  survived and the entry is now written; see below.
- `routeTree.gen.ts` — generated; regenerated with
`regen-route-tree.mjs` after
  the install.

`pnpm-lock.yaml` auto-merged rather than conflicting, so it was reset to
`upstream/main` and the fork edges re-derived with `vp i`. The remaining
diff
against upstream is exactly the `@t3tools/moatless-api` workspace link,
`mermaid ^11.17.2`, and one alchemy peer hash.

Two findings worth naming here:

- **A fork gate's host file was replaced by a file upstream had not
written yet.**
  pingdotgg#11315 extracted the whole sidebar header into
`apps/web/src/components/sidebar/SidebarThreadHeader.tsx`. Both fork
deltas
were re-applied there additively — the `FEATURES.projectManagement` gate
on
New project, and `<SidebarThreadFilter />` as a third child of
upstream's new
segmented icon well. No props threaded, no state added, no upstream JSX
re-indented. The one edit outside that file is
`SidebarThreadFilter.tsx`'s
  trigger className, now `size-7` so it matches upstream's own
  `SidebarHeaderIconButton` in the well it now sits in.
- **The unsupported-method derivation could not read the backend, and
that was
the script's fault, not a finding.** `unsupported-methods` exited 2 with
"could
not read the backend dispatch". The Moatless backend moved its dispatch
a
second time: `crates/t3code/src/rpc/dispatch.rs` is now a module stub
over an
`rpc/dispatch/` directory whose `routing.rs` holds the arms and whose
siblings
hold the handler bodies. `BACKEND_APIS` now names the directory and the
script
concatenates every `.rs` file in it — pointing it at `routing.rs` alone
would
have read the arms and lost the handlers, and `refusesInside` only
follows
calls it can find in the same source, so every conditional refusal would
have
  come back as a false DROP.

## Inventory

- `moatless-admin-pages` was stale: it still listed the two Workspaces
admin
routes that the 2026-09-12 commit folded into the project settings page.
Re-pointed to the five surfaces that remain, and the untracked delta
that move
  left behind is now its own entry, `project-workspace-settings`.
- `chat-surface-gates` gained
`apps/web/src/components/chat/ChatComposer.tsx`
with a guard on `FEATURES.accessMode`, plus a `chat-composer-gates` path
policy
so the next merge gets a cached verdict instead of the same decision.
The two
deltas there are the runtime-mode picker lifted into a
`runtimeModePicker`
  const behind the flag, and `phase === "running"` left out of
  `collapsedComposerPrimaryActionDisabled`.
- `inventory-check.mjs` is clean.

## Unsupported methods

0 ADD, 0 DROP, 2 KEEP (`git.preparePullRequestThread`, `vcs.switchRef`),
5 known
exceptions still firing, no stale ones. `packages/contracts/src/rpc.ts`
is
unchanged: the range's one unsupported-surface change is upstream's
Cursor
`--classic` launcher fix, which lands on a method already refused.

## Feature classification

### Usable as-is

Client-side work the fork can expose with no Moatless backend or
deployment
change. 28 of the 32 commits.

- Open-source license notices page (pingdotgg#8962) — new
`/settings/open-source-licenses`
route; upstream also made `/settings/general` stay active while it is
open.
- Client perf: fewer repeated sorts and date formats (pingdotgg#11019).
- Inline file previews and attachment chips across surfaces (pingdotgg#11265) —
rides
  `attachments.createUploadUrl` and `assets.createUrl`, both dispatched.
- Subagent spawns as an expandable work row (pingdotgg#11433) and those rows kept
visible
under folded turns (pingdotgg#11474) — derived from the orchestration event
stream the
  backend already serves.
- Opt-in thread notifications and sounds (pingdotgg#11481) — client settings,
persisted
  through the `server.getSettings` read the backend serves.
- Large pastes folded into text attachments (pingdotgg#11442); user input kept
outside
collapsed work (pingdotgg#11363); each chat message exposed as a heading for
screen
  readers (pingdotgg#11199); the default diff file state (pingdotgg#11484).
- Sidebar project scope folded into the search row (pingdotgg#11315); thread
status icons
completed and input threads kept prominent (pingdotgg#11461); sidebar search and
footer
  spacing (pingdotgg#11466); draft row heights matched to thread rows (pingdotgg#11512).
- Image chips tinted with their average colour (pingdotgg#11468); viewer controls
moved
outside the media with arrow navigation restored (pingdotgg#11470); snapshot
preview size
preserved in sent messages (pingdotgg#11429); preview focus preserved on window
return
  (pingdotgg#11444).
- Unavailable account limits made more visible (pingdotgg#10601) — web-only; the
backend
  dispatches `server.getUsageSummary`.
- Saved environments switched off instead of removed (pingdotgg#11478) — entirely
client-side (connection catalog and registry). This build runs one
environment
and gates the Connections settings page, so nothing on screen changes;
the
  catalog behaviour carries.
- Desktop and mobile: long offscreen text in SnapShots (pingdotgg#11250), native
preview
User-Agent kept for Turnstile (pingdotgg#7110), bounded backend shutdown wait on
quit
(pingdotgg#7599), expo-audio pinned (pingdotgg#11426), photo library picks rendered to a
bounded
JPEG off the JS thread (pingdotgg#11440), launch crash with a PR stack (pingdotgg#11486),
the
  shared-content alert after sending (pingdotgg#11487).
- Repository hygiene: `.pnpm-store/v11` deleted.

### Unsupported in Moatless / needs implementation

- **Cursor links open in classic IDE mode (pingdotgg#11498).** Upstream gave
Cursor
`baseArgs: ["--classic"]` in `packages/contracts/src/editor.ts` so a
file open
  targets the IDE rather than its Agents Window, and tested it in
  `apps/server/src/process/externalLauncher.ts`. The method behind it,
`shell.openInEditor`, is not dispatched — the browser is not on the
machine the
workspace is on — so this lands in the contract and in `apps/server` and
changes nothing here. Recorded in `docs/fork/gaps.md` under _Opening in
an
external editor_, whose standing conclusion is that the surface is a
candidate
  for deletion rather than for serving.

### Backend behavior to consider reproducing in Moatless

All three are recorded in `docs/fork/gaps.md` under _Runtime fixes
upstream made
to its own server_. Nothing in this repository holds them open; they are
Moatless-side work.

- **Listing pull requests should read only the projects asked about
(pingdotgg#11299).**
`listWorkspaceProjects` fetched the whole shell snapshot and filtered
it; it now
  asks the projection for the one project, or for the listed ids
  (`apps/server/src/pullRequest/PullRequestService.ts`,
  `persistence/Layers/ProjectionSnapshotQuery.ts`). Moatless dispatches
`pullRequests.summary`, so the same cost lands on it as soon as a
summary is
  derived from a list.
- **Usage should read each provider account's own history directory
(pingdotgg#11485).**
Upstream resolves an account's home from its home setting or its
`CODEX_HOME` /
`CLAUDE_CONFIG_DIR` / `GROK_HOME` variable, counts disabled accounts,
and
de-duplicates accounts sharing a directory
(`apps/server/src/usage/UsageService.ts`).
Moatless serves `server.getUsageSummary` itself, so an account with a
custom
home reports zero there — or double — until it resolves homes the same
way.
- **Forgejo and Gitea remotes should be first-class source control
(pingdotgg#11436).**
Upstream recognises both hosts and drives them with the `fj` and `tea`
CLIs
  across remote identity, PR creation and PR sync (`git/GitManager.ts`,
  `project/RepositoryIdentityResolver.ts`,
`orchestration/PullRequestSyncReactor.ts`). Moatless owns git and pull
requests,
so a Forgejo or Gitea project is an unrecognised host there regardless
of what
  the client can render.

## Verification

`verify.mjs` is green on seven of eight checks: `duplicate-adds` (none
across 34
files both sides changed), `tripwires` (3 deleted surfaces intact,
exactly the 5
known re-deletions, 3 allowed workflows), `resolution-check` (16
fork-delta paths
still differ from upstream, 17 carry upstream's change, 17
theirs-verbatim
byte-identical, 18 unlisted), `unsupported-methods`, `fmt:check`,
`lint`,
`typecheck`.

`test` is red on one file, and it is the standing environmental failure
rather
than a merge regression:

- `@t3tools/desktop` → `scripts/browser-secret-native.test.mjs > bundled
libsecret
helper` fails with `Package 'libsecret-1' not found` from `pkg-config`.
1 file
  of 105; the rest of the package is 1341 tests passed. The test file is
byte-identical to upstream, arrived on the fork before this merge, and
the
sandbox image ships neither `libsecret-1` nor its pkg-config file. There
is no
  root in the sandbox, so it cannot be installed here. Recorded in
`docs/fork/gaps.md` under _The desktop suite needs libsecret, which the
sandbox
  does not have_.

Four packages did not finish under `vp run -r test` and were each run
alone
again, all green: `@t3tools/mobile` (165 files, 1528 tests), `t3` (317
files,
4528 tests), `@t3tools/web` (412 files, 5205 tests), `t3code-relay` (30
files,
284 tests).

The owned-concern sweep over newly added upstream files found no keyword
hits, so
no `concerns` entry was needed.

**CI caught one thing no local check runs.** `Build & push moatless-t3`
failed on
the first push: upstream's new `t3code:third-party-licenses` plugin
(pingdotgg#8962) runs
in `generateBundle` and refuses any bundled package whose license it
cannot
resolve, and three packages reach the web bundle only through the fork's
own
`mermaid` edge — `khroma` via mermaid, `fastdom` and `strictdom` via
cytoscape
under it — so upstream's config has never carried overrides for them.
Fixed with
three `packageOverrides` entries: `khroma` needed a `license: "MIT"`
declaration
only, since it ships its own `license` file, and `fastdom` and
`strictdom` needed
a `generatedNotice` each, since both declare MIT and ship no notice
file.
Verified with the build itself — all three now appear in
`apps/web/dist/third-party-licenses.json` with a license and a notice,
and the
workflow is green. The delta is held by the `mermaid-diagrams` inventory
entry
plus a `third-party-licenses-config` path policy, and the reason it
escaped
`verify.mjs` — which has no build step at all — is now
`docs/fork/gaps.md`, _Nothing builds the web app before a merge is
pushed_.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants