Let a jump stand where more than one scroll is on the screen
Every jump on the Flutter half landed and was hauled back to the bottom in the same frame. Pressing a reply chip scrolled to the line and returned to the present before it had been painted, which reads exactly like a click that did nothing — and the click, the mark, the context and the scroll were all working. The trace says so: `ensureVisible` moved the list from 14522 to 10657 and `end-ward!` put it back at 14546. The rule it broke is the one written above it: being taken to a line and being taken to the end are the same gesture as far as `jump-tick` is concerned, so the end stands down when a jump is pending. It read `jump-context` to know that, and `jump-context` is one atom for the whole screen while `:scroll` is two widgets — the message list and the people panel. Their post-frame callbacks interleave, the panel's `here-ward!` serves the jump and clears the context, and the list's `end-ward!` then reads that nil as "no jump pending". So standing down no longer asks what is pending. `here-ward!` counts the jumps it serves, `end-ward!` reads the count when it schedules and again when it runs, and a count that moved in between means this frame belonged to a jump whatever the context says by then. Ordering cannot fool it, and neither can a third scroll. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2abafda parent: 6908605 modified
flutter/src/frq/hiccup.cljd +41 -11 | @@ -239,6 +239,20 @@ | ||
| 239 | 239 | ;; around it after the frame. |
| 240 | 240 | (defonce ^:private jump-context (atom nil)) |
| 241 | 241 | |
| 242 | +;; How many jumps have been served. `here-ward!` bumps it as it scrolls, and | |
| 243 | +;; `end-ward!` reads it when it schedules and again when it runs: a jump that | |
| 244 | +;; landed in between is one this frame belongs to, whatever `jump-context` | |
| 245 | +;; says by then. | |
| 246 | +;; | |
| 247 | +;; The pending flag alone was not enough, and the trace says why. There is | |
| 248 | +;; more than one `:scroll` on the chat screen — the message list and the | |
| 249 | +;; people panel — so `end-ward!` and `here-ward!` are each called once per | |
| 250 | +;; scroll per frame, and the callbacks interleave: the first scroll's | |
| 251 | +;; here-ward serves the jump and clears the context, and the message list's | |
| 252 | +;; end-ward then reads that nil as "no jump pending" and hauls the view back | |
| 253 | +;; to the bottom. The scroll was right for one frame and gone by the next. | |
| 254 | +(defonce ^:private jumps-served (atom 0)) | |
| 255 | + | |
| 242 | 256 | (defn- end-ward! |
| 243 | 257 | "Put `k` at the end after this frame, when it should be. |
| 244 | 258 | |
| @@ -277,16 +291,26 @@ | ||
| 277 | 291 | (- (.-maxScrollExtent ^m/ScrollPosition pos) 24.0)))] |
| 278 | 292 | (when (or (not= mark token) at-end) |
| 279 | 293 | (swap! scroll-marks assoc k token) |
| 280 | - (.addPostFrameCallback | |
| 281 | - (.-instance m/WidgetsBinding) | |
| 282 | - (fn [_] | |
| 283 | - ;; Not when a jump is pending: being taken to a line and being taken | |
| 284 | - ;; to the end are the same gesture as far as `jump-tick` is | |
| 285 | - ;; concerned — the screen renames the viewport either way — and the | |
| 286 | - ;; end is the wrong one of the two. By now the marked row has been | |
| 287 | - ;; built, so this knows which gesture it was. | |
| 288 | - (when (and (nil? @jump-context) (.-hasClients ctrl)) | |
| 289 | - (.jumpTo ctrl (.-maxScrollExtent (.-position ctrl))))))))) | |
| 294 | + (let [served @jumps-served] | |
| 295 | + (.addPostFrameCallback | |
| 296 | + (.-instance m/WidgetsBinding) | |
| 297 | + (fn [_] | |
| 298 | + ;; Not when a jump is pending, and not when one was served after | |
| 299 | + ;; this callback was scheduled: being taken to a line and being | |
| 300 | + ;; taken to the end are the same gesture as far as `jump-tick` is | |
| 301 | + ;; concerned, and the end is the wrong one of the two. | |
| 302 | + ;; | |
| 303 | + ;; The counter is what makes that ordering-proof. The pending flag | |
| 304 | + ;; alone read right and was not: there is more than one `:scroll` | |
| 305 | + ;; on the chat screen, so these callbacks interleave with | |
| 306 | + ;; `here-ward!`'s, and the first scroll's here-ward serves the jump | |
| 307 | + ;; and clears the flag before the message list's end-ward ever | |
| 308 | + ;; looks at it. Every jump landed and was hauled back to the bottom | |
| 309 | + ;; in the same frame. | |
| 310 | + (when (and (nil? @jump-context) | |
| 311 | + (= served @jumps-served) | |
| 312 | + (.-hasClients ctrl)) | |
| 313 | + (.jumpTo ctrl (.-maxScrollExtent (.-position ctrl)))))))))) | |
| 290 | 314 | |
| 291 | 315 | (defonce ^:private scroll-watch |
| 292 | 316 | ;; Per `:scroll-key`: the controller currently being listened to, the |
| @@ -349,6 +373,11 @@ | ||
| 349 | 373 | ;; that may be gone. |
| 350 | 374 | (reset! jump-context nil) |
| 351 | 375 | (when (.-mounted c) |
| 376 | + ;; Counted before the scroll rather than after it: an `end-ward!` | |
| 377 | + ;; callback queued behind this one has to see that this frame was | |
| 378 | + ;; spent on a jump, and `ensureVisible` answers a Future — by the | |
| 379 | + ;; time that completes, the callback it is racing has run. | |
| 380 | + (swap! jumps-served inc) | |
| 352 | 381 | (m/Scrollable.ensureVisible c .alignment 0.3))) |
| 353 | 382 | ;; The callback is void and `ensureVisible` answers a Future: returned |
| 354 | 383 | ;; from the tail, it is a value out of a function that has none to give, |
| @@ -672,7 +701,8 @@ | ||
| 672 | 701 | ;; The row a jump is aiming at, saying so as it is built. Here and |
| 673 | 702 | ;; not in an arm of the `case` below, because the mark is a property |
| 674 | 703 | ;; any tag may carry — it happens to be a `:vbox` today. |
| 675 | - _ (when (:scroll-here p) (reset! jump-context ctx)) | |
| 704 | + _ (when (:scroll-here p) | |
| 705 | + (reset! jump-context ctx)) | |
| 676 | 706 | kids (children (body node)) |
| 677 | 707 | ;; One column builder for every container, so the rule about what |
| 678 | 708 | ;; fills is applied in one place: children that take what is left are |
| @@ -239,6 +239,20 @@ | |||
| 239 | ;; around it after the frame. | 239 | ;; around it after the frame. |
| 240 | (defonce ^:private jump-context (atom nil)) | 240 | (defonce ^:private jump-context (atom nil)) |
| 241 | 241 | ||
| 242 | +;; How many jumps have been served. `here-ward!` bumps it as it scrolls, and | ||
| 243 | +;; `end-ward!` reads it when it schedules and again when it runs: a jump that | ||
| 244 | +;; landed in between is one this frame belongs to, whatever `jump-context` | ||
| 245 | +;; says by then. | ||
| 246 | +;; | ||
| 247 | +;; The pending flag alone was not enough, and the trace says why. There is | ||
| 248 | +;; more than one `:scroll` on the chat screen — the message list and the | ||
| 249 | +;; people panel — so `end-ward!` and `here-ward!` are each called once per | ||
| 250 | +;; scroll per frame, and the callbacks interleave: the first scroll's | ||
| 251 | +;; here-ward serves the jump and clears the context, and the message list's | ||
| 252 | +;; end-ward then reads that nil as "no jump pending" and hauls the view back | ||
| 253 | +;; to the bottom. The scroll was right for one frame and gone by the next. | ||
| 254 | +(defonce ^:private jumps-served (atom 0)) | ||
| 255 | + | ||
| 242 | (defn- end-ward! | 256 | (defn- end-ward! |
| 243 | "Put `k` at the end after this frame, when it should be. | 257 | "Put `k` at the end after this frame, when it should be. |
| 244 | 258 | ||
| @@ -277,16 +291,26 @@ | |||
| 277 | (- (.-maxScrollExtent ^m/ScrollPosition pos) 24.0)))] | 291 | (- (.-maxScrollExtent ^m/ScrollPosition pos) 24.0)))] |
| 278 | (when (or (not= mark token) at-end) | 292 | (when (or (not= mark token) at-end) |
| 279 | (swap! scroll-marks assoc k token) | 293 | (swap! scroll-marks assoc k token) |
| 280 | - (.addPostFrameCallback | 294 | + (let [served @jumps-served] |
| 281 | - (.-instance m/WidgetsBinding) | 295 | + (.addPostFrameCallback |
| 282 | - (fn [_] | 296 | + (.-instance m/WidgetsBinding) |
| 283 | - ;; Not when a jump is pending: being taken to a line and being taken | 297 | + (fn [_] |
| 284 | - ;; to the end are the same gesture as far as `jump-tick` is | 298 | + ;; Not when a jump is pending, and not when one was served after |
| 285 | - ;; concerned — the screen renames the viewport either way — and the | 299 | + ;; this callback was scheduled: being taken to a line and being |
| 286 | - ;; end is the wrong one of the two. By now the marked row has been | 300 | + ;; taken to the end are the same gesture as far as `jump-tick` is |
| 287 | - ;; built, so this knows which gesture it was. | 301 | + ;; concerned, and the end is the wrong one of the two. |
| 288 | - (when (and (nil? @jump-context) (.-hasClients ctrl)) | 302 | + ;; |
| 289 | - (.jumpTo ctrl (.-maxScrollExtent (.-position ctrl))))))))) | 303 | + ;; The counter is what makes that ordering-proof. The pending flag |
| 304 | + ;; alone read right and was not: there is more than one `:scroll` | ||
| 305 | + ;; on the chat screen, so these callbacks interleave with | ||
| 306 | + ;; `here-ward!`'s, and the first scroll's here-ward serves the jump | ||
| 307 | + ;; and clears the flag before the message list's end-ward ever | ||
| 308 | + ;; looks at it. Every jump landed and was hauled back to the bottom | ||
| 309 | + ;; in the same frame. | ||
| 310 | + (when (and (nil? @jump-context) | ||
| 311 | + (= served @jumps-served) | ||
| 312 | + (.-hasClients ctrl)) | ||
| 313 | + (.jumpTo ctrl (.-maxScrollExtent (.-position ctrl)))))))))) | ||
| 290 | 314 | ||
| 291 | (defonce ^:private scroll-watch | 315 | (defonce ^:private scroll-watch |
| 292 | ;; Per `:scroll-key`: the controller currently being listened to, the | 316 | ;; Per `:scroll-key`: the controller currently being listened to, the |
| @@ -349,6 +373,11 @@ | |||
| 349 | ;; that may be gone. | 373 | ;; that may be gone. |
| 350 | (reset! jump-context nil) | 374 | (reset! jump-context nil) |
| 351 | (when (.-mounted c) | 375 | (when (.-mounted c) |
| 376 | + ;; Counted before the scroll rather than after it: an `end-ward!` | ||
| 377 | + ;; callback queued behind this one has to see that this frame was | ||
| 378 | + ;; spent on a jump, and `ensureVisible` answers a Future — by the | ||
| 379 | + ;; time that completes, the callback it is racing has run. | ||
| 380 | + (swap! jumps-served inc) | ||
| 352 | (m/Scrollable.ensureVisible c .alignment 0.3))) | 381 | (m/Scrollable.ensureVisible c .alignment 0.3))) |
| 353 | ;; The callback is void and `ensureVisible` answers a Future: returned | 382 | ;; The callback is void and `ensureVisible` answers a Future: returned |
| 354 | ;; from the tail, it is a value out of a function that has none to give, | 383 | ;; from the tail, it is a value out of a function that has none to give, |
| @@ -672,7 +701,8 @@ | |||
| 672 | ;; The row a jump is aiming at, saying so as it is built. Here and | 701 | ;; The row a jump is aiming at, saying so as it is built. Here and |
| 673 | ;; not in an arm of the `case` below, because the mark is a property | 702 | ;; not in an arm of the `case` below, because the mark is a property |
| 674 | ;; any tag may carry — it happens to be a `:vbox` today. | 703 | ;; any tag may carry — it happens to be a `:vbox` today. |
| 675 | - _ (when (:scroll-here p) (reset! jump-context ctx)) | 704 | + _ (when (:scroll-here p) |
| 705 | + (reset! jump-context ctx)) | ||
| 676 | kids (children (body node)) | 706 | kids (children (body node)) |
| 677 | ;; One column builder for every container, so the rule about what | 707 | ;; One column builder for every container, so the rule about what |
| 678 | ;; fills is applied in one place: children that take what is left are | 708 | ;; fills is applied in one place: children that take what is left are |