Repository navigation
fix(mobile-web): page inputs lose the browser focus ring and hairlines draw one device pixel (OTA phase C follow-up) - #22569
Conversation
Chromium rings every focused text field (:focus-visible); no native TextInput paints one. A zero-specificity rule in its own inline block beside the Expo root reset removes it for every page input; buttons keep the browser's ring. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
react-native-web pins StyleSheet.hairlineWidth to 1 CSS px, three device pixels on a 480 dpi phone; native draws one. A build shim replaces that one assignment with React Native's own formula (roundToNearestPixel(0.4), else 1/ratio), so every page hairline matches native without touching components. A rendered check at a real device scale (Playwright's emulated scale floors borders to CSS px, which no phone does) measures both parity fixes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…JS build The page's dependencies require react-native, so esbuild resolves every importer to react-native-web's dist/cjs build, which the previous filter did not match: the shipped bundle still assigned hairlineWidth=1. The filter now matches both builds, the rendered check requires the package the way the page does, and a builder test reads every hairlineWidth assignment in the bundle. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
1/ratio is exactly one device pixel, and WebKit floors that to 0 and paints nothing (0.3333px at a scale of 3), so the iOS shell would have lost every hairline. The shim now uses native's device-pixel count plus half a pixel; both engines floor a border to whole device pixels, so each paints what React Native paints at ratios 1, 2, 3, 3.5 and 4, measured per engine. The rendered parity check now runs in WebKit at a device scale of 3 as well as Chromium. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…uild Drops a second full build that read one HTML string, plus two assertions that tested the constant against itself. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Half a pixel over native's count kept borders at one device pixel but made the separators drawn as `height: StyleSheet.hairlineWidth` straddle two rows at about half of all offsets. Both engines lay out in 1/64 CSS px, and WebKit stores an exact 1/3 as 21/64 and paints nothing, so the width is now native's device-pixel count over the ratio, rounded up to the next 1/64 (22/64 at 3). The rendered check adds a separator at a 10.1 px offset in Chromium and WebKit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe mobile web build now calculates Priority: ⬇️ Low Merge Risk: 🔵 Low · up to Task checkboxes may be difficult to navigate by keyboard, and hairlines may appear too thick on higher-density displays. These bounded issues warrant a fix or explicit acceptance before merging. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the motivation, implementation, scope, and test evidence, but it does not follow the required template. It omits the ELI5, Linked Issue, Visual Proof, Testing, Review, boundary, Notes, and Checklist sections. Resolution Add all required template sections. Include the issue link after “Fixes #”, attach before-and-after visual proof or state why it is not applicable, document manual and automated testing with platforms, complete the checklist, and provide the required review, compatibility, security, and AI disclosure information.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: e62cd492-bb3f-4748-9fed-bdd959c0450c
📒 Files selected for processing (3)
config/scripts/build-mobile-web-app-bundle.mjsconfig/scripts/build-mobile-web-app-bundle.test.mjsconfig/scripts/mobile-web-app-native-parity-render.test.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| * Those only: a button reached by a hardware keyboard keeps the browser's ring. | ||
| */ | ||
| export const MOBILE_WEB_APP_NATIVE_PARITY_STYLE = | ||
| '<style id="orca-native-parity">:where(input:focus,textarea:focus){outline:none}</style>' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '88,125p' mobile/src/components/rich-markdown/document-style.ts
sed -n '25,43p' mobile/src/components/rich-markdown/markdown-list-render.ts
sed -n '132,146p' config/scripts/build-mobile-web-app-bundle.mjsRepository: stablyai/orca
Length of output: 2847
🏁 Script executed:
rg -n -i 'checkbox|focus-visible|focus-within|:focus|outline|box-shadow' mobile/src/components/rich-markdown mobile/src/components/MobileRichMarkdownEditor.web.tsx config/scripts/build-mobile-web-app-bundle.mjsRepository: stablyai/orca
Length of output: 5311
Preserve focus indication on rich-markdown task checkboxes.
When an editable Markdown document contains a task list, markdown-list-render.ts emits an enabled input[type="checkbox"]. The checkbox uses appearance: none without a focus style, while the shipped parity rule removes its outline. Keyboard users can therefore lose visible focus.
🐛 Suggested fix
ul[data-type="taskList"] input[type="checkbox"]:disabled {
opacity: 0.65;
}
+ ul[data-type="taskList"] input[type="checkbox"]:focus-visible {
+ outline: 2px solid var(--primary);
+ outline-offset: 2px;
+ }| // Both builds: once a dependency requires it, esbuild resolves every importer to cjs. | ||
| { filter: /react-native-web[\\/]dist[\\/](cjs[\\/])?exports[\\/]StyleSheet[\\/]index\.js$/ }, | ||
| async (args) => { | ||
| const source = await readFile(args.path, 'utf8') | ||
| if (!source.includes(RNW_HAIRLINE_ASSIGNMENT)) { | ||
| throw new Error(`${HAIRLINE_PLUGIN_NAME}: ${args.path} no longer assigns hairlineWidth`) | ||
| } | ||
| return { | ||
| contents: source.replace(RNW_HAIRLINE_ASSIGNMENT, DEVICE_PIXEL_HAIRLINE_ASSIGNMENT), | ||
| loader: 'js' | ||
| } | ||
| } | ||
| ) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '225,263p' config/scripts/build-mobile-web-app-bundle.mjs
sed -n '115,157p' config/scripts/mobile-web-app-native-parity-render.test.mjsRepository: stablyai/orca
Length of output: 3679
🏁 Script executed:
set -eu
printf '%s\n' '--- changed files and hunk ---'
git diff --stat 90f0c5c8aeffa5572e2d9f412c4d6f9cc9bc6d5b e2b9233cff730de378d30f403b2c09fe6129d185
git diff --unified=35 90f0c5c8aeffa5572e2d9f412c4d6f9cc9bc6d5b e2b9233cff730de378d30f403b2c09fe6129d185 -- config/scripts/build-mobile-web-app-bundle.mjs config/scripts/mobile-web-app-native-parity-render.test.mjs
printf '%s\n' '--- hairline references ---'
rg -n -S -g '!node_modules' -g '!dist' -g '!build' 'hairline-device-pixel|hairlineWidth|data-testid="hairline"|separator-box|devicePixelRatio|pixelRatio|native.?width|nativeWidth' .
printf '%s\n' '--- relevant build-options region ---'
sed -n '250,370p' config/scripts/build-mobile-web-app-bundle.mjs
printf '%s\n' '--- parity test setup and fixtures ---'
sed -n '1,180p' config/scripts/mobile-web-app-native-parity-render.test.mjsRepository: stablyai/orca
Length of output: 45544
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions
Length of output: 39315
🏁 Script executed:
set -eu
printf '%s\n' '--- changed files and hunk ---'
git diff --stat 90f0c5c8aeffa5572e2d9f412c4d6f9cc9bc6d5b e2b9233cff730de378d30f403b2c09fe6129d185
git diff --unified=35 90f0c5c8aeffa5572e2d9f412c4d6f9cc9bc6d5b e2b9233cff730de3782e2d9f412c4d6f9cc9bc6d5b -- config/scripts/build-mobile-web-app-bundle.mjs config/scripts/mobile-web-app-native-parity-render.test.mjs
printf '%s\n' '--- hairline references ---'
rg -n -S -g '!node_modules' -g '!dist' -g '!build' 'hairline-device-pixel|hairlineWidth|data-testid="hairline"|separator-box|devicePixelRatio|pixelRatio|native.?width|nativeWidth' .
printf '%s\n' '--- relevant build-options region ---'
sed -n '250,370p' config/scripts/build-mobile-web-app-bundle.mjs
printf '%s\n' '--- parity test setup and fixtures ---'
sed -n '1,180p' config/scripts/mobile-web-app-native-parity-render.test.mjsRepository: stablyai/orca
Length of output: 510
🏁 Script executed:
set -eu
git diff --stat 90f0c5c8aeffa5572e2d9f412c4d6f9cc9bc6d5b e2b9233cff730de378d30f403b2c09fe6129d185
git diff --unified=25 90f0c5c8aeffa5572e2d9f412c4d6f9cc9bc6d5b e2b9233cff730de378d30f403b2c09fe6129d185 -- config/scripts/build-mobile-web-app-bundle.mjs config/scripts/mobile-web-app-native-parity-render.test.mjs
printf '%s\n' '--- references ---'
rg -n -S -g '!node_modules' -g '!dist' -g '!build' 'hairline-device-pixel|hairlineWidth|data-testid="hairline"|separator-box|devicePixelRatio|pixelRatio|native.?width|nativeWidth' .
printf '%s\n' '--- build options ---'
sed -n '250,370p' config/scripts/build-mobile-web-app-bundle.mjs
printf '%s\n' '--- parity test ---'
sed -n '1,180p' config/scripts/mobile-web-app-native-parity-render.test.mjsRepository: stablyai/orca
Length of output: 45544
🏁 Script executed:
set -eu
printf '%s\n' '--- build hairline declarations ---'
sed -n '80,108p' config/scripts/build-mobile-web-app-bundle.mjs
sed -n '188,252p' config/scripts/build-mobile-web-app-bundle.mjs
printf '%s\n' '--- build tests around hairline ---'
sed -n '320,365p' config/scripts/build-mobile-web-app-bundle.test.mjs
printf '%s\n' '--- native substitute exception ---'
sed -n '1,115p' mobile/src/test-support/rpc-recording/screen-native-substitutes.ts
sed -n '60,90p' mobile/src/test-support/rpc-recording/screen-native-substitutes.test.ts
printf '%s\n' '--- focused production consumers ---'
sed -n '1,35p' mobile/src/tasks/mobile-tasks-diff-comment-styles.ts
sed -n '132,154p' mobile/src/tasks/mobile-tasks-diff-comment-styles.ts
sed -n '1,22p' mobile/src/tasks/mobile-tasks-detail-styles.tsRepository: stablyai/orca
Length of output: 14500
Keep StyleSheet.hairlineWidth at one device pixel.
At devicePixelRatio 4, the replacement computes Math.round(0.4 * 4) = 2, so the width is 0.5 CSS pixels and paints two device pixels. DevicePixelRatio 5 also produces a width above two device pixels. This reaches production styles that use StyleSheet.hairlineWidth and violates the documented one-device-pixel contract.
Suggested fix
-// React Native's device-pixel count (roundToNearestPixel(0.4), else one) over the ratio, rounded up
+// One native device pixel over the ratio, rounded up
// to the 1/64 CSS px both engines lay out in. WebKit stores an exact 1/3 as 21/64, under one device
// pixel at 3, and paints nothing; 22/64 is the smallest step that paints. Any width above 1/ratio
// can still straddle two rows at some offsets; the smallest such step does so least.
const DEVICE_PIXEL_HAIRLINE_ASSIGNMENT =
'StyleSheet.hairlineWidth = (function (ratio) {' +
- ' return Math.ceil((64 * (Math.round(0.4 * ratio) || 1)) / ratio) / 64; })' +
+ ' return Math.ceil(64 / ratio) / 64; })' +
"(typeof window !== 'undefined' && window.devicePixelRatio > 0 ? window.devicePixelRatio : 1);"![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — page-side native-parity polish for the Route A mobile-web bundle; native pixels and the native bundle are untouched.
- Focus ring parity — a new
MOBILE_WEB_APP_NATIVE_PARITY_STYLE(:where(input:focus,textarea:focus){outline:none}) is injected into the built document head after the root reset. Verified againstreact-native-web@0.21.2:Switchdraws its own focus state (handleFocusStatesets the thumbboxShadow), and the app renders no RNWCheckbox, so no input currently loses its only focus affordance. - Hairline device-pixel shim —
hairlineDevicePixelPluginmatches RNW's wholeStyleSheet.hairlineWidth = 1;assignment in both its ESM and CJSStyleSheet/index.jsand replaces it with RN's device-pixel count over the ratio, rounded up to the 1/64 CSS-px step. I confirmed both upstream builds carry the exact string the throw guards on, and confirmed via a local esbuild transform that the minified replacement still matches the build test's\.hairlineWidth=[^;]{0,120}window (so the assertion genuinely fails if the shim is dropped). The formula tracks RN'sroundToNearestPixel(0.4) || 1 device pxfor ratios 0.5–5. - Registry + tests — the shim is registered in
MOBILE_WEB_APP_SHIMSand the new render check measures chromium (--force-device-scale-factor=3) and webkit at scale 3 for the hairline/separator pixels and the computed input outline.
The one thing worth keeping in mind (not blocking): input:focus covers every input type, not just text fields, so it would also strip the ring from a checkbox/radio that relied on the UA outline. Nothing shipped today does — RNW Switch self-draws and no page <input type=checkbox> outside the editor document is reachable — so this is latent breadth rather than a regression.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Why
The page-vs-native visual audit found two discrepancies where the browser's defaults differ from what the native app paints. Native is the reference; both fixes are page-side only, in the page bundle builder, and leave native pixels untouched.
StyleSheet.hairlineWidthis pinned to 1 CSS px by react-native-web, which is 3 device px on a phone, while native draws one device pixel.What
:where(input:focus,textarea:focus){outline:none}. Inputs and textareas only, so a button reached by a hardware keyboard keeps the browser's ring; RNW Switch's hidden checkbox draws its own focus state.onLoadshim that replaces react-native-web'sStyleSheet.hairlineWidth = 1;assignment (matched whole, so an upgrade that moves it fails the build; applied to the ESM and CommonJS builds) with React Native's own device-pixel count over the ratio, rounded up to the 1/64 CSS px both engines lay out in. Why not1/ratioexactly: WebKit stores 1/3 as 21/64 and paints nothing, so the iOS shell would lose every hairline; 22/64 is the smallest step that paints, and it also keepsheight: hairlineWidthseparators at one device pixel where a wider value straddled two rows half the time. A runtime assignment cannot do this: consumers read the value inside module-scopeStyleSheet.create(162 uses), and the bundle banner runs before react-native-web's StyleSheet exists. The shim registers in the existingMOBILE_WEB_APP_SHIMScontract like the lucide shim.Skipped after measurement (not simple, or not a discrepancy): row heights differ by Android font padding (page text boxes 54 px vs native 61) with no page-side rule that matches native within 1–2 px; the audit's red "Waiting for desktop…" flashes did not reproduce on a healthy link and trace to a separate socket stall that native's shared screens show the same way.
Proof
'auto'vs'none'); rendered parity test 3 device px vs 1; WebKit casedevicePixels: +0with the exact1/ratiovalue; separator caseexpected 2 to be 1with a 0.5 px value.#e59700at x=33–38); files header border 3 → 1 device px at y=330 = native; filter-sheet separator 1 px.