Repository navigation
fix(scroll): take edge hints from a same-rect scrollable ancestor - #3784
Conversation
On Android Settings, the RecyclerView nested in main_content_scrollable_container offers ACTION_SCROLL_BACKWARD even at the top, so `scroll top` kept swiping a list that could not move and failed with scroll_edge_no_progress. When a scroller has the same rect as its nearest scrollable ancestor, the edge verdict now reads the ancestor's hints. The snapshot's own per-node hints are unchanged. Closes callstack#3737 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Container selection reads each node's own hints again, as on main; only the final canScroll verdict consults the same-rect scrollable ancestor. The ancestor walk tracks visited indexes so a cyclic parentIndex chain in a malformed tree cannot hang edge analysis. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Scoping the ancestor's hints to the verdict alone let selection pick the nested RecyclerView at the top; the loop then scoped its recapture to that subtree, lost the ancestor, and fell back to the stale hint (scroll_edge_no_progress on a second `scroll top`). Scroll edge selection now stands a same-rect scrollable ancestor in for the scroller nested in it, so the chosen container, its scope and its verdict all come from the ancestor. The visited-index guard stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
findNearestScrollableAncestor followed parentIndex without a visited set, so a targeted scroll on a malformed tree whose parent chain loops with no scrollable ancestor could hang edge analysis. It now stops on a revisited index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The PR is ready at 3818a90. Not blocking: the same-rect test in selection.ts ignores scroll axis, so a horizontal scroller with the same rect as a nested vertical list could replace it (I found no real screen that does this; consider requiring the ancestor to scroll on the requested axis, or name the heuristic in the doc comment), and the new same-rect ancestor test file could be folded into the existing selection-level test file, or The three cycle and selection threads from cubic-dev-ai are fixed at this head, so please resolve them: visited set in the ancestor walk, selection and verdict now agree, and visited set in the nearest-ancestor walk. CI is green: 12 checks, none failing. There are no conflicts. Nothing else must happen before merge. On evidence: the live Android Settings table in the PR body was run at 273e341, and 3818a90 only adds cycle guards, so I did not re-run it. Only the ADB path was exercised live, and I did not inspect any Appium or iOS tree for same-rect scroller pairs. I did not run the tests locally. |
thymikee
added
the
ready-for-human
…ubtree fixture The doc comment now states that the same-rect check is geometric and ignores scroll axis. subtreeFor moves to the shared scroll-edge-state fixtures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Geometry carries no scroll axis, so a horizontal pager framing the same pixels as a vertical list on its page could stand in for that list and hide its edge hints. The snapshot emits vertical edge hints only for vertical scrollers, so an ancestor now stands in only when it reports one. subtreeFor matches scopes and reindexes nodes the way an Android scoped snapshot does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment * origin/main: build(package): stop shipping Swift test sources (#3785) fix(scroll): take edge hints from a same-rect scrollable ancestor (#3784) docs(agents): write agent docs in ASD-STE100 and remove misleading docs (#3723) fix(daemon): retain cleanup locks and preserve healed snapshots (#3760) # Conflicts: # packages/daemon/src/sessions/__tests__/replay-repair/session-replay-repair-record-exclusion.test.ts
…refactor/daemon-runtime-no-process-globals * origin/refactor/host-environment: build(package): stop shipping Swift test sources (#3785) fix(scroll): take edge hints from a same-rect scrollable ancestor (#3784) docs(agents): write agent docs in ASD-STE100 and remove misleading docs (#3723) fix(daemon): retain cleanup locks and preserve healed snapshots (#3760) test: say which package still reads PATH from the process in the recording scenario
2 participants
Summary
scroll topon the Android Settings home screen reached the top, then failed withscroll_edge_no_progress. The nestedrecycler_viewreportshiddenContentAboveeven at the top. The outermain_content_scrollable_container, with the identical rect, reports the real position.As agreed in #3737 (option 1), scroll edge selection now stands a same-rect scrollable ancestor in for the scroller nested in it, so the chosen container, its scope and its verdict all come from the ancestor. Selecting the ancestor (not only reading its hints) keeps the nested list inside the scoped recapture
runScrollEdgePassesmakes. An ancestor stands in only when it reports a vertical edge hint, so a same-rect horizontal pager never replaces the vertical list on its page. Both ancestor walks guard against cyclicparentIndexchains. Snapshot hints are unchanged. Three files:scroll-edge-state/selection.ts, a regression fixture trimmed from the live Settings trees, and its tests.Closes #3737
Validation
pnpm check:affected --run --head f4c2c2406passes (exactCommit=true): format, lint, typecheck, layering, fallow, build, vitest-related.android-helper0.21.29-dev, same steps as the issue,mainat 92db564 vs this branch (rerun at f4c2c24):mainscroll downscroll topscroll_edge_no_progressscroll bottomscroll topscroll_edge_no_progressscroll topagainAlso live at f4c2c24:
scroll topon a freshly opened screen reports Already at top.🤖 Generated with Claude Code