Skip to content

fix: stop scroll ticks cancelling the floating toolbar's pending show - #1222

Open
matanrotman wants to merge 1 commit into
AppFlowy-IO:mainfrom
matanrotman:fix/floating-toolbar-debounce-race
Open

fix: stop scroll ticks cancelling the floating toolbar's pending show#1222
matanrotman wants to merge 1 commit into
AppFlowy-IO:mainfrom
matanrotman:fix/floating-toolbar-debounce-race

Conversation

@matanrotman

Copy link
Copy Markdown

Problem

The desktop floating toolbar debounces two independent triggers onto a single key ('show the toolbar'):

  • selection changes — 200ms
  • scroll offset changes — Duration.zero

Debounce.debounce() is keyed by name, so the later call replaces the earlier one. Whichever fired last won; the other was silently dropped.

These two interleave during any drag that auto-scrolls — selecting text and pulling past the viewport edge, which is precisely when the toolbar has to track a selection extent that doesn't exist on screen until the scroll happens. A scroll tick lands on the shared key, cancels the pending (and more authoritative) selection-driven show, and the toolbar never appears.

There is a second issue in the same path. Auto-scroll calls _onScrollPositionChanged from its own ticker, in the same call stack as the offset change — before the layout pass that repositions the scrolled content has run. _showToolbar therefore measures geometry left over from the previous frame.

Fix

  • Give each trigger its own debounce key (_selectionDebounceKey, _scrollDebounceKey) so they can no longer evict one another. _showAfterDelay takes the key as a parameter; _clear() and dispose() cancel both.
  • Defer the scroll-triggered show to addPostFrameCallback with a mounted guard, so the toolbar reads geometry only after the frame has settled.

No public API change — both keys are private to _FloatingToolbarState. The mobile toolbar has a single trigger and is deliberately untouched.

Testing

This has been running in a downstream fork (AppFlowy) for several sessions, where the floating toolbar behaves correctly — including after selections that require scrolling, which is the case that originally surfaced the bug.

I should be upfront that I could not get a widget test to fail against the unfixed code. A single jumpTo() + pumpAndSettle() doesn't recreate what a real drag does — a continuous stream of scroll ticks that never fully settles — and that stream is exactly what makes the two debounce calls race. So the diagnosis here rests on reading the code path rather than on a reproducing test. Happy to add one if you can point me at a pattern that drives auto-scroll faithfully.

🤖 Generated with Claude Code

The desktop floating toolbar debounced two independent triggers onto a
single key ('show the toolbar'):

  - selection changes, at 200ms
  - scroll offset changes, at Duration.zero

Debounce.debounce() keyed by name means the later call replaces the
earlier one, so whichever fired last won and the other was silently
dropped. During a drag that auto-scrolls -- selecting text and pulling
past the viewport edge, exactly when the toolbar must track a selection
extent that is off-screen until the scroll happens -- the two interleave
unpredictably, and a scroll tick could cancel the pending, more
authoritative selection-driven show. The toolbar then fails to appear.

Give each trigger its own debounce key so they no longer evict each
other, and cancel both wherever the toolbar is cleared or disposed.

Also defer the scroll-triggered show to a post-frame callback. Auto-scroll
fires _onScrollPositionChanged from its own ticker, in the same call stack
as the offset change and ahead of the layout pass that repositions the
scrolled content -- so _showToolbar could read stale geometry from the
previous frame. The post-frame callback lets layout settle first, with a
mounted guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CLAassistant

CLAassistant commented Jul 15, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

matanrotman added a commit to matanrotman/Ludwig that referenced this pull request Jul 27, 2026
…wy-editor#1222)

Pushed and opened at the user's explicit instruction. STATUS.md and the RTL spec
said "prepared, NOT sent", which is now false.

Also records the two things that gate it, neither of which we control:
- The CLA is unsigned, so it cannot merge. Only the user can sign it.
- Upstream's Flutter 3.38.5 CI is the first real compile this fix has ever had;
  it was never compilable locally on 3.27.4, so it was verified by inspection
  only (parses, dart format-clean, no dangling _debounceKey).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants