fix(mobile): keep latest messages above composer - #4981
Conversation
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
|
🤖 Reviewed at The number that proves it. I instrumented the widget tree and read the geometry directly. Post-fix, the bottom of the newest message group sits at exactly the top of the composer dock: gap Your test is a real recognizer, not decoration. Control at head passes. Production-only revert with the new test retained fails, expected Two independent seats ran the full mobile suite. 1,243 passed, 0 failed, at both this head and the merge-base. On the two red checks: they are ours, not yours. Three coverage gaps, all non-blocking. A mutation sweep found three changes to the production code that your new test does not notice. None is a defect in this PR. Each is a place where a future edit could regress silently:
Tightening the bound on the existing assertion would pick up the first one cheaply. The other two are new cases, and are fine as follow-ups. |
🤖 Dismissing my own approval. A third review seat measured a regression that my arms and one other seat both missed: the realignment path double-applies the composer inset at short content, and the new assertion is one-sided so it cannot see an overshoot. I reproduced it independently. Full finding in a follow-up comment. My approval should not have stood.
|
🤖 Correction: I dismissed my own approval. There is a real regression here, and my earlier comment was wrong. Measured at My earlier comment said the fix was correct because the newest row sits exactly at the composer top, gap The defectThe The list already passes the inset as Direction of failure is overshoot, not the original bug. The newest row floats 62px above the composer, leaving a dead band. It does not go back behind the composer. Why long content hides it. When the content overflows the viewport the scroll position clamps at the maximum scroll extent, which absorbs the extra anchor offset. Short content has no clamp headroom, so the doubling becomes visible. My own sweep, 400x600 viewport, 62px composer dock, gap = composer top minus newest row bottom:
Why the new test cannot catch itThe new assertion is one-sided: it requires the newest row bottom to be at most the composer top. An overshoot makes that value smaller, so it passes. The test structurally cannot fail in the overshoot direction. The remedy is one line, and it was mutation-tested rather than assertedDrop the
That third row is the control that matters: the Second-order effect, same root causeThe at-boundary predicate is unsatisfiable at short content, because the leading edge sits at twice the boundary it compares against. So the realignment early-return never fires in a short channel and it re-jumps on every metrics change. The user-visible fallout is contained: the jump-to-latest button does not spuriously appear. This is wasted work and a dead predicate, not a stuck control. Two smaller notes
Where this leaves the PRThe core insight is right and the Also worth adding a short-content case to the new test, since the current one passes in both directions. |
Category: fix
User Impact: Mobile users who jump to Latest now see the newest message fully above the composer instead of partially hidden behind it.
Problem: The channel message list treated the raw viewport bottom as the latest boundary even though the composer occupies part of that viewport. Latest jumps and follow-mode corrections could therefore place the newest message underneath the composer.
Solution: Derive the latest alignment from the measured composer inset and use that same boundary for scrolling, follow detection, and layout correction.
File changes
mobile/lib/features/channels/channel_detail_page/message_list.dart
Aligns Latest navigation and follow-mode correction with the visible bottom edge above the composer, and evaluates boundary state against the same geometry.
mobile/test/features/channels/channel_detail_page_test.dart
Adds a regression assertion that the newest live message clears the composer and that the Latest control disappears after navigation.
Reproduction steps
Tested fix
The newest message remains fully visible above the composer after jumping to Latest.
Validation
flutter analyze— no issuesflutter test— 1,243 passed