fix(mobile): keep the unread jump after a deep boundary give-up - #4641
Draft
brow wants to merge 1 commit into
Draft
fix(mobile): keep the unread jump after a deep boundary give-up#4641brow wants to merge 1 commit into
brow wants to merge 1 commit into
Conversation
The unread-boundary effect bailed out entirely once it gave up paging toward the target, so a channel whose oldest unread sat further back than the four-fetch cap never offered the jump control again for that visit, even after the user scrolled the target into view themselves. Scope the give-up flag to fetching, which is what it is for, and let candidate resolution keep running. Direction of the bug was fail-closed (a missing control, never a wrong jump), so this only restores a control that PR #4239's predecessor head offered. Co-authored-by: Tom Brow <tomb@block.xyz> Signed-off-by: Tom Brow <tomb@block.xyz>
Contributor
Author
|
@codex review |
Contributor
Author
|
@builderbot review |
|
To use Codex here, create a Codex account and connect to github. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
_MessageList's unread-boundary effect used its give-up flag as a whole-effect kill switch: once the automatic fetch loop stopped paging toward the oldest unread message, the effect returned early on every later run, so the candidate resolution below it never ran again for that channel visit.This scopes the flag to what it is named for. It still stops further fetching, but resolution keeps running, so a target that shows up later is still offered.
Why?
A channel whose oldest unread message sits further back than the four-fetch cap can reach would show no jump-to-unread chevron at all, and would keep showing none even after the user scrolled that message into view themselves. The control was simply gone for the rest of the visit.
This is a regression against the behavior in #4239's predecessor head, which did offer a working chevron on the same fixture, so this restores rather than adds. The direction is fail-closed: the old code could only omit the control, never send you to the wrong message, which is why it was not worth holding #4239 for.
How is it tested?
Build and run.
Added tests:
ChannelDetailPage still offers the unread jump once the user pages the target inThe new test drives pagination through real scroll gestures and asserts on where the chevron actually lands, not merely that it rendered. Presence alone is too weak here: in a virtualized list a control can be visible while the tap does nothing. It fails against
maintoday and passes with this change.Reviewer guidance
Two shapes were measured before choosing this one. The alternative was deleting the flag outright at all five of its sites. Both give the same passing suite, so the tests could not distinguish them; removing the flag entirely also deletes the retry suppression it was added for, which is wider than the problem being fixed. That choice was made deliberately, not by the test suite.
Out of scope, filed separately as #4643: a mixed forced-unread plus ordinary-unread channel can still offer the newer message as "oldest unread". That is pre-existing at every head involved here, is a silent wrong jump rather than a missing control, and needs its own fix and its own tests.