Skip to content

refactor(mobile): git sheets use uniwind platform variants instead of className ternaries - #13161

Merged
juliusmarminge merged 1 commit into
mainfrom
agent/mobile-audit-platform-variants-retry
Sep 23, 2026
Merged

juliusmarminge merged 1 commit into
mainfrom
agent/mobile-audit-platform-variants-retry

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Problem

The mobile audit found 389 Platform.OS checks in apps/mobile while Uniwind's ios:/android: className variants were used only a handful of times — including 65 inline Platform.OS === "android" ? "..." : "..." className ternaries. The git sheet family was one of the densest clusters.

Change

Converts every inline platform className ternary in the git sheet family to a single class string using Uniwind ios:/android: variants, following the pattern already used in apps/mobile/src/features/settings:

  • GitCommitSheet, GitBranchesSheet, GitConfirmSheet, GitOverviewSheet, gitSheetComponents
  • Shared classes stay in base; classes that differ per platform move under ios:/android: prefixes, so the two platforms never fight over the same utility. Where the converted call site renders an AppText/AppTextInput, the component's own defaults (font-sans text-foreground) stay in the base layer and the new ios:/android: utilities override them through Uniwind's variant cascade — the same mechanism the existing settings-screen android: usage relies on.
  • Structural platform branches (android-only headers, checkbox views, dividers), style objects, sheet options, and icon-size props are untouched — those are real platform logic, not styling.

Measurements

Metric Before After
Platform.OS occurrences in apps/mobile/src 385 336
className ternaries in the five git-sheet files ~49 0
ios:/android: variant utilities in apps/mobile/src 11 222

HomeScreen and ThreadNavigationSidebar were deliberately left alone (active recycler/error work). ReviewSheet/AddProjectScreen can follow the same pattern in a separate pass.

Verification

  • tsc --noEmit (apps/mobile): clean
  • vp lint apps/mobile/src/features/threads/git/: 0 errors; the four react-compiler warnings are pre-existing (identical on the HEAD versions of the same files)
  • vp test run src/features/threads/git: passing
  • expo export --platform ios --dev false: succeeds, and the exported bundle contains compiled selector artifacts for the new variant classes (e.g. ios:border-border-subtle, android:active:bg-subtle), confirming Uniwind compiles them rather than dropping them
  • Before/after device evidence per platform (iOS + Android) through the overview, branches, commit, and confirm sheets is still required before merge and will come from the integrated mobile pass. One item to inspect explicitly there: the GitConfirmSheet description line, where base leading-normal coexists with per-platform text sizes (android:text-base / ios:text-sm) — Tailwind 4 emits a paired line-height on text-*, so the effective line-height (original: multiplier 1.5 from leading-normal) needs a visual confirmation on both platforms.

Surface checklist

  • Clients: mobile only; no wire contract, no web/desktop impact.
  • Providers: n/a.
  • Reverse states / connection modes: n/a (pure presentational class refactor).

Model: Claude (Apex by Callstack, pi harness).


Devin Review

Summary by CodeRabbit

  • Refactor
    • Updated Git-related mobile sheets to use platform-specific styling consistently across Android and iOS.
    • Preserved existing layouts, visual appearance, validation, loading states, navigation, and Git actions.

… className ternaries

The mobile audit found hundreds of Platform.OS checks where Uniwind's
ios:/android: className variants already express the same thing. This
converts the inline Platform.OS === "android" ? "..." : "..." className
ternaries in the git sheet family (GitCommitSheet, GitBranchesSheet,
GitConfirmSheet, GitOverviewSheet, gitSheetComponents) to single class
strings with ios:/android: variants. Structural platform branches, style
objects, and icon-size props are unchanged.

apps/mobile Platform.OS occurrences drop from 385 to 336; the five files
now contain zero className ternaries and ~210 platform-variant utilities
in their place. Verified with targeted tsc, lint, feature tests, and a
Metro iOS export confirming every new variant class compiles.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 22, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 5c191ec

Macroscope's review found this PR approvable — This is a focused refactor that replaces existing iOS/Android className ternaries with equivalent Uniwind platform variants across five Git sheet files. Functional logic, native platform branches, and product defaults remain unchanged; the impact is limited to existing presentation styling.

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

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −9 B (−0.1%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −11 B (−0.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.3 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 10 10 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +29 B (+0.2%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −5 B (−0.1%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.5 KiB +34 B (+0.5%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.1 KiB +44 B (+0.1%) 66.4 KiB ✅
Claude Live turn messages 9 10 +1 (+11.1%) 21 ✅

Baseline: bed0b4c · PR result: 5c191ec · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: fde8b2ad-fa37-4198-85f3-9687061513a9

📥 Commits

Reviewing files that changed from the base of the PR and between bed0b4c and 5c191ec.

📒 Files selected for processing (5)
  • apps/mobile/src/features/threads/git/GitBranchesSheet.tsx
  • apps/mobile/src/features/threads/git/GitCommitSheet.tsx
  • apps/mobile/src/features/threads/git/GitConfirmSheet.tsx
  • apps/mobile/src/features/threads/git/GitOverviewSheet.tsx
  • apps/mobile/src/features/threads/git/gitSheetComponents.tsx

Limit details: You’ve used all 10 included reviews currently available.


📝 Walkthrough

Walkthrough

Changes

NativeWind platform styling

Layer / File(s) Summary
Shared sheet component styling
apps/mobile/src/features/threads/git/gitSheetComponents.tsx
MetaCard and SheetListRow replace runtime platform class selection with Android and iOS utility variants.
Overview and confirmation sheet styling
apps/mobile/src/features/threads/git/GitOverviewSheet.tsx, apps/mobile/src/features/threads/git/GitConfirmSheet.tsx
Platform-specific classes move to NativeWind variants for scroll content, cards, headers, spacing, and actions.
Branch and commit sheet styling
apps/mobile/src/features/threads/git/GitBranchesSheet.tsx, apps/mobile/src/features/threads/git/GitCommitSheet.tsx
Platform-specific classes move to NativeWind variants across forms, rows, controls, inputs, and action areas. Runtime behavior remains unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Refactor

Suggested reviewers: pixpmusic

Merge Risk: ⚪ Minimal · up to 5c191

No concrete current-head regression was established; the styling migration is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly and concisely describes the main change: replacing className ternaries with Uniwind platform variants in the mobile git sheets.
Description check ✅ Passed The description clearly explains the problem, scope, implementation, validation, and affected surfaces. It does not include the required before/after UI evidence, but it explicitly identifies that evi…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@juliusmarminge
juliusmarminge merged commit ec28eef into main Sep 23, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the agent/mobile-audit-platform-variants-retry branch September 23, 2026 00:11
@juliusmarminge

Copy link
Copy Markdown
Member Author

Root cause found — closing this PR (revert pushed: f317a434)

Both device-pass regressions (iOS commit-footer shift, Android blank sheets) are real and share one root cause: ios:/android: variants are not platform-guarded in this repo's uniwind 1.11.0 pipeline, so the premise of this refactor does not hold.

Evidence

Compiled stylesheet embedded in an expo export --platform ios bundle of 5c191ec (production-like Metro pipeline) contains the Android entries in the iOS bundle, with their platform metadata stripped:

"android:p-4": [{ "entries": [["padding", … * 4]], "native": false, "complexity": 0, … }]
"android:bg-sheet-solid": [ … "native": false … ]
"ios:flex-1": [ … "native": false … ]
  • addMetaToStylesTemplate is supposed to drop styles where platform !== currentPlatform, but the entries arrive with platform: null (the native: false field is literally platform !== null), so nothing is dropped.
  • The runtime store (node_modules/uniwind/src/core/native/store.ts) has no platform check in its style filter — guards cover only minWidth/maxWidth/theme/orientation/rtl/active/focus/disabled/dataAttributes.
  • All variant tokens compile with complexity: 0, so the only remaining tie-break for a shared property is last token in the className string wins — identically on both platforms.

Why each device pass saw what it saw

  • iOS commit block +14px down/right, gray input: commit-message wrapper's android:p-4 leaks (no iOS counterpart to overtake it; with polyfills: { rem: 14 } in metro.config.js, p-4 = 14px) and TextInput's android:bg-sheet-solid leaks. Branch/overview/confirm matched only because the leaked android:p-4/android:gap-3 was masked by later iOS utilities for the same properties.
  • Android blank sheets: className="android:shrink android:grow-0 ios:flex-1" — ios:flex-1 is the last token, so on Android the ScrollView also gets flex-basis: 0% + grow 1, collapsing content to zero height in the fitToContents sheet. Same for the root bg-sheet ios:flex-1.

Impact beyond this PR

This also affects already-shipped android: usages (they apply on iOS today, masked or subtle): SettingsControlRow (android:gap-1, android:text-base), SettingsServerControlsRouteScreen (android:px-4), SettingsControlRow layout (android:min-h-14 android:py-3). Worth its own repro + fix (uniwind platform tagging, likely in the MQ/mediaType handling of the 1.11.0 pipeline or the lightningcss version it runs against) before any ios:/android:-based migration is attempted again.

Revert commit restores the exact ternaries; branch history is kept so the conversion can be re-attempted once platform variants are actually guarded. Net PR diff after the revert is zero — there is nothing to dual-audit here anymore.

juliusmarminge added a commit that referenced this pull request Sep 23, 2026
#13161 replaced Platform.OS className ternaries in the git sheets with
ios:/android: Uniwind variants; record the tiering: class variants for
styling, Platform.OS for behavior, file variants for wholesale UI.

Co-authored-by: Apex <noreply@callstack.com>
juliusmarminge added a commit that referenced this pull request Sep 23, 2026
Device root-cause analysis showed Uniwind 1.11.0 ios:/android: class
variants are not platform-guarded in this repo's Metro pipeline; classes
leak across platforms. Keep Platform.OS className branches for styling
until #13161 is reverted on main and re-proved on both devices.

Co-authored-by: Apex <noreply@callstack.com>
juliusmarminge added a commit that referenced this pull request Sep 23, 2026
#13169 reverted #13161 on main, but Uniwind 1.11.0 platform variants stay
unguarded in the Metro pipeline. Say they must not be used until the
pipeline is fixed/upgraded and proved on both devices, and that existing
variant usages are unsafe and should not be copied.

Co-authored-by: Apex <noreply@callstack.com>
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 23, 2026
## What's Changed
* chore(mobile): drop dead nitro-markdown tgz override and @expo/metro-runtime by @juliusmarminge in pingdotgg/t3code#13148
* feat(web): show settings scope as a sentence at the top of the page by @juliusmarminge in pingdotgg/t3code#13139
* refactor(web): move settings scope pickers into breadcrumbs by @Yash-Singh1 in pingdotgg/t3code#13165
* feat(auth): share provider sign-in flows and credential bindings by @juliusmarminge in pingdotgg/t3code#12983
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13161
* chore(mobile): name the two project favicon caches by their job by @juliusmarminge in pingdotgg/t3code#13160
* revert(mobile): git sheets back to Platform.OS ternaries (un-guarded uniwind variants broke both platforms) by @juliusmarminge in pingdotgg/t3code#13169
* docs(mobile): document the two mobile routes that intentionally skip deep links by @juliusmarminge in pingdotgg/t3code#13164
* refactor(mobile): break module cycles with focused extractions by @juliusmarminge in pingdotgg/t3code#13151
* fix(server): generate PR diffs from branch changes by @Yash-Singh1 in pingdotgg/t3code#13170
* fix(web): preserve nested scroll behavior in chat timeline by @Yash-Singh1 in pingdotgg/t3code#13167
* test(web): cover usage model ordering without static markup by @flamboh in pingdotgg/t3code#13104
* fix(desktop): find linuxbrew node for the WSL backend by @CodyRay in pingdotgg/t3code#7827
* chore(models): use GPT-6 Luna for text generation by @extoci in pingdotgg/t3code#13115
* fix(mobile): keep ordinary offline outbox failures out of console.warn by @juliusmarminge in pingdotgg/t3code#13144
* feat(providers): check remote compatibility ranges by @juliusmarminge in pingdotgg/t3code#13130
* chore(lint): keep mobile theme escape-hatch allowlist honest by @juliusmarminge in pingdotgg/t3code#13146
* fix(web): the pull request badge reads at the meta size again by @juliusmarminge in pingdotgg/t3code#13175
* fix(mobile): uniwind platform variants stay guarded on both platforms by @juliusmarminge in pingdotgg/t3code#13172
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13185
* refactor(mobile): remaining className platform ternaries become class variants by @juliusmarminge in pingdotgg/t3code#13188
* fix(web): align provider emails without clipping by @Derpedyea in pingdotgg/t3code#13174
* perf(mobile): recycle the default v2 home list and scope the snooze minute tick by @juliusmarminge in pingdotgg/t3code#13149
* refactor(mobile): retire the legacy grouped thread list by @juliusmarminge in pingdotgg/t3code#13183
* fix(server): background PR checks spend less GitHub quota by @juliusmarminge in pingdotgg/t3code#13189
* fix(server): background PR sync reads summaries in batches by @juliusmarminge in pingdotgg/t3code#13198
* fix(server): GitHub PR lookups stop probing owner-qualified heads by @juliusmarminge in pingdotgg/t3code#13200
* chore(mobile): clear the legacy-list deletion fallout by @juliusmarminge in pingdotgg/t3code#13203

## New Contributors
* @CodyRay made their first contribution in pingdotgg/t3code#7827

**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260922.2123...v0.0.43-nightly.20260923.2135

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260923.2135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 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