Skip to content

Revert "fix(mobile): keep Android markdown icons aligned with text" - #11098

Merged
juliusmarminge merged 1 commit into
mainfrom
revert-11079-t3code/fix-android-text-overlays
Sep 10, 2026
Merged

Revert "fix(mobile): keep Android markdown icons aligned with text"#11098
juliusmarminge merged 1 commit into
mainfrom
revert-11079-t3code/fix-android-text-overlays

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 10, 2026

Copy link
Copy Markdown
Member

Reverts #11079

PR adds a bunch of platform conditionals in a file that already runs on a single platform. I am very confused and haven't seen a proof that this doesn't regress the ios side of thigns so reverting.

Summary by CodeRabbit

  • Bug Fixes

    • Improved text copying so inline image placeholders are removed without altering surrounding authored whitespace.
    • Removed unnecessary spacing before file and external-link icons on Android.
    • Standardized vertical spacing between document blocks for a more consistent reading experience.
  • Documentation

    • Updated composer guidance to remove outdated Android-specific text selection and message-copy instructions.

@juliusmarminge
juliusmarminge merged commit 3836890 into main Sep 10, 2026
19 of 20 checks passed
@juliusmarminge
juliusmarminge deleted the revert-11079-t3code/fix-android-text-overlays branch September 10, 2026 17:35
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 10, 2026
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 105423d5-56dd-47be-afa1-52f17212080c

📥 Commits

Reviewing files that changed from the base of the PR and between 444fd8b and 942b48a.

📒 Files selected for processing (9)
  • apps/mobile/modules/t3-markdown-text/android/build.gradle
  • apps/mobile/modules/t3-markdown-text/android/src/main/java/expo/modules/t3markdowntext/T3MarkdownTextSelectionModule.kt
  • apps/mobile/modules/t3-markdown-text/android/src/test/java/expo/modules/t3markdowntext/MarkdownSelectionCopyTest.kt
  • apps/mobile/modules/t3-markdown-text/src/NativeMarkdownBlock.ios.tsx
  • apps/mobile/modules/t3-markdown-text/src/NativeMarkdownSelectableText.ios.tsx
  • apps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx
  • apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts
  • apps/mobile/src/lib/nativeMarkdownText.test.ts
  • docs/user/composer.md

📝 Walkthrough

Walkthrough

The changes simplify Android selection sanitization, remove obsolete Android test setup, inline markdown block spacing, and adjust selectable text rendering for non-rich chunks and icon runs. Related composer documentation is removed.

Changes

Markdown rendering and selection

Layer / File(s) Summary
Android selection sanitization
apps/mobile/modules/t3-markdown-text/android/...
Selection copying now removes only inline-image replacement characters. The obsolete Robolectric test class and test-only Gradle configuration are removed.
Markdown block spacing
apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts, apps/mobile/modules/t3-markdown-text/src/NativeMarkdownBlock.ios.tsx, apps/mobile/src/lib/nativeMarkdownText.test.ts
Block spacing is calculated inline. Document children use uniform spacing and direct rendering. Tests retain spacer-run validation.
Selectable rendering paths
apps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx, apps/mobile/modules/t3-markdown-text/src/NativeMarkdownSelectableText.ios.tsx, docs/user/composer.md
Non-rich Android chunks use selectable text rendering. Icon runs no longer receive a leading non-breaking space. Android selection guidance is removed from the composer documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: sunkenintime

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch revert-11079-t3code/fix-android-text-overlays

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

github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 10, 2026
## What's Changed
* fix(mobile): keep Android markdown icons aligned with text by @SunkenInTime in pingdotgg/t3code#11079
* Revert "fix(mobile): keep Android markdown icons aligned with text" by @juliusmarminge in pingdotgg/t3code#11098


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260910.1486...v0.0.41-nightly.20260910.1507

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260910.1507
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 11, 2026
Merges `upstream/main` at `02297e3db` into the fork, 35 commits from
base
`0f602b337`. Merge commit, not a rebase. Tracker entry:
`docs/fork/upstream-merge-log.md`, 2026-09-11.

`170` files landed against `166` in the upstream range; fork delta `756`
files.
The gap is six named files and reconciles:
`ThreadStatusIndicators.test.tsx` and
`sandboxControl.placement.test.tsx` landed as fork-test fixes amended
into the
merge, the three fork documents landed with it, and
`PreviewLocalServerCard.tsx` was re-deleted per the inventory's
deliberate-deletion list. `duplicate-adds.mjs` and
`resolution-check.mjs` are
clean — nothing landed as one side whole.

## What upstream shipped, and where it stands on Moatless

### Usable as-is

These run on the fork's backend with no further work.

- **A compact right-panel surface menu** (pingdotgg#11111) — the add-surface
launcher
goes from a card grid to keyboard-shortcut rows. This is where the
fork's
sandbox status badge lives, so the badge was re-stated on upstream's row
  rather than replayed; see the conflict notes below.
- **Multiple-linked-PR badges, simplified** (pingdotgg#11104, pingdotgg#11180, pingdotgg#11101) — a
thread
with several links now shows the total linked count coloured by
aggregate
status, instead of naming a primary and counting the extras. This is
**live on
  Moatless**: the backend serves `thread.pullRequests` and reports
`threadPullRequests`, so the badge resolves. It is also a behaviour
change a
user will see, and it is what broke the fork's own badge test — the only
thing
  that caught it.
- **Settled threads recede in the sidebar** (pingdotgg#11101) — a settled row
dims until
  hover or focus. Rides the settlement state Moatless already serves.
- **Environments in the command palette** (pingdotgg#10722) — searching now
returns
  environments beside threads and projects, with a subtitle.
- **A blue/orange diff palette** (pingdotgg#10671) — client-side theme only.
- **Question answers folded into tool activity** (pingdotgg#11014) — the chat
timeline
renders an answer to an agent's async question inside the tool call that
asked
  it, rather than as a separate turn. The client half
(`client-runtime/src/work-log/userInput.ts`,
`shared/src/toolActivity.ts`,
`MessagesTimeline.tsx`) works off data Moatless already sends. The
server half
  is in the third bucket.
- **Unpriced model activity is flagged** (pingdotgg#11021) — usage rows with
tokens and
no price read as unpriced instead of `$0.00`. Shared merge logic over
the
  `server.getUsageSummary` the backend serves.
- **Return to picture-in-picture when the right panel closes** (pingdotgg#11102)
and
**aligned floating-preview corners** (pingdotgg#10915) — both land in the hosted
preview surface and both were taken; the fork's framed-runtime condition
in
`ThreadPreviewMiniPlayer.tsx` still covers the case upstream's `framed`
check
  does not.
- **Collapse a tool call by clicking its expanded label** (pingdotgg#11017),
**provider
update text fitting inside sidebar notices** (pingdotgg#11034), **no seams in the
topbar scroll fade** (pingdotgg#10914), **centered PR unavailable states**
(pingdotgg#11110), **no
  sidebar PR link icon** (pingdotgg#11179).
- **Mobile** (pingdotgg#11128, pingdotgg#11127, pingdotgg#11114, pingdotgg#11115, pingdotgg#11118 / pingdotgg#11079 / reverted
in
pingdotgg#11098, pingdotgg#11113) — shared markdown renderer rename, composer transition
and
final-frame fixes, tablet close controls for files and terminal, and
playback
  preserved across fullscreen transitions.

### Unsupported in Moatless / needs implementation

- **The device hub** — the whole of pingdotgg#10677, pingdotgg#10854, pingdotgg#10855 and pingdotgg#10856:
iOS
simulators and Android emulators a person and an agent can share. Server
side
is `apps/server/src/device/` (`LocalDeviceHost.ts` drives the machine
the
  server runs on, `SshDeviceHost.ts` drives another over SSH,
`DeviceHubProxy.ts` fronts the video and accessibility streams) plus an
MCP
  toolkit at `apps/server/src/mcp/toolkits/device/` that gives the agent
  tap/type/screenshot. Client side is a `device` right-panel surface
  (`apps/web/src/components/device/`) and a Device hosts settings page.

Eight methods plus one stream, all newly declaring
`UnsupportedMethodError` in
  `packages/contracts/src/rpc.ts`: `device.configure`, `device.list`,
  `device.testHost`, `device.open`, `device.close`, `device.shutdown`,
`device.detail`, `device.action`, and the `subscribeDeviceState` push
stream.

  **Nothing gates the surface on a capability.** `ChatView.tsx` passes
`deviceAvailable={activeThreadRef !== null}`, so the launcher offers a
Device
row on every thread. `subscribeDeviceState` never resolves on Moatless,
so the
state stays empty, `onboardingCompleted` is false, and clicking the row
opens
`DeviceSetup` rather than the panel — whose first step, "Enable the
device
hub", calls `device.configure` and shows the refusal. A dead end a
person can
walk into. One additive `FEATURES.deviceHub` read on the two
`deviceAvailable`
props would drop the row instead; **this merge did not add it**, and it
is
recorded as the open work in `docs/fork/gaps.md` under _The device hub_.

Implementing it on the backend is a real question rather than a stub:
the hub
needs Xcode or the Android SDK on whatever host it drives, and its
stream is a
  second connection beside the RPC one.
- **Label and reviewer updates without redundant reloads** (pingdotgg#11117) —
optimistic
  cache writes in `client-runtime/src/state/pullRequests.ts` over
`pullRequests.update`. Rides the `pullRequests.*` group, which Moatless
does
not serve and which the `pullRequests` capability already keeps off, so
it
  changes nothing here until that group lands.
- **Emphasised primary PR actions** (pingdotgg#11105) and **save a PR body with
  Cmd/Ctrl+Enter** (pingdotgg#10660) — same surface, same condition.
- **Zed remote links accepting root paths and Windows servers** (pingdotgg#11044)
— builds
an SSH open target the Electron shell hands to a local editor. That path
needs
  a desktop shell, so it does not reach this fork's browser client;
`shell.openInEditor` remains unsupported. Not a fork target, listed for
  completeness.
- **Marketing** (pingdotgg#11146, pingdotgg#11145) — `apps/marketing` is upstream's own
site;
  inherited and inert here.

### Backend behavior to consider reproducing in Moatless

Server-side fixes upstream made to its own runtime. Moatless implements
the same
contract, so each is worth checking against its own implementation.

- **Claude launch args override the derived permission mode** (pingdotgg#11026,
`apps/server/src/provider/Layers/ClaudeAdapter.ts`). Upstream derives a
permission mode from the thread's settings and then appends the agent's
launch
args; an explicit `--permission-mode` in those args used to be
overridden by
the derived one instead of winning. If Moatless derives a permission
mode the
same way, a user who set the flag explicitly is being ignored in the
same
  place. Cheapest of the three to check.
- **Project identity resolved before legacy PR relinks** (pingdotgg#11045,
`apps/server/src/orchestration/Layers/OrchestrationEngine.ts`). A legacy
pull-request link is relinked to its thread on load; upstream now
resolves the
project's repository identity first, so a relink cannot bind a link to
the
  wrong project when two projects share a branch name. The fork fills
`thread.pullRequests` from a Task's GitHub bindings, so it has the same
ordering question: the binding has to know which project it belongs to
before
  it is attached.
- **Question answers in the published activity payload** (pingdotgg#11014,
`apps/server/src/orchestration/ActivityPayloadProjection.ts`). The
projection
now folds `projectQuestionToolInput` into the activity payload for both
`mcp_tool_call` and plain tool items, which is what lets a client render
the
  answer inside the tool call. Moatless does not report
`agentActivityPublishing`, so nothing reads this today — but if it ever
  publishes activity, this is the shape to publish.

## Conflicts and how they were resolved

11 conflicted files, resolved on the verdicts `preflight.mjs` printed.

Additive on both sides, both kept: `MessagesTimeline.tsx` (imports),
`client-runtime/src/rpc/client.ts` (the fork's four subscription tags
against
upstream's `subscribeDeviceState`), `rightPanelStore.ts` (the fork's
`sandbox`
kind against upstream's `device`), `rpc.ts` and `RpcAuthorization.ts`
(the
fork's thread-server / sandbox / subtasks methods and scopes against
upstream's
eight `device.*` ones), and `ChatView.tsx` (both right-panel arms, both
`onAdd*`
props at the inline and sheet call sites, and upstream's extended
`closePreviewPanel` under the fork's proactive preview-open effect).

`PreviewEmptyState.tsx` and `ThreadPreviewMiniPlayer.tsx` took
upstream's
`DiscoveryList` and `rounded-[inherit]` with the fork's sandbox-read
error line
and framed `hasPreviewSurface` condition re-stated on top.
`PreviewLocalServerCard.tsx` was a modify/delete conflict and was
re-deleted.
`pnpm-lock.yaml` auto-merged this time, so `--theirs` had nothing to do;
it was
reset to `upstream/main` and the fork edges re-derived with `vp i`.

Two findings worth reading:

**A `converged` delta can lose the line it was anchored to.** pingdotgg#11111
rebuilt the
add-surface menu from cards into rows, and the fork's sandbox badge in
`RightPanelTabs.tsx` (7 conflicts, the hard one) had no literal home
left. It
was re-stated on upstream's row — between the label and the `Kbd`,
additively,
no prop threaded and no upstream JSX re-indented — rather than replayed.
The
fork's placement test caught the other half: upstream's rows stopped
rendering
the action `description` at all, in its own Device action too, so the
test was
asserting on markup nobody emits. It now asserts the rendered label.

**A silent auto-merge changed behaviour with no marker, no type error
and no
resolution-check hit.** pingdotgg#11104 and pingdotgg#11180 changed what the multi-PR
badge counts
and hoisted `state` onto both badge shapes with a new `draft` colour.
Only the
fork's `ThreadStatusIndicators.test.tsx` failed, on `+1` against `+2`.
Fixture
gained the now-required `state`; the expectation was updated to
upstream's
semantics.

## Inventory and gaps

- New `pathPolicy` rows for the paths this merge decided without a
cached
  verdict: `right-panel-surfaces`, `client-runtime-rpc-client`,
`thread-status-indicators`, `fork-sandbox-components`.
`inventory-check.mjs`
  is clean.
- New gaps entry: _The device hub_, under **Methods the backend does not
dispatch**. It names the nine union entries holding it open, the missing
  `FEATURES.deviceHub` gate, and what closing it costs.
- Owned-concern sweep: 11 keyword hits, all false positives. Ten are
device-hub
files matching the `client-identity` concern on `host`/`proxy` — that
concern
  is about device *pairing* identity, not simulators — and the eleventh,
  `McpProviderSession.test.ts`, matched on `session`. No concern entry.

## Verification

`verify.mjs`: `duplicate-adds`, `tripwires`, `resolution-check`,
`unsupported-methods`, `fmt:check`, `lint` and `typecheck` pass.

`test` is red on **one** package, and it is not this merge:
`@t3tools/desktop`'s `scripts/browser-secret-native.test.mjs > bundled
libsecret
helper` cannot find `libsecret-1` in this sandbox's pkg-config path. A
missing
system package, not a code defect — 1 file failed of 102, and the
standing entry
is in `docs/fork/gaps.md` under _The desktop suite needs libsecret_.

One thing to know about reading that log: four packages did not finish
under
`vp run -r test` and were each re-run alone — mobile `157` files, `t3`
`315`,
web `389`, relay `30`, all passing. The truncated parallel pass reported
a
failure that does not exist, `shikiReviewHighlighter.test.ts >
initializes
source and snippet highlighting without a warmup`, which passes in the
alone run
and which neither side of this merge touches. The retry lines at the end
of the
log are the result; the parallel output above them is not.

Not reported green. The fork has no CI on pull requests
(`docs/fork/gaps.md`, _Nothing checks a pull request_), so this is the
whole of
the evidence.

---
Moatless task:
https://moatless.soaplabstest.com/tasks/211b4f8f-7de2-46c8-9ded-330d0cda4add
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. size:M 30-99 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.

1 participant