test(mobile): allow a subpixel overlap when the macro builder field scrolls into view - #2139
Conversation
…crolls into view On a display whose scroll positions are whole pixels (the Linux e2e runner), scrollIntoView leaves the field 0.28 px over the footer, so the spec failed although the field is in view. Without the fix the field is 197 px under the footer, so a 1 px tolerance still catches that.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe macro-builder keyboard test now considers the field covered only when its bottom extends more than one pixel past the footer’s top. ChangesKeyboard Coverage Check
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~5 minutes Change: Other Merge Risk: ⚪ Minimal · up to The test retains its check that the keyboard scrolls the field clear of the footer while tolerating the intended one-pixel alignment difference. No actionable merge risk remains. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. A rabbit checks the footer’s line Comment |
The new
phone-keyboardspec from #2133 failed on the Linux e2e runner (full run and alone): afterkeyboardDidShow, the macro builder's script field ends at 378.28 px and the footer starts at 378 px. scrollIntoView did its job; the scroll position is whole pixels at 1x, so 0.28 px of the field's bottom stays under the footer. On the Mac where the spec was written (2x), it lines up exactly.The check now allows 1 px. Before the scroll the field is 197 px under the footer, so the spec still fails without the fix. Test-only;
phone-keyboard.test.tspasses 3/3 here.Note
Allow 1px subpixel overlap in phone keyboard footer-coverage e2e test
Relaxes the footer-coverage predicate in phone-keyboard.test.ts so the macro builder field counts as covered only when its bottom exceeds the footer top by more than one pixel. A comment documents that the tolerance handles fractional scroll positioning. Test-only change; no product behavior is affected.
Macroscope summarized b3296dc.
Summary by CodeRabbit