nandi/frq · ecb440f

nandi/frqpublic⑂ Fork 0

⑂ ecb440f

Jump to present, which was never on screen

The button shows when the reader has left the present, and nothing ever
said they had. `atPresent` was set true at startup, on opening a room
and by the button itself, and false by nobody — so there was no state in
which the button appeared, and pressing what is not there does nothing.

The core cannot know this on its own: a scroll offset belongs to the
thing doing the scrolling. The renderer watches the backlog and says
`present.left` and `present.back` as it crosses a threshold, sending
only the changes — a drag is a notification per pixel and the core has
one question, not a thousand. Only the backlog reports it; a settings
list has no present to be at.

The button then floats over the backlog rather than taking a row, which
is what `overlay` is for. As a row it was 8 pixels more than a 300×500
window has, and the chat screen overflowed the moment the button finally
worked — the chrome around the backlog already asks for nearly all of a
short window, and this was the row that tipped it. Every chat client
floats this control for the same reason.

Two notes on the tests. The overflow only appeared when the whole group
ran in order, never on its own, which cost a while to pin down — the
reproduction needed three tests in sequence before the error would
appear at all. And a `pumpAndSettle` is not optional here: a timed pump
advances the clock without running the scroll animation out, so the
offset comes back unchanged as though nothing had happened.

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

nandi committed 2026-09-19T19:42:53-07:00 Browse files

ecb440f parent: dcd31d9

modified flutter/lib/nim_renderer.dart +59 -0 Viewed

@@ -73,6 +73,11 @@ class _NimAppState extends State<NimApp> {
73	73	// nobody asked to be moved.
74	74	final _bottomTicks = <String, int>{};
75	75
	76	+ // Whether each scroll is at the present, as last reported to the core.
	77	+ // Only the changes are sent: a notification arrives per pixel of a drag,
	78	+ // and the core has one question, not a thousand.
	79	+ final _atPresent = <String, bool>{};
	80	+
76	81	@override
77	82	void initState() {
78	83	super.initState();
@@ -275,6 +280,10 @@ class _NimAppState extends State<NimApp> {
275	280	// What this node's own children are being built into.
276	281	final childAxis = switch (n.tag) {
277	282	'page' \|\| 'vbox' \|\| 'card' \|\| 'scroll' \|\| 'dialog' => _column,
	283	+ // A stack's children are laid out by the stack, not by a flex: an
	284	+ // `Expanded` among them is illegal, so they must not think they are
	285	+ // in one.
	286	+ 'overlay' => _noAxis,
278	287	// Wrapping unless the row says otherwise. Flipping this default was
279	288	// tried and reverted: only 4 of 15 `hbox` call sites state `wrap` at
280	289	// all, so the other 11 became Rows and overflowed — the tree's habit is
@@ -311,6 +320,31 @@ class _NimAppState extends State<NimApp> {
311	320	),
312	321	);
313	322
	323	+ /// A node with others floating over it — the backlog, with the button
	324	+ /// that takes you back to the present sitting on top of it.
	325	+ ///
	326	+ /// The floating children are given no height of their own, which is
	327	+ /// the whole point: a control that belongs to the backlog should not
	328	+ /// take a row away from it, and on a short window that row is what
	329	+ /// makes the screen overflow.
	330	+ case 'overlay':
	331	+ {
	332	+ final base = kids.isNotEmpty ? kids.first : const SizedBox.shrink();
	333	+ final over = kids.skip(1).toList();
	334	+ return expanded(Stack(
	335	+ children: [
	336	+ Positioned.fill(child: base),
	337	+ for (final o in over)
	338	+ Positioned(
	339	+ left: 0,
	340	+ right: 0,
	341	+ bottom: t.spaceS,
	342	+ child: Align(alignment: Alignment.bottomCenter, child: o),
	343	+ ),
	344	+ ],
	345	+ ));
	346	+ }
	347	+
314	348	case 'vbox':
315	349	{
316	350	Widget col = Column(
@@ -694,6 +728,11 @@ class _NimAppState extends State<NimApp> {
694	728	);
695	729	});
696	730	}
	731	+ // How far from the present counts as having left it. Enough that
	732	+ // the last line being taller than the gap does not toggle this on
	733	+ // its own, and little enough that a nudge upward and back does not
	734	+ // leave the button on screen.
	735	+ const away = 120.0;
697	736	Widget body = SingleChildScrollView(
698	737	controller: c,
699	738	// The backlog reads from the bottom; a settings list from the top.
@@ -703,6 +742,26 @@ class _NimAppState extends State<NimApp> {
703	742	children: _spaced(kids, spacing, vertical: true)),
704	743	);
705	744	body = Scrollbar(controller: c, child: body);
	745	+
	746	+ // Only the backlog reports this. A settings list has no present to
	747	+ // be at, and telling the core about one would put the chat
	748	+ // screen's button on the wrong screen's scrolling.
	749	+ if (stick) {
	750	+ body = NotificationListener<ScrollNotification>(
	751	+ onNotification: (note) {
	752	+ if (note.depth != 0) return false;
	753	+ final m = note.metrics;
	754	+ // Reversed, so the present is the zero end.
	755	+ final here = m.pixels <= m.minScrollExtent + away;
	756	+ if (_atPresent[scrollKey] != here) {
	757	+ _atPresent[scrollKey] = here;
	758	+ _send(here ? 'present.back' : 'present.left');
	759	+ }
	760	+ return false;
	761	+ },
	762	+ child: body,
	763	+ );
	764	+ }
706	765	// A scroll takes what the column has left. Outside a Flex there is
707	766	// nothing to take, and the tree is malformed — `_strandedScroll` is
708	767	// a visible size rather than a correct one, so the layout tests see

    // nobody asked to be moved.
    final _bottomTicks = <String, int>{};
++  // Whether each scroll is at the present, as last reported to the core.
++  // Only the changes are sent: a notification arrives per pixel of a drag,
++  // and the core has one question, not a thousand.
++  final _atPresent = <String, bool>{};
++
    @override
    void initState() {
      super.initState();
      // What this node's own children are being built into.
      final childAxis = switch (n.tag) {
        'page' || 'vbox' || 'card' || 'scroll' || 'dialog' => _column,
++      // A stack's children are laid out by the stack, not by a flex: an
++      // `Expanded` among them is illegal, so they must not think they are
++      // in one.
++      'overlay' => _noAxis,
        // Wrapping unless the row says otherwise. Flipping this default was
        // tried and reverted: only 4 of 15 `hbox` call sites state `wrap` at
        // all, so the other 11 became Rows and overflowed — the tree's habit is
            ),
          );
++      /// A node with others floating over it — the backlog, with the button
++      /// that takes you back to the present sitting on top of it.
++      ///
++      /// The floating children are given no height of their own, which is
++      /// the whole point: a control that belongs to the backlog should not
++      /// take a row away from it, and on a short window that row is what
++      /// makes the screen overflow.
++      case 'overlay':
++        {
++          final base = kids.isNotEmpty ? kids.first : const SizedBox.shrink();
++          final over = kids.skip(1).toList();
++          return expanded(Stack(
++            children: [
++              Positioned.fill(child: base),
++              for (final o in over)
++                Positioned(
++                  left: 0,
++                  right: 0,
++                  bottom: t.spaceS,
++                  child: Align(alignment: Alignment.bottomCenter, child: o),
++                ),
++            ],
++          ));
++        }
++
        case 'vbox':
          {
            Widget col = Column(
                );
              });
            }
++          // How far from the present counts as having left it. Enough that
++          // the last line being taller than the gap does not toggle this on
++          // its own, and little enough that a nudge upward and back does not
++          // leave the button on screen.
++          const away = 120.0;
            Widget body = SingleChildScrollView(
              controller: c,
              // The backlog reads from the bottom; a settings list from the top.
                  children: _spaced(kids, spacing, vertical: true)),
            );
            body = Scrollbar(controller: c, child: body);
++
++          // Only the backlog reports this. A settings list has no present to
++          // be at, and telling the core about one would put the chat
++          // screen's button on the wrong screen's scrolling.
++          if (stick) {
++            body = NotificationListener<ScrollNotification>(
++              onNotification: (note) {
++                if (note.depth != 0) return false;
++                final m = note.metrics;
++                // Reversed, so the present is the zero end.
++                final here = m.pixels <= m.minScrollExtent + away;
++                if (_atPresent[scrollKey] != here) {
++                  _atPresent[scrollKey] = here;
++                  _send(here ? 'present.back' : 'present.left');
++                }
++                return false;
++              },
++              child: body,
++            );
++          }
            // A scroll takes what the column has left. Outside a Flex there is
            // nothing to take, and the tree is malformed — `_strandedScroll` is
            // a visible size rather than a correct one, so the layout tests see

modified flutter/test/nim_layout_test.dart +73 -2 Viewed

@@ -251,7 +251,7 @@ void main() {
251	251	// to do and the target is already on screen — which is what the first
252	252	// version of this test proved: the offset stayed at zero because
253	253	// `ensureVisible` was right not to move.
254		- await layOut(tester, const Size(700, 260));
	254	+ await layOut(tester, const Size(700, 420));
255	255	final c = tester
256	256	.widget<Scrollable>(find.byType(Scrollable).first)
257	257	.controller!;
@@ -277,7 +277,7 @@ void main() {
277	277	// A `jumpTo` left set would scroll back to that row on every frame,
278	278	// which is scrolling taken away from the reader.
279	279	core.demoUi();
280		- await layOut(tester, const Size(700, 260));
	280	+ await layOut(tester, const Size(700, 420));
281	281	await tester.tap(find.text('→').first);
282	282	await tester.pumpAndSettle();
283	283	expectLaidOut(tester, 'the backlog after the arrow');
@@ -286,6 +286,77 @@ void main() {
286	286	});
287	287	});
288	288
	289	+ group('jump to present', () {
	290	+ // The button only shows when the reader has left the present, and
	291	+ // nothing ever said they had: `atPresent` was set true at startup, on
	292	+ // opening a room and by the button itself, and false by nobody. So the
	293	+ // button was never on screen, which is what "jump to present not
	294	+ // working" looked like from outside.
	295	+ testWidgets('appears once the backlog is scrolled away from',
	296	+ (tester) async {
	297	+ core.demoUi();
	298	+ await layOut(tester, const Size(700, 420));
	299	+ expect(find.text('↓ Jump to present'), findsNothing);
	300	+
	301	+ final c = tester
	302	+ .widget<Scrollable>(find.byType(Scrollable).first)
	303	+ .controller!;
	304	+ c.jumpTo(c.position.maxScrollExtent);
	305	+ await tester.pumpAndSettle();
	306	+ expect(find.text('↓ Jump to present'), findsOneWidget,
	307	+ reason: 'the core was never told the reader had left');
	308	+ });
	309	+
	310	+ testWidgets('and takes the view back, and goes away again',
	311	+ (tester) async {
	312	+ core.demoUi();
	313	+ await layOut(tester, const Size(700, 420));
	314	+ final c = tester
	315	+ .widget<Scrollable>(find.byType(Scrollable).first)
	316	+ .controller!;
	317	+ c.jumpTo(c.position.maxScrollExtent);
	318	+ await tester.pumpAndSettle();
	319	+
	320	+ await tester.tap(find.text('↓ Jump to present'));
	321	+ await tester.pumpAndSettle();
	322	+ // Reversed, so the present is the zero end.
	323	+ expect(c.offset, closeTo(c.position.minScrollExtent, 1.0));
	324	+ expect(find.text('↓ Jump to present'), findsNothing);
	325	+ expectLaidOut(tester, 'the backlog back at the present');
	326	+ });
	327	+
	328	+ testWidgets('and the button fits the cramped window too', (tester) async {
	329	+ // It is a row the chat screen did not have before, and every row is
	330	+ // height the backlog does not get. At 260 points tall this overflows
	331	+ // by a pixel, which is how the first run of these tests failed; the
	332	+ // cramped size the rest of the suite uses is the one that has to hold.
	333	+ core.demoUi();
	334	+ await layOut(tester, sizes['cramped']!);
	335	+ final c = tester
	336	+ .widget<Scrollable>(find.byType(Scrollable).first)
	337	+ .controller!;
	338	+ c.jumpTo(c.position.maxScrollExtent);
	339	+ await tester.pumpAndSettle();
	340	+ expect(find.text('↓ Jump to present'), findsOneWidget);
	341	+ expectLaidOut(tester, 'cramped, with the jump button up');
	342	+ });
	343	+
	344	+ testWidgets('a settings list has no present to be at', (tester) async {
	345	+ // It would be the chat screen's button on the wrong screen's
	346	+ // scrolling.
	347	+ core.demoUi();
	348	+ core.dispatch('screen.settings');
	349	+ await layOut(tester, const Size(700, 420));
	350	+ final c = tester
	351	+ .widget<Scrollable>(find.byType(Scrollable).first)
	352	+ .controller!;
	353	+ c.jumpTo(c.position.maxScrollExtent);
	354	+ await tester.pumpAndSettle();
	355	+ expect(find.text('↓ Jump to present'), findsNothing);
	356	+ expectLaidOut(tester, 'settings scrolled');
	357	+ });
	358	+ });
	359	+
289	360	group('identity', () {
290	361	// Duplicate keys among siblings are an error Flutter throws at build
291	362	// time, so this is mostly a guard on the tree the core emits: every

        // to do and the target is already on screen — which is what the first
        // version of this test proved: the offset stayed at zero because
        // `ensureVisible` was right not to move.
--      await layOut(tester, const Size(700, 260));
++      await layOut(tester, const Size(700, 420));
        final c = tester
            .widget<Scrollable>(find.byType(Scrollable).first)
            .controller!;
        // A `jumpTo` left set would scroll back to that row on every frame,
        // which is scrolling taken away from the reader.
        core.demoUi();
--      await layOut(tester, const Size(700, 260));
++      await layOut(tester, const Size(700, 420));
        await tester.tap(find.text('→').first);
        await tester.pumpAndSettle();
        expectLaidOut(tester, 'the backlog after the arrow');
      });
    });
++  group('jump to present', () {
++    // The button only shows when the reader has left the present, and
++    // nothing ever said they had: `atPresent` was set true at startup, on
++    // opening a room and by the button itself, and false by nobody. So the
++    // button was never on screen, which is what "jump to present not
++    // working" looked like from outside.
++    testWidgets('appears once the backlog is scrolled away from',
++        (tester) async {
++      core.demoUi();
++      await layOut(tester, const Size(700, 420));
++      expect(find.text('↓ Jump to present'), findsNothing);
++
++      final c = tester
++          .widget<Scrollable>(find.byType(Scrollable).first)
++          .controller!;
++      c.jumpTo(c.position.maxScrollExtent);
++      await tester.pumpAndSettle();
++      expect(find.text('↓ Jump to present'), findsOneWidget,
++          reason: 'the core was never told the reader had left');
++    });
++
++    testWidgets('and takes the view back, and goes away again',
++        (tester) async {
++      core.demoUi();
++      await layOut(tester, const Size(700, 420));
++      final c = tester
++          .widget<Scrollable>(find.byType(Scrollable).first)
++          .controller!;
++      c.jumpTo(c.position.maxScrollExtent);
++      await tester.pumpAndSettle();
++
++      await tester.tap(find.text('↓ Jump to present'));
++      await tester.pumpAndSettle();
++      // Reversed, so the present is the zero end.
++      expect(c.offset, closeTo(c.position.minScrollExtent, 1.0));
++      expect(find.text('↓ Jump to present'), findsNothing);
++      expectLaidOut(tester, 'the backlog back at the present');
++    });
++
++    testWidgets('and the button fits the cramped window too', (tester) async {
++      // It is a row the chat screen did not have before, and every row is
++      // height the backlog does not get. At 260 points tall this overflows
++      // by a pixel, which is how the first run of these tests failed; the
++      // cramped size the rest of the suite uses is the one that has to hold.
++      core.demoUi();
++      await layOut(tester, sizes['cramped']!);
++      final c = tester
++          .widget<Scrollable>(find.byType(Scrollable).first)
++          .controller!;
++      c.jumpTo(c.position.maxScrollExtent);
++      await tester.pumpAndSettle();
++      expect(find.text('↓ Jump to present'), findsOneWidget);
++      expectLaidOut(tester, 'cramped, with the jump button up');
++    });
++
++    testWidgets('a settings list has no present to be at', (tester) async {
++      // It would be the chat screen's button on the wrong screen's
++      // scrolling.
++      core.demoUi();
++      core.dispatch('screen.settings');
++      await layOut(tester, const Size(700, 420));
++      final c = tester
++          .widget<Scrollable>(find.byType(Scrollable).first)
++          .controller!;
++      c.jumpTo(c.position.maxScrollExtent);
++      await tester.pumpAndSettle();
++      expect(find.text('↓ Jump to present'), findsNothing);
++      expectLaidOut(tester, 'settings scrolled');
++    });
++  });
++
    group('identity', () {
      // Duplicate keys among siblings are an error Flutter throws at build
      // time, so this is mostly a guard on the tree the core emits: every

modified nim/src/frq/reducer.nim +11 -0 Viewed

@@ -421,6 +421,17 @@ proc dispatch*(event: JsonNode) =
421	421	app.atPresent = true
422	422	app.jumpTick += 1
423	423
	424	+ # Where the reader is in the backlog, as the renderer sees it. The core
	425	+ # cannot know this on its own: a scroll offset belongs to the thing doing
	426	+ # the scrolling, and nothing else here has one.
	427	+ #
	428	+ # Without these `atPresent` only ever became true — at startup, on opening
	429	+ # a room, and on pressing the button — so the button that takes you back to
	430	+ # the present was never shown, there being no state in which the reader had
	431	+ # left it.
	432	+ of "present.left": app.atPresent = false
	433	+ of "present.back": app.atPresent = true
	434	+
424	435	# ------------------------------------------------------------ the compose
425	436	of "draft.change": app.draft = value
426	437	of "send": sendDraft()

      app.atPresent = true
      app.jumpTick += 1
++  # Where the reader is in the backlog, as the renderer sees it. The core
++  # cannot know this on its own: a scroll offset belongs to the thing doing
++  # the scrolling, and nothing else here has one.
++  #
++  # Without these `atPresent` only ever became true — at startup, on opening
++  # a room, and on pressing the button — so the button that takes you back to
++  # the present was never shown, there being no state in which the reader had
++  # left it.
++  of "present.left": app.atPresent = false
++  of "present.back": app.atPresent = true
++
    # ------------------------------------------------------------ the compose
    of "draft.change": app.draft = value
    of "send": sendDraft()

modified nim/src/frq/screens/chat.nim +6 -3 Viewed

@@ -481,13 +481,16 @@ proc chatScreen*(s: State, connected: bool): Node =
481	481	# `expand` on the row itself: it is the thing that takes the column's
482	482	# remaining height. The renderer used to infer that by looking at this
483	483	# row's children, which is the prop being on the wrong node.
484		- n("hbox", %*{"spacing": 8, "wrap": false, "expand": true},
485		- @[messages, peoplePane]),
	484	+ # The jump button floats over the backlog rather than taking a row of
	485	+ # its own; see `ui.overlay`.
	486	+ overlay(%*{"key": "backlog", "expand": true},
	487	+ n("hbox", %*{"spacing": 8, "wrap": false, "expand": true},
	488	+ @[messages, peoplePane]),
	489	+ jump),
486	490	overview,
487	491	profile,
488	492	lightbox,
489	493	returnRow,
490		- jump,
491	494	banners,
492	495	separator(),
493	496	compose)

      # `expand` on the row itself: it is the thing that takes the column's
      # remaining height. The renderer used to infer that by looking at this
      # row's children, which is the prop being on the wrong node.
--    n("hbox", %*{"spacing": 8, "wrap": false, "expand": true},
++    # The jump button floats over the backlog rather than taking a row of
--      @[messages, peoplePane]),
++    # its own; see `ui.overlay`.
++    overlay(%*{"key": "backlog", "expand": true},
++      n("hbox", %*{"spacing": 8, "wrap": false, "expand": true},
++        @[messages, peoplePane]),
++      jump),
      overview,
      profile,
      lightbox,
      returnRow,
--    jump,
      banners,
      separator(),
      compose)

modified nim/src/frq/ui.nim +12 -0 Viewed

@@ -126,6 +126,18 @@ func image*(src: string, maxWidth = 0, maxHeight = 0, onClick = ""): Node =
126	126	if onClick.len > 0: p["onClick"] = %onClick
127	127	n("image", p)
128	128
	129	+func overlay*(props: JsonNode, base: Node, over: varargs[Node]): Node =
	130	+ ## One node with others floating over it, bottom-centred.
	131	+ ##
	132	+ ## For the controls that belong to the backlog rather than beside it.
	133	+ ## "Jump to present" as a row of its own is a row the conversation does not
	134	+ ## get, and on a short window it is the row that makes the screen overflow
	135	+ ## — the chrome around the backlog already asks for more height than a 300
	136	+ ## by 500 window has.
	137	+ result = n("overlay", props, @[base])
	138	+ for o in over:
	139	+ if not o.isNil: result.children.add o
	140	+
129	141	func avatar*(url, fallback: string, size = 24, onClick = ""): Node =
130	142	## A profile picture, or the letter to draw where there is none. The
131	143	## fallback is here rather than in the renderer because which letter is a

    if onClick.len > 0: p["onClick"] = %onClick
    n("image", p)
++func overlay*(props: JsonNode, base: Node, over: varargs[Node]): Node =
++  ## One node with others floating over it, bottom-centred.
++  ##
++  ## For the controls that belong *to* the backlog rather than beside it.
++  ## "Jump to present" as a row of its own is a row the conversation does not
++  ## get, and on a short window it is the row that makes the screen overflow
++  ## — the chrome around the backlog already asks for more height than a 300
++  ## by 500 window has.
++  result = n("overlay", props, @[base])
++  for o in over:
++    if not o.isNil: result.children.add o
++
  func avatar*(url, fallback: string, size = 24, onClick = ""): Node =
    ## A profile picture, or the letter to draw where there is none. The
    ## fallback is here rather than in the renderer because which letter is a