Prevent whole-window scroll bounce - #41
Conversation
Signed-off-by: klopez4212 <klopez4212@gmail.com>
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4e28cdb40
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| font-synthesis: none; | ||
| -webkit-font-smoothing: antialiased; | ||
| /* Keep wheel/trackpad gestures from rubber-banding the whole app. */ | ||
| overscroll-behavior: none; |
There was a problem hiding this comment.
Add the required Signed-off-by trailer
The reviewed commit contains no Signed-off-by trailer, so it violates the repository’s DCO requirement for every PR commit and cannot pass the hosted DCO gate. Recreate the commit with git commit --signoff using the verified effective Git identity.
AGENTS.md reference: AGENTS.md:L51-L54
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: no blocking findings
Reviewed head a4e28cdb4015d9ee8404dbb4c83d98486c628bb8 against base 834433c580d8e36a8ae9cd9da5695d1166604b94.
- Small, correctly owned change:
globals.css:64–73sets the outer viewport’s overscroll policy in the host’s existing base layer. The app’s full-height shell and nested history/document/panel scrolling retain their existing ownership and overflow rules; no global wheel handler or page lifecycle state is added. - Coverage matches its stated limits:
layout.spec.mjs:74–114checks the computed root policy, actual history scrolling, and shell/window stability over empty Projects in both axes. Headless wheel input does not reproduce macOS trackpad rubber-banding, as the test explicitly notes. The author reports desktop verification; this automated review did not independently exercise native behavior. - Exact-head hosted checks passed: CI run 34751530959, including JavaScript, Rust/tool integration, measurements, all Chromium/WebKit functional shards and
CI required. DCO also passed. The existing bot comment claiming a missing sign-off is contradicted by this commit’s actualSigned-off-bytrailer; no attribution rewrite is needed for that claim.
Source/diff review ran only on strict-verified BLKD2G9MWNRWY.local using pinned Git objects. The unrelated checkout stayed clean at dbad6526346e7350c528280ccc5cf6c68e740607; exact base/head git diff --check passed. No checkout, installation, build, test, import or execution of PR code, and no CI rerun. This is a source-review clearance, not GitHub approval, merge authorization or an independent native acceptance claim.
Signed-off-by: klopez4212 <klopez4212@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: no blocking findings in this change
Reviewed head 95393ade2af7deb10004d5f9db3449483ff797a1 against base 834433c580d8e36a8ae9cd9da5695d1166604b94, including the incremental diff from the previously cleared head a4e28cdb.
- The only new delta is test input:
typeahead.spec.mjs:532–560uses macOS’s caret shortcut instead ofEnd, retaining the exact draft and signed-publication assertions. This matches the existing platform-aware convention incompletion-layout.spec.mjs:86–93; it does not change composer behavior or relax the expected result. - The overscroll fix remains appropriately scoped: the root CSS policy, full-height shell and nested scroll ownership are unchanged since the previous review. The layout regression checks computed viewport policy, actual history scrolling and stationary empty-page bounds; existing coverage checks independent companion-panel scrolling. No event interception, persistence or protocol change is introduced.
- CI is not fully green: exact-head run 34752445324 passed JavaScript, Rust/tool integration, measurements, both Chromium shards and WebKit shard 1, including both changed browser cases. WebKit shard 2 failed the unchanged
sidebar-unread.spec.mjs:125query-scope assertion (1 failed, 69 passed). The test observed a post-baseline channel query outside the two permitted focused rows. Its cause is not established; neither that test nor its app implementation changed in this incremental update. I am reporting this as a separate unresolved integration gate, not inventing a caret/overscroll defect.CI requiredremains failed; DCO passed.
Carl and Princess Donut reviewed pinned source on strict-verified BLKD2G9MWNRWY.local; Carl verified and integrated the independent lane. Exact-base/head git diff --check passed. The unrelated checkout stayed clean at dbad6526346e7350c528280ccc5cf6c68e740607. No checkout, installation, build, test, import or execution of PR code, and no CI rerun. Headless input does not establish native macOS trackpad rubber-banding behavior; desktop verification remains author-reported. This is source-review clearance, not GitHub approval, a green integration gate or merge authorization.
|
Carl, an automated reviewer, commenting via Wes’s GitHub account. CI diagnosis update at
|
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: no blocking findings in this change
Reviewed head 655c2ffbe77d2f25de1b273baa9b57b16e87bf9d against base 61274d40e0171cae4ef519021370345e18b884d3, including merge integration from the previously reviewed parent 95393ade.
- The merge preserves the reviewed contract. All three PR files are byte-for-byte identical to that parent. The root overscroll policy stays in the host base layer; full-height shell, document pages, message history, and companion panels retain their nested scroll ownership. Mainline attachment changes and browser-baseline fixes introduce no identified conflict with this policy. Shared layout fixture/settling helpers are unchanged. No event interception, persistence, protocol, or lifecycle change is introduced by this PR.
- Both changed browser cases pass in Chromium and WebKit. Run 34764139052 passes the overscroll/history regression and custom-catalog/typeahead case, retaining exact draft and signed-publication assertions. The previously failing sidebar-unread case now passes. CI checked synthetic merge
206a1b0ea85cf746a842dd793ad33e441e7e2a55; its tree is identical to this head (622fe35b725798b5ae7412f7c4213cc08ef0ce7e). Linux CI does not exercise the macOS-only caret branch or native trackpad rubber-banding; desktop verification remains author-reported. - CI remains a separate unresolved integration gate. WebKit shard 1 failed the unchanged
terminal-renderer.spec.mjs:293disabled-color assertion: Restart was expected to havergb(187, 187, 187), but the eventual enabled control hadrgb(10, 10, 10)(73 passed, 1 failed). The log shows intermediate disabled colors before the control re-enabled; it does not establish an overscroll defect. Terminal test/implementation and design-system paths are unchanged from the reviewed parent. JavaScript, Rust/tool integration, measurements, both Chromium shards, WebKit shard 2, and DCO passed;CI requiredfailed. No claim of full validation or merge readiness.
Carl and Princess Donut reviewed pinned source on strict-verified BLKD2G9MWNRWY.local; Carl verified and integrated the independent lane. Exact-base/head git diff --check passed. The unrelated checkout remained clean at dbad6526346e7350c528280ccc5cf6c68e740607. No checkout, installation, build, test, import or execution of PR code, and no CI rerun. This is source-review clearance, not GitHub approval, merge authorization, or independent native acceptance.
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
|
Carl, an automated contributor, commenting via Wes’s GitHub account. Updated this branch to main At the exact clean merge head on Apple Silicon macOS: all 1,117 Vitest tests, 3 cache-integration tests, and 64 browser cases across complete layout, typeahead, terminal-renderer and image-scroll files passed in Chromium/WebKit. Pre-commit and pre-push hooks passed without bypass. Independent bounded merge review found no integration defect. Full scan/native/measurement gates were not repeated for this integration. Headless wheel evidence is not native trackpad acceptance. Required hosted checks/DCO and reviewer approval remain gates; this update is not approval or authorization to merge. Wes is watching CI; no polling, manual rerun or merge performed. |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: no blocking findings
Reviewed head d007e99b7336536532c421c6b94591499b984157 against base d8fa1a2dcaa8ea87cbbd7cdc1652b1e277dd89a4, including the merge from previously reviewed 655c2ffb.
- The overscroll contract is preserved. The host root policy, its layout regression, and the app’s shell/panel/scroll owners are unchanged from the reviewed parent. The full-height shell still contains document-page scrolling, message history and independent panels. The regression retains computed root policy, real history movement, and stable empty-Projects bounds in both axes. Existing layout coverage retains document/Settings scrolling and panel independence. No input interceptor, persistence, protocol or new lifecycle owner is introduced.
- Main’s typeahead fixes coexist with the caret correction. The increment adopts the isolated Vite fixture wrapper and waits for the recovery option before keyboard selection. The platform-specific caret shortcut, exact resulting draft, one-publication count, signed emoji tag and message-content assertions remain intact. No test skip, looser expectation or timeout increase is added by this PR.
- Current-head hosted CI is green. Run 34771875795 passed JavaScript, Rust/tool integration, measurements, all four Chromium/WebKit journey shards and
CI required; DCO, Semgrep and zizmor also passed. This clears the hosted integration gate that remained open in the previous review; it does not establish universal flake freedom. The clean CI synthetic merge138e58e01ffc7ffcc869992d26e93d716a385151was fetched and verified to have the same tree as this head:85bb6a787379505c1dfbbd061db3d71c2d7a1bd4.
Princess Donut independently reviewed the typeahead merge lane; Carl verified its diff/lifecycle claims and integrated the result. All delegated work is complete.
Source/diff and static CI-artifact inspection ran only on strict-verified Studio BLKD2G9MWNRWY.local, addressing pinned objects in /Users/wesb/.buzz/REPOS/buzz-app. Exact base/head git diff --check and merge re-resolution inspection were clean. The unrelated checkout remained clean at dbad6526346e7350c528280ccc5cf6c68e740607. No checkout, installation, build, test, import, runtime probe or execution of PR code, and no CI rerun/polling.
Linux/headless CI does not exercise the macOS-specific caret branch or native trackpad rubber-banding. Desktop verification remains author-reported; native acceptance is not independently established here. This is COMMENTED, not GitHub approval or merge authorization.
* origin/main: fix(messages): prevent native WebKit prepend blanking (#49) feat: add shared browser and standard desktop notifications (#38) Prevent whole-window scroll bounce (#41) test: replace timing races with explicit lifecycle barriers (#45) test(browser): isolate fixture optimizer caches (#46) test(browser): settle startup catch-up before request baselines (#43) feat(messages): show message-carried blurhash before decoded originals (#40) Add a shared full-page surface (#29) fix(messages): prevent image-driven timeline scroll snap-back (#31) Restore grouped channel membership activity (#30) feat: add broker-backed Agent Activity panel (#36) Signed-off-by: Clay Delk <clay.delk@gmail.com> # Conflicts: # src/bundled/channels/ChannelsPage.tsx # src/features/messages/ChannelTimeline.tsx # src/features/messages/MessageRow.tsx # src/features/relay/contracts.ts # src/features/relay/fold.ts
Disable root overscroll so empty sections and scroll boundaries cannot bounce the whole window, while panels scroll normally. Add browser coverage and correct an existing test’s macOS caret shortcut.
Validation: desktop confirmed; unit, integration, build, design, and Clippy checks passed. Browser suite: 291/292 initially; corrected test passes 3× per engine. Native tests: 8/9; login-shell probe blocked by the machine’s security acknowledgement prompt.