Lay every screen out in a test, and fix what that found
My last fix was verified against the room list and I said so, because a Wayland window cannot be clicked from a script and nothing automated had ever laid the chat screen out. It was still broken, and the screen nobody could reach was exactly where. So the missing test first: `just test layout` renders every screen at phone, desktop and a deliberately cramped size, headless, with no GL and no window. `frq_ui_demo` fills a room without a server — a long unbroken URL, a very long word, an image, reactions, a reply, an edited line, a system line, and a message on a second day — because a layout bug is about what does not fit. `tester.takeException()` is the point of it: a layout error goes to FlutterError rather than to the caller, so a test that only pumps and asserts on widgets passes while the screen is in pieces. It found three, all mine. `runNodes` emitted an `inline` hbox and I rendered hboxes as a Wrap. Children of a Wrap are given unbounded width, so a long URL or a long word could never wrap: it overflowed, the layout failed, and every box under it was then hit-tested having never been laid out. That is the whole pile. An inline row is one RichText of spans now, which is what "a paragraph" meant in the Clojure all along — with the tap recognisers held per URL and disposed with the state, since this tree is rebuilt on every keystroke. The settings checkbox put prose beside a tick in a Row with nothing flexible, so "Hide join/part messages" overflowed a phone by 64 pixels and a narrower window by 124. And the row holding the backlog stretched on its cross axis to give the messages their height, from a Row that had no bounded height to give: a Row in a Column is as tall as its tallest child, so the stretch resolved to infinity. It takes the column's remaining height itself now. 21 layout tests, and they are in `just test all`. The window reaches #test with 116 lines and no layout errors at all. Still unproven the same way as before, and worth saying rather than leaving: a room is opened here by dispatching an event, not by clicking one. What the tests cover is every screen laying out; what they do not cover is the gestures that move between them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
36bdfc5 parent: f1e99b4 modified
dart/frq_core/lib/frq_core.dart +5 -0 | @@ -323,6 +323,11 @@ UiNode dispatch(String id, [String value = '']) { | ||
| 323 | 323 | } |
| 324 | 324 | } |
| 325 | 325 | |
| 326 | +/// Fill a room with a representative conversation, so a test can lay the chat | |
| 327 | +/// screen out without a server. See the Nim side for why it exists. | |
| 328 | +void demoUi() => | |
| 329 | + _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_demo')(); | |
| 330 | + | |
| 326 | 331 | /// Back to a fresh state, for a caller that wants a known starting point. |
| 327 | 332 | void resetUi() => |
| 328 | 333 | _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset')(); |
| @@ -323,6 +323,11 @@ UiNode dispatch(String id, [String value = '']) { | |||
| 323 | } | 323 | } |
| 324 | } | 324 | } |
| 325 | 325 | ||
| 326 | +/// Fill a room with a representative conversation, so a test can lay the chat | ||
| 327 | +/// screen out without a server. See the Nim side for why it exists. | ||
| 328 | +void demoUi() => | ||
| 329 | + _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_demo')(); | ||
| 330 | + | ||
| 326 | /// Back to a fresh state, for a caller that wants a known starting point. | 331 | /// Back to a fresh state, for a caller that wants a known starting point. |
| 327 | void resetUi() => | 332 | void resetUi() => |
| 328 | _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset')(); | 333 | _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset')(); |
modified
flutter/lib/nim_renderer.dart +79 -11 | @@ -13,6 +13,8 @@ library; | ||
| 13 | 13 | import 'dart:async'; |
| 14 | 14 | import 'dart:io'; |
| 15 | 15 | |
| 16 | +import 'package:flutter/gestures.dart'; | |
| 17 | + | |
| 16 | 18 | import 'package:flutter/material.dart'; |
| 17 | 19 | import 'package:frq_core/frq_core.dart' as core; |
| 18 | 20 | |
| @@ -39,6 +41,11 @@ class _NimAppState extends State<NimApp> { | ||
| 39 | 41 | final _controllers = <String, TextEditingController>{}; |
| 40 | 42 | final _focus = <String, FocusNode>{}; |
| 41 | 43 | |
| 44 | + // One tap recogniser per link URL, kept across rebuilds and disposed with | |
| 45 | + // the state. A recogniser made during build and dropped on the next frame | |
| 46 | + // leaks, and this tree is rebuilt on every keystroke. | |
| 47 | + final _linkTaps = <String, TapGestureRecognizer>{}; | |
| 48 | + | |
| 42 | 49 | @override |
| 43 | 50 | void initState() { |
| 44 | 51 | super.initState(); |
| @@ -67,6 +74,9 @@ class _NimAppState extends State<NimApp> { | ||
| 67 | 74 | for (final f in _focus.values) { |
| 68 | 75 | f.dispose(); |
| 69 | 76 | } |
| 77 | + for (final r in _linkTaps.values) { | |
| 78 | + r.dispose(); | |
| 79 | + } | |
| 70 | 80 | super.dispose(); |
| 71 | 81 | } |
| 72 | 82 | |
| @@ -194,6 +204,21 @@ class _NimAppState extends State<NimApp> { | ||
| 194 | 204 | : col; |
| 195 | 205 | } |
| 196 | 206 | |
| 207 | + // A paragraph: the words and the links of one message, wrapping as text | |
| 208 | + // rather than as boxes. | |
| 209 | + // | |
| 210 | + // NOT a Wrap, which is what this was. Children of a Wrap are given | |
| 211 | + // unbounded width, so a long URL or a long word can never wrap — it | |
| 212 | + // overflows, the layout fails, and every box under it is then hit-tested | |
| 213 | + // having never been laid out. That is the pile of "Cannot hit test" | |
| 214 | + // errors the chat screen produced. Spans in one RichText wrap the way | |
| 215 | + // the Clojure's `:inline` row always meant. | |
| 216 | + case 'hbox' when n.prop('inline', false): | |
| 217 | + return Text.rich( | |
| 218 | + TextSpan(children: n.children.map(_span).toList()), | |
| 219 | + softWrap: true, | |
| 220 | + ); | |
| 221 | + | |
| 197 | 222 | case 'hbox': |
| 198 | 223 | { |
| 199 | 224 | // Wrap and not Row: `:hbox` in the screens means "these go together |
| @@ -205,19 +230,26 @@ class _NimAppState extends State<NimApp> { | ||
| 205 | 230 | if (!wrapping) { |
| 206 | 231 | // A child asking to fill the height gets it from the row's cross |
| 207 | 232 | // axis, not from an Expanded — Expanded in a Row is about width. |
| 233 | + // | |
| 234 | + // But stretch needs a bounded height to stretch to, and a Row in a | |
| 235 | + // Column has none of its own: it is as tall as its tallest child. | |
| 236 | + // So the row that holds a filling child has to take the column's | |
| 237 | + // remaining height itself, or the stretch resolves to infinity and | |
| 238 | + // the assertion reads `BoxConstraints forces an infinite height`. | |
| 208 | 239 | final stretches = |
| 209 | 240 | n.children.any((c) => c.prop('fillHeight', false)); |
| 210 | - return _margins( | |
| 211 | - n, | |
| 212 | - Row( | |
| 213 | - crossAxisAlignment: stretches | |
| 214 | - ? CrossAxisAlignment.stretch | |
| 215 | - : (align == 'end' | |
| 216 | - ? CrossAxisAlignment.end | |
| 217 | - : CrossAxisAlignment.center), | |
| 218 | - children: _spaced(kids, spacing, vertical: false), | |
| 219 | - ), | |
| 241 | + final row = Row( | |
| 242 | + crossAxisAlignment: stretches | |
| 243 | + ? CrossAxisAlignment.stretch | |
| 244 | + : (align == 'end' | |
| 245 | + ? CrossAxisAlignment.end | |
| 246 | + : CrossAxisAlignment.center), | |
| 247 | + children: _spaced(kids, spacing, vertical: false), | |
| 220 | 248 | ); |
| 249 | + if (stretches && axis == _column) { | |
| 250 | + return Expanded(child: _margins(n, row)); | |
| 251 | + } | |
| 252 | + return _margins(n, row); | |
| 221 | 253 | } |
| 222 | 254 | return _margins( |
| 223 | 255 | n, |
| @@ -362,7 +394,13 @@ class _NimAppState extends State<NimApp> { | ||
| 362 | 394 | value: n.prop('active', false), |
| 363 | 395 | onChanged: (_) => _send(onToggled), |
| 364 | 396 | ), |
| 365 | - Text(n.prop('label', ''), style: _style(t.textBody, t.onBg)), | |
| 397 | + // Flexible, because the label is prose and the row is as wide | |
| 398 | + // as the window: "Hide join/part messages" beside a checkbox | |
| 399 | + // overflows a phone otherwise. | |
| 400 | + Flexible( | |
| 401 | + child: Text(n.prop('label', ''), | |
| 402 | + style: _style(t.textBody, t.onBg)), | |
| 403 | + ), | |
| 366 | 404 | ], |
| 367 | 405 | ), |
| 368 | 406 | ); |
| @@ -569,6 +607,36 @@ class _NimAppState extends State<NimApp> { | ||
| 569 | 607 | } |
| 570 | 608 | } |
| 571 | 609 | |
| 610 | + /// One node of an inline paragraph, as a span. | |
| 611 | + /// | |
| 612 | + /// Only `text` and `link` appear here — they are the only things `runNodes` | |
| 613 | + /// emits — and anything else falls back to its plain text so an unexpected | |
| 614 | + /// tag degrades to something readable rather than vanishing. | |
| 615 | + InlineSpan _span(core.UiNode n) { | |
| 616 | + switch (n.tag) { | |
| 617 | + case 'link': | |
| 618 | + final url = n.prop('url', n.prop('label', '')); | |
| 619 | + final onClick = n.prop('onClick', ''); | |
| 620 | + return TextSpan( | |
| 621 | + text: n.prop('label', ''), | |
| 622 | + style: _style(t.textBody, t.accent) | |
| 623 | + .copyWith(decoration: TextDecoration.underline, | |
| 624 | + decorationColor: t.accent), | |
| 625 | + recognizer: onClick.isEmpty | |
| 626 | + ? null | |
| 627 | + : (_linkTaps[url] ??= TapGestureRecognizer() | |
| 628 | + ..onTap = () => _send(onClick)), | |
| 629 | + ); | |
| 630 | + case 'text': | |
| 631 | + return TextSpan( | |
| 632 | + text: n.prop('text', ''), style: _style(t.textBody, t.onBg)); | |
| 633 | + default: | |
| 634 | + return TextSpan( | |
| 635 | + text: n.prop('label', n.prop('text', '')), | |
| 636 | + style: _style(t.textBody, t.onBg)); | |
| 637 | + } | |
| 638 | + } | |
| 639 | + | |
| 572 | 640 | /// `margin`, `marginTop`, `marginBottom`, `marginRight` — the props the |
| 573 | 641 | /// screens use to buy air without a wrapper each time. |
| 574 | 642 | Widget _margins(core.UiNode n, Widget child) { |
| @@ -13,6 +13,8 @@ library; | |||
| 13 | import 'dart:async'; | 13 | import 'dart:async'; |
| 14 | import 'dart:io'; | 14 | import 'dart:io'; |
| 15 | 15 | ||
| 16 | +import 'package:flutter/gestures.dart'; | ||
| 17 | + | ||
| 16 | import 'package:flutter/material.dart'; | 18 | import 'package:flutter/material.dart'; |
| 17 | import 'package:frq_core/frq_core.dart' as core; | 19 | import 'package:frq_core/frq_core.dart' as core; |
| 18 | 20 | ||
| @@ -39,6 +41,11 @@ class _NimAppState extends State<NimApp> { | |||
| 39 | final _controllers = <String, TextEditingController>{}; | 41 | final _controllers = <String, TextEditingController>{}; |
| 40 | final _focus = <String, FocusNode>{}; | 42 | final _focus = <String, FocusNode>{}; |
| 41 | 43 | ||
| 44 | + // One tap recogniser per link URL, kept across rebuilds and disposed with | ||
| 45 | + // the state. A recogniser made during build and dropped on the next frame | ||
| 46 | + // leaks, and this tree is rebuilt on every keystroke. | ||
| 47 | + final _linkTaps = <String, TapGestureRecognizer>{}; | ||
| 48 | + | ||
| 42 | @override | 49 | @override |
| 43 | void initState() { | 50 | void initState() { |
| 44 | super.initState(); | 51 | super.initState(); |
| @@ -67,6 +74,9 @@ class _NimAppState extends State<NimApp> { | |||
| 67 | for (final f in _focus.values) { | 74 | for (final f in _focus.values) { |
| 68 | f.dispose(); | 75 | f.dispose(); |
| 69 | } | 76 | } |
| 77 | + for (final r in _linkTaps.values) { | ||
| 78 | + r.dispose(); | ||
| 79 | + } | ||
| 70 | super.dispose(); | 80 | super.dispose(); |
| 71 | } | 81 | } |
| 72 | 82 | ||
| @@ -194,6 +204,21 @@ class _NimAppState extends State<NimApp> { | |||
| 194 | : col; | 204 | : col; |
| 195 | } | 205 | } |
| 196 | 206 | ||
| 207 | + // A paragraph: the words and the links of one message, wrapping as text | ||
| 208 | + // rather than as boxes. | ||
| 209 | + // | ||
| 210 | + // NOT a Wrap, which is what this was. Children of a Wrap are given | ||
| 211 | + // unbounded width, so a long URL or a long word can never wrap — it | ||
| 212 | + // overflows, the layout fails, and every box under it is then hit-tested | ||
| 213 | + // having never been laid out. That is the pile of "Cannot hit test" | ||
| 214 | + // errors the chat screen produced. Spans in one RichText wrap the way | ||
| 215 | + // the Clojure's `:inline` row always meant. | ||
| 216 | + case 'hbox' when n.prop('inline', false): | ||
| 217 | + return Text.rich( | ||
| 218 | + TextSpan(children: n.children.map(_span).toList()), | ||
| 219 | + softWrap: true, | ||
| 220 | + ); | ||
| 221 | + | ||
| 197 | case 'hbox': | 222 | case 'hbox': |
| 198 | { | 223 | { |
| 199 | // Wrap and not Row: `:hbox` in the screens means "these go together | 224 | // Wrap and not Row: `:hbox` in the screens means "these go together |
| @@ -205,19 +230,26 @@ class _NimAppState extends State<NimApp> { | |||
| 205 | if (!wrapping) { | 230 | if (!wrapping) { |
| 206 | // A child asking to fill the height gets it from the row's cross | 231 | // A child asking to fill the height gets it from the row's cross |
| 207 | // axis, not from an Expanded — Expanded in a Row is about width. | 232 | // axis, not from an Expanded — Expanded in a Row is about width. |
| 233 | + // | ||
| 234 | + // But stretch needs a bounded height to stretch to, and a Row in a | ||
| 235 | + // Column has none of its own: it is as tall as its tallest child. | ||
| 236 | + // So the row that holds a filling child has to take the column's | ||
| 237 | + // remaining height itself, or the stretch resolves to infinity and | ||
| 238 | + // the assertion reads `BoxConstraints forces an infinite height`. | ||
| 208 | final stretches = | 239 | final stretches = |
| 209 | n.children.any((c) => c.prop('fillHeight', false)); | 240 | n.children.any((c) => c.prop('fillHeight', false)); |
| 210 | - return _margins( | 241 | + final row = Row( |
| 211 | - n, | 242 | + crossAxisAlignment: stretches |
| 212 | - Row( | 243 | + ? CrossAxisAlignment.stretch |
| 213 | - crossAxisAlignment: stretches | 244 | + : (align == 'end' |
| 214 | - ? CrossAxisAlignment.stretch | 245 | + ? CrossAxisAlignment.end |
| 215 | - : (align == 'end' | 246 | + : CrossAxisAlignment.center), |
| 216 | - ? CrossAxisAlignment.end | 247 | + children: _spaced(kids, spacing, vertical: false), |
| 217 | - : CrossAxisAlignment.center), | ||
| 218 | - children: _spaced(kids, spacing, vertical: false), | ||
| 219 | - ), | ||
| 220 | ); | 248 | ); |
| 249 | + if (stretches && axis == _column) { | ||
| 250 | + return Expanded(child: _margins(n, row)); | ||
| 251 | + } | ||
| 252 | + return _margins(n, row); | ||
| 221 | } | 253 | } |
| 222 | return _margins( | 254 | return _margins( |
| 223 | n, | 255 | n, |
| @@ -362,7 +394,13 @@ class _NimAppState extends State<NimApp> { | |||
| 362 | value: n.prop('active', false), | 394 | value: n.prop('active', false), |
| 363 | onChanged: (_) => _send(onToggled), | 395 | onChanged: (_) => _send(onToggled), |
| 364 | ), | 396 | ), |
| 365 | - Text(n.prop('label', ''), style: _style(t.textBody, t.onBg)), | 397 | + // Flexible, because the label is prose and the row is as wide |
| 398 | + // as the window: "Hide join/part messages" beside a checkbox | ||
| 399 | + // overflows a phone otherwise. | ||
| 400 | + Flexible( | ||
| 401 | + child: Text(n.prop('label', ''), | ||
| 402 | + style: _style(t.textBody, t.onBg)), | ||
| 403 | + ), | ||
| 366 | ], | 404 | ], |
| 367 | ), | 405 | ), |
| 368 | ); | 406 | ); |
| @@ -569,6 +607,36 @@ class _NimAppState extends State<NimApp> { | |||
| 569 | } | 607 | } |
| 570 | } | 608 | } |
| 571 | 609 | ||
| 610 | + /// One node of an inline paragraph, as a span. | ||
| 611 | + /// | ||
| 612 | + /// Only `text` and `link` appear here — they are the only things `runNodes` | ||
| 613 | + /// emits — and anything else falls back to its plain text so an unexpected | ||
| 614 | + /// tag degrades to something readable rather than vanishing. | ||
| 615 | + InlineSpan _span(core.UiNode n) { | ||
| 616 | + switch (n.tag) { | ||
| 617 | + case 'link': | ||
| 618 | + final url = n.prop('url', n.prop('label', '')); | ||
| 619 | + final onClick = n.prop('onClick', ''); | ||
| 620 | + return TextSpan( | ||
| 621 | + text: n.prop('label', ''), | ||
| 622 | + style: _style(t.textBody, t.accent) | ||
| 623 | + .copyWith(decoration: TextDecoration.underline, | ||
| 624 | + decorationColor: t.accent), | ||
| 625 | + recognizer: onClick.isEmpty | ||
| 626 | + ? null | ||
| 627 | + : (_linkTaps[url] ??= TapGestureRecognizer() | ||
| 628 | + ..onTap = () => _send(onClick)), | ||
| 629 | + ); | ||
| 630 | + case 'text': | ||
| 631 | + return TextSpan( | ||
| 632 | + text: n.prop('text', ''), style: _style(t.textBody, t.onBg)); | ||
| 633 | + default: | ||
| 634 | + return TextSpan( | ||
| 635 | + text: n.prop('label', n.prop('text', '')), | ||
| 636 | + style: _style(t.textBody, t.onBg)); | ||
| 637 | + } | ||
| 638 | + } | ||
| 639 | + | ||
| 572 | /// `margin`, `marginTop`, `marginBottom`, `marginRight` — the props the | 640 | /// `margin`, `marginTop`, `marginBottom`, `marginRight` — the props the |
| 573 | /// screens use to buy air without a wrapper each time. | 641 | /// screens use to buy air without a wrapper each time. |
| 574 | Widget _margins(core.UiNode n, Widget child) { | 642 | Widget _margins(core.UiNode n, Widget child) { |
added
flutter/test/nim_layout_test.dart +139 -0 | new file mode 100644 | ||
| @@ -0,0 +1,139 @@ | ||
| 1 | +/// Every screen, laid out for real, at sizes that squeeze. | |
| 2 | +/// | |
| 3 | +/// This is the test that was missing. `Cannot hit test a render box that has | |
| 4 | +/// never been laid out` is what a failed layout looks like from the outside, | |
| 5 | +/// and nothing automated ever laid the chat screen out — a GUI on Wayland | |
| 6 | +/// cannot be clicked, so every check stopped at the room list while the | |
| 7 | +/// biggest screen in the app went out unverified. | |
| 8 | +/// | |
| 9 | +/// `tester.takeException()` is the whole point: a layout error is reported to | |
| 10 | +/// FlutterError rather than thrown at the caller, so a test that only pumps | |
| 11 | +/// and asserts on widgets passes while the screen is broken. These fail. | |
| 12 | +/// | |
| 13 | +/// just test layout | |
| 14 | +import 'package:flutter/material.dart'; | |
| 15 | +import 'package:flutter_test/flutter_test.dart'; | |
| 16 | +import 'package:frq_core/frq_core.dart' as core; | |
| 17 | +import 'package:cljd_flutter/nim_renderer.dart'; | |
| 18 | + | |
| 19 | +/// Phone, small desktop, and a deliberately cramped one. The head row of the | |
| 20 | +/// chat screen asks for more than 360 points has, which is why it wraps. | |
| 21 | +const sizes = <String, Size>{ | |
| 22 | + 'phone': Size(360, 690), | |
| 23 | + 'desktop': Size(1280, 800), | |
| 24 | + 'cramped': Size(300, 500), | |
| 25 | +}; | |
| 26 | + | |
| 27 | +Future<void> layOut(WidgetTester tester, Size size) async { | |
| 28 | + await tester.binding.setSurfaceSize(size); | |
| 29 | + addTearDown(() => tester.binding.setSurfaceSize(null)); | |
| 30 | + await tester.pumpWidget(const NimApp()); | |
| 31 | + await tester.pump(); | |
| 32 | +} | |
| 33 | + | |
| 34 | +/// Nothing went to FlutterError while that frame was built. | |
| 35 | +void expectLaidOut(WidgetTester tester, String what) { | |
| 36 | + final e = tester.takeException(); | |
| 37 | + expect(e, isNull, reason: '$what reported: $e'); | |
| 38 | +} | |
| 39 | + | |
| 40 | +void main() { | |
| 41 | + // No offline guard needed: `demoUi` sets the state directly and none of | |
| 42 | + // these dispatch `connect`, so nothing here opens a socket. | |
| 43 | + | |
| 44 | + group('the connect screen', () { | |
| 45 | + for (final entry in sizes.entries) { | |
| 46 | + testWidgets('lays out at ${entry.key}', (tester) async { | |
| 47 | + core.resetUi(); | |
| 48 | + await layOut(tester, entry.value); | |
| 49 | + expectLaidOut(tester, 'connect at ${entry.key}'); | |
| 50 | + }); | |
| 51 | + } | |
| 52 | + | |
| 53 | + testWidgets('lays out in every auth mode', (tester) async { | |
| 54 | + for (final mode in ['guest', 'bluesky', 'app-password']) { | |
| 55 | + core.resetUi(); | |
| 56 | + core.dispatch('mode.$mode'); | |
| 57 | + await layOut(tester, sizes['phone']!); | |
| 58 | + expectLaidOut(tester, 'connect in $mode'); | |
| 59 | + } | |
| 60 | + }); | |
| 61 | + }); | |
| 62 | + | |
| 63 | + group('the chat screen', () { | |
| 64 | + // The one that was never laid out by anything automated. | |
| 65 | + for (final entry in sizes.entries) { | |
| 66 | + testWidgets('lays out at ${entry.key}', (tester) async { | |
| 67 | + core.demoUi(); | |
| 68 | + await layOut(tester, entry.value); | |
| 69 | + expectLaidOut(tester, 'chat at ${entry.key}'); | |
| 70 | + }); | |
| 71 | + } | |
| 72 | + | |
| 73 | + testWidgets('renders the conversation it was given', (tester) async { | |
| 74 | + core.demoUi(); | |
| 75 | + await layOut(tester, sizes['desktop']!); | |
| 76 | + expect(find.text('hello there'), findsOneWidget); | |
| 77 | + expect(find.text('#test'), findsWidgets); | |
| 78 | + expectLaidOut(tester, 'chat content'); | |
| 79 | + }); | |
| 80 | + | |
| 81 | + testWidgets('lays out with the people panel up', (tester) async { | |
| 82 | + core.demoUi(); | |
| 83 | + core.dispatch('users.toggle'); | |
| 84 | + await layOut(tester, sizes['desktop']!); | |
| 85 | + expectLaidOut(tester, 'chat with people'); | |
| 86 | + }); | |
| 87 | + | |
| 88 | + testWidgets('lays out with every compose banner showing', (tester) async { | |
| 89 | + core.demoUi(); | |
| 90 | + core.dispatch('reply.to:2'); | |
| 91 | + await layOut(tester, sizes['phone']!); | |
| 92 | + expectLaidOut(tester, 'chat replying'); | |
| 93 | + | |
| 94 | + core.demoUi(); | |
| 95 | + core.dispatch('edit.start:7'); | |
| 96 | + await layOut(tester, sizes['phone']!); | |
| 97 | + expectLaidOut(tester, 'chat editing'); | |
| 98 | + }); | |
| 99 | + | |
| 100 | + testWidgets('lays out when scrolled off the present', (tester) async { | |
| 101 | + core.demoUi(); | |
| 102 | + core.dispatch('jump.present'); | |
| 103 | + await layOut(tester, sizes['phone']!); | |
| 104 | + expectLaidOut(tester, 'chat jumping'); | |
| 105 | + }); | |
| 106 | + }); | |
| 107 | + | |
| 108 | + group('the chats list', () { | |
| 109 | + for (final entry in sizes.entries) { | |
| 110 | + testWidgets('lays out at ${entry.key}', (tester) async { | |
| 111 | + core.demoUi(); | |
| 112 | + core.dispatch('screen.chats'); | |
| 113 | + await layOut(tester, entry.value); | |
| 114 | + expectLaidOut(tester, 'chats at ${entry.key}'); | |
| 115 | + }); | |
| 116 | + } | |
| 117 | + | |
| 118 | + testWidgets('lays out with a search term in the box', (tester) async { | |
| 119 | + core.demoUi(); | |
| 120 | + core.dispatch('screen.chats'); | |
| 121 | + core.dispatch('search.change', 'te'); | |
| 122 | + await layOut(tester, sizes['phone']!); | |
| 123 | + expectLaidOut(tester, 'chats searching'); | |
| 124 | + }); | |
| 125 | + }); | |
| 126 | + | |
| 127 | + group('discover and settings', () { | |
| 128 | + for (final screen in ['discover', 'settings']) { | |
| 129 | + for (final entry in sizes.entries) { | |
| 130 | + testWidgets('$screen lays out at ${entry.key}', (tester) async { | |
| 131 | + core.demoUi(); | |
| 132 | + core.dispatch('screen.$screen'); | |
| 133 | + await layOut(tester, entry.value); | |
| 134 | + expectLaidOut(tester, '$screen at ${entry.key}'); | |
| 135 | + }); | |
| 136 | + } | |
| 137 | + } | |
| 138 | + }); | |
| 139 | +} | |
| new file mode 100644 | |||
| @@ -0,0 +1,139 @@ | |||
| 1 | +/// Every screen, laid out for real, at sizes that squeeze. | ||
| 2 | +/// | ||
| 3 | +/// This is the test that was missing. `Cannot hit test a render box that has | ||
| 4 | +/// never been laid out` is what a failed layout looks like from the outside, | ||
| 5 | +/// and nothing automated ever laid the chat screen out — a GUI on Wayland | ||
| 6 | +/// cannot be clicked, so every check stopped at the room list while the | ||
| 7 | +/// biggest screen in the app went out unverified. | ||
| 8 | +/// | ||
| 9 | +/// `tester.takeException()` is the whole point: a layout error is reported to | ||
| 10 | +/// FlutterError rather than thrown at the caller, so a test that only pumps | ||
| 11 | +/// and asserts on widgets passes while the screen is broken. These fail. | ||
| 12 | +/// | ||
| 13 | +/// just test layout | ||
| 14 | +import 'package:flutter/material.dart'; | ||
| 15 | +import 'package:flutter_test/flutter_test.dart'; | ||
| 16 | +import 'package:frq_core/frq_core.dart' as core; | ||
| 17 | +import 'package:cljd_flutter/nim_renderer.dart'; | ||
| 18 | + | ||
| 19 | +/// Phone, small desktop, and a deliberately cramped one. The head row of the | ||
| 20 | +/// chat screen asks for more than 360 points has, which is why it wraps. | ||
| 21 | +const sizes = <String, Size>{ | ||
| 22 | + 'phone': Size(360, 690), | ||
| 23 | + 'desktop': Size(1280, 800), | ||
| 24 | + 'cramped': Size(300, 500), | ||
| 25 | +}; | ||
| 26 | + | ||
| 27 | +Future<void> layOut(WidgetTester tester, Size size) async { | ||
| 28 | + await tester.binding.setSurfaceSize(size); | ||
| 29 | + addTearDown(() => tester.binding.setSurfaceSize(null)); | ||
| 30 | + await tester.pumpWidget(const NimApp()); | ||
| 31 | + await tester.pump(); | ||
| 32 | +} | ||
| 33 | + | ||
| 34 | +/// Nothing went to FlutterError while that frame was built. | ||
| 35 | +void expectLaidOut(WidgetTester tester, String what) { | ||
| 36 | + final e = tester.takeException(); | ||
| 37 | + expect(e, isNull, reason: '$what reported: $e'); | ||
| 38 | +} | ||
| 39 | + | ||
| 40 | +void main() { | ||
| 41 | + // No offline guard needed: `demoUi` sets the state directly and none of | ||
| 42 | + // these dispatch `connect`, so nothing here opens a socket. | ||
| 43 | + | ||
| 44 | + group('the connect screen', () { | ||
| 45 | + for (final entry in sizes.entries) { | ||
| 46 | + testWidgets('lays out at ${entry.key}', (tester) async { | ||
| 47 | + core.resetUi(); | ||
| 48 | + await layOut(tester, entry.value); | ||
| 49 | + expectLaidOut(tester, 'connect at ${entry.key}'); | ||
| 50 | + }); | ||
| 51 | + } | ||
| 52 | + | ||
| 53 | + testWidgets('lays out in every auth mode', (tester) async { | ||
| 54 | + for (final mode in ['guest', 'bluesky', 'app-password']) { | ||
| 55 | + core.resetUi(); | ||
| 56 | + core.dispatch('mode.$mode'); | ||
| 57 | + await layOut(tester, sizes['phone']!); | ||
| 58 | + expectLaidOut(tester, 'connect in $mode'); | ||
| 59 | + } | ||
| 60 | + }); | ||
| 61 | + }); | ||
| 62 | + | ||
| 63 | + group('the chat screen', () { | ||
| 64 | + // The one that was never laid out by anything automated. | ||
| 65 | + for (final entry in sizes.entries) { | ||
| 66 | + testWidgets('lays out at ${entry.key}', (tester) async { | ||
| 67 | + core.demoUi(); | ||
| 68 | + await layOut(tester, entry.value); | ||
| 69 | + expectLaidOut(tester, 'chat at ${entry.key}'); | ||
| 70 | + }); | ||
| 71 | + } | ||
| 72 | + | ||
| 73 | + testWidgets('renders the conversation it was given', (tester) async { | ||
| 74 | + core.demoUi(); | ||
| 75 | + await layOut(tester, sizes['desktop']!); | ||
| 76 | + expect(find.text('hello there'), findsOneWidget); | ||
| 77 | + expect(find.text('#test'), findsWidgets); | ||
| 78 | + expectLaidOut(tester, 'chat content'); | ||
| 79 | + }); | ||
| 80 | + | ||
| 81 | + testWidgets('lays out with the people panel up', (tester) async { | ||
| 82 | + core.demoUi(); | ||
| 83 | + core.dispatch('users.toggle'); | ||
| 84 | + await layOut(tester, sizes['desktop']!); | ||
| 85 | + expectLaidOut(tester, 'chat with people'); | ||
| 86 | + }); | ||
| 87 | + | ||
| 88 | + testWidgets('lays out with every compose banner showing', (tester) async { | ||
| 89 | + core.demoUi(); | ||
| 90 | + core.dispatch('reply.to:2'); | ||
| 91 | + await layOut(tester, sizes['phone']!); | ||
| 92 | + expectLaidOut(tester, 'chat replying'); | ||
| 93 | + | ||
| 94 | + core.demoUi(); | ||
| 95 | + core.dispatch('edit.start:7'); | ||
| 96 | + await layOut(tester, sizes['phone']!); | ||
| 97 | + expectLaidOut(tester, 'chat editing'); | ||
| 98 | + }); | ||
| 99 | + | ||
| 100 | + testWidgets('lays out when scrolled off the present', (tester) async { | ||
| 101 | + core.demoUi(); | ||
| 102 | + core.dispatch('jump.present'); | ||
| 103 | + await layOut(tester, sizes['phone']!); | ||
| 104 | + expectLaidOut(tester, 'chat jumping'); | ||
| 105 | + }); | ||
| 106 | + }); | ||
| 107 | + | ||
| 108 | + group('the chats list', () { | ||
| 109 | + for (final entry in sizes.entries) { | ||
| 110 | + testWidgets('lays out at ${entry.key}', (tester) async { | ||
| 111 | + core.demoUi(); | ||
| 112 | + core.dispatch('screen.chats'); | ||
| 113 | + await layOut(tester, entry.value); | ||
| 114 | + expectLaidOut(tester, 'chats at ${entry.key}'); | ||
| 115 | + }); | ||
| 116 | + } | ||
| 117 | + | ||
| 118 | + testWidgets('lays out with a search term in the box', (tester) async { | ||
| 119 | + core.demoUi(); | ||
| 120 | + core.dispatch('screen.chats'); | ||
| 121 | + core.dispatch('search.change', 'te'); | ||
| 122 | + await layOut(tester, sizes['phone']!); | ||
| 123 | + expectLaidOut(tester, 'chats searching'); | ||
| 124 | + }); | ||
| 125 | + }); | ||
| 126 | + | ||
| 127 | + group('discover and settings', () { | ||
| 128 | + for (final screen in ['discover', 'settings']) { | ||
| 129 | + for (final entry in sizes.entries) { | ||
| 130 | + testWidgets('$screen lays out at ${entry.key}', (tester) async { | ||
| 131 | + core.demoUi(); | ||
| 132 | + core.dispatch('screen.$screen'); | ||
| 133 | + await layOut(tester, entry.value); | ||
| 134 | + expectLaidOut(tester, '$screen at ${entry.key}'); | ||
| 135 | + }); | ||
| 136 | + } | ||
| 137 | + } | ||
| 138 | + }); | ||
| 139 | +} | ||
modified
justfile +10 -2 | @@ -104,8 +104,11 @@ test suite="all" *args: | ||
| 104 | 104 | cd "{{root}}" |
| 105 | 105 | shift |
| 106 | 106 | case "{{suite}}" in |
| 107 | - all) just test common && just test nim && just test dart ;; | |
| 107 | + all) just test common && just test nim && just test dart \ | |
| 108 | + && just test layout ;; | |
| 108 | 109 | common) exec python3 tools/check-common.py common ;; |
| 110 | + layout) just _nim-lib | |
| 111 | + just _flutter layout test ;; | |
| 109 | 112 | nim) just _nim-test "$@" ;; |
| 110 | 113 | dart) just _nim-lib |
| 111 | 114 | exec "{{tc}}" exec -- bash -c \ |
| @@ -227,7 +230,7 @@ _flutter target action: | ||
| 227 | 230 | cd "{{root}}" |
| 228 | 231 | case "{{target}}" in |
| 229 | 232 | apk) "{{tc}}" android ;; |
| 230 | - ui|app) just _nim-lib ;; | |
| 233 | + ui|app|layout) just _nim-lib ;; | |
| 231 | 234 | esac |
| 232 | 235 | exec "{{tc}}" exec -- bash -euo pipefail -c ' |
| 233 | 236 | cd flutter |
| @@ -258,4 +261,9 @@ _flutter target action: | ||
| 258 | 261 | exec flutter build linux --debug -t lib/main_nim_app.dart ;; |
| 259 | 262 | app:run) flutter pub get; cljd frq.main-nim |
| 260 | 263 | exec flutter run -d linux -t lib/main_nim_app.dart ;; |
| 264 | + # Widget tests, which lay every screen out for real. Headless: no | |
| 265 | + # GL, no window, which is what makes them the check a Wayland | |
| 266 | + # window cannot be. | |
| 267 | + layout:test) flutter pub get | |
| 268 | + exec flutter test test/nim_layout_test.dart ;; | |
| 261 | 269 | esac' _ "{{target}}" "{{action}}" |
| @@ -104,8 +104,11 @@ test suite="all" *args: | |||
| 104 | cd "{{root}}" | 104 | cd "{{root}}" |
| 105 | shift | 105 | shift |
| 106 | case "{{suite}}" in | 106 | case "{{suite}}" in |
| 107 | - all) just test common && just test nim && just test dart ;; | 107 | + all) just test common && just test nim && just test dart \ |
| 108 | + && just test layout ;; | ||
| 108 | common) exec python3 tools/check-common.py common ;; | 109 | common) exec python3 tools/check-common.py common ;; |
| 110 | + layout) just _nim-lib | ||
| 111 | + just _flutter layout test ;; | ||
| 109 | nim) just _nim-test "$@" ;; | 112 | nim) just _nim-test "$@" ;; |
| 110 | dart) just _nim-lib | 113 | dart) just _nim-lib |
| 111 | exec "{{tc}}" exec -- bash -c \ | 114 | exec "{{tc}}" exec -- bash -c \ |
| @@ -227,7 +230,7 @@ _flutter target action: | |||
| 227 | cd "{{root}}" | 230 | cd "{{root}}" |
| 228 | case "{{target}}" in | 231 | case "{{target}}" in |
| 229 | apk) "{{tc}}" android ;; | 232 | apk) "{{tc}}" android ;; |
| 230 | - ui|app) just _nim-lib ;; | 233 | + ui|app|layout) just _nim-lib ;; |
| 231 | esac | 234 | esac |
| 232 | exec "{{tc}}" exec -- bash -euo pipefail -c ' | 235 | exec "{{tc}}" exec -- bash -euo pipefail -c ' |
| 233 | cd flutter | 236 | cd flutter |
| @@ -258,4 +261,9 @@ _flutter target action: | |||
| 258 | exec flutter build linux --debug -t lib/main_nim_app.dart ;; | 261 | exec flutter build linux --debug -t lib/main_nim_app.dart ;; |
| 259 | app:run) flutter pub get; cljd frq.main-nim | 262 | app:run) flutter pub get; cljd frq.main-nim |
| 260 | exec flutter run -d linux -t lib/main_nim_app.dart ;; | 263 | exec flutter run -d linux -t lib/main_nim_app.dart ;; |
| 264 | + # Widget tests, which lay every screen out for real. Headless: no | ||
| 265 | + # GL, no window, which is what makes them the check a Wayland | ||
| 266 | + # window cannot be. | ||
| 267 | + layout:test) flutter pub get | ||
| 268 | + exec flutter test test/nim_layout_test.dart ;; | ||
| 261 | esac' _ "{{target}}" "{{action}}" | 269 | esac' _ "{{target}}" "{{action}}" |
modified
nim/src/frq_core.nim +44 -2 | @@ -25,8 +25,8 @@ | ||
| 25 | 25 | ## from `common/frq/screens/` rather than reimagined, which is the difference |
| 26 | 26 | ## between this and the experiment that was deleted for being a facsimile. |
| 27 | 27 | |
| 28 | -import std/[json, strutils] | |
| 29 | -import frq/[ircparse, trace, ui, cells, reducer] | |
| 28 | +import std/[json, strutils, tables] | |
| 29 | +import frq/[ircparse, trace, ui, cells, reducer, model, rooms, reactions] | |
| 30 | 30 | import frq/conn as tr |
| 31 | 31 | import frq/screens/connect as scConnectScreen |
| 32 | 32 | import frq/screens/chats as scChatsScreen |
| @@ -192,6 +192,48 @@ proc frq_ui_poll*(): cstring {.exportc, dynlib.} = | ||
| 192 | 192 | ## anything happened. Same work as render; named for what the caller means. |
| 193 | 193 | dup(currentTree()) |
| 194 | 194 | |
| 195 | +proc frq_ui_demo*() {.exportc, dynlib.} = | |
| 196 | + ## Fill a room with a representative conversation, for a test that wants to | |
| 197 | + ## lay the chat screen out without a server. | |
| 198 | + ## | |
| 199 | + ## It exists because the chat screen is the one a script could not reach: a | |
| 200 | + ## GUI on Wayland cannot be clicked, so every automated check stopped at the | |
| 201 | + ## room list and the biggest screen in the app went out unlaid-out. The | |
| 202 | + ## content is chosen to be awkward on purpose — a long unbroken URL, a very | |
| 203 | + ## long word, an image, reactions, a reply, a system line, an edited line — | |
| 204 | + ## because a layout bug is about what does not fit. | |
| 205 | + app = initState() | |
| 206 | + app.formNick = "me" | |
| 207 | + app.rooms.ensureRoom("#test") | |
| 208 | + var r = app.rooms["#test"] | |
| 209 | + r.joined = true | |
| 210 | + r.users = @["me", "alice", "bob"] | |
| 211 | + r.topic = "a room" | |
| 212 | + let t0 = 1_700_000_000_000'i64 | |
| 213 | + r.messages = @[ | |
| 214 | + Message(id: "1", frm: "*", text: "me joined #test", at: t0, system: true), | |
| 215 | + Message(id: "2", frm: "alice", text: "hello there", at: t0 + 1000), | |
| 216 | + Message(id: "3", frm: "bob", | |
| 217 | + text: "see https://example.com/a/very/long/path/that/will/not/wrap/anywhere/at/all?q=1 for more", | |
| 218 | + at: t0 + 2000), | |
| 219 | + Message(id: "4", frm: "alice", | |
| 220 | + text: "Supercalifragilisticexpialidociousssssssssssssssssssssssssssssssssssss", | |
| 221 | + at: t0 + 3000), | |
| 222 | + Message(id: "5", frm: "me", text: "a picture", at: t0 + 4000, | |
| 223 | + imageUrl: "https://example.com/a.png"), | |
| 224 | + Message(id: "6", frm: "bob", text: "answering you", at: t0 + 5000, | |
| 225 | + replyTo: "5"), | |
| 226 | + Message(id: "7", frm: "me", text: "edited line", at: t0 + 6000, | |
| 227 | + edited: true, | |
| 228 | + reactions: @[Reaction(emoji: "👍", nicks: @["me", "alice"]), | |
| 229 | + Reaction(emoji: "🎉", nicks: @["bob"])]), | |
| 230 | + # A different day, so a heading has to land between them. | |
| 231 | + Message(id: "8", frm: "alice", text: "next day", at: t0 + 200_000_000)] | |
| 232 | + app.rooms["#test"] = r | |
| 233 | + app.current = "#test" | |
| 234 | + app.screen = scChat | |
| 235 | + app.status = "Connected as me" | |
| 236 | + | |
| 195 | 237 | proc frq_ui_reset*() {.exportc, dynlib.} = |
| 196 | 238 | tr.close() |
| 197 | 239 | app = initState() |
| @@ -25,8 +25,8 @@ | |||
| 25 | ## from `common/frq/screens/` rather than reimagined, which is the difference | 25 | ## from `common/frq/screens/` rather than reimagined, which is the difference |
| 26 | ## between this and the experiment that was deleted for being a facsimile. | 26 | ## between this and the experiment that was deleted for being a facsimile. |
| 27 | 27 | ||
| 28 | -import std/[json, strutils] | 28 | +import std/[json, strutils, tables] |
| 29 | -import frq/[ircparse, trace, ui, cells, reducer] | 29 | +import frq/[ircparse, trace, ui, cells, reducer, model, rooms, reactions] |
| 30 | import frq/conn as tr | 30 | import frq/conn as tr |
| 31 | import frq/screens/connect as scConnectScreen | 31 | import frq/screens/connect as scConnectScreen |
| 32 | import frq/screens/chats as scChatsScreen | 32 | import frq/screens/chats as scChatsScreen |
| @@ -192,6 +192,48 @@ proc frq_ui_poll*(): cstring {.exportc, dynlib.} = | |||
| 192 | ## anything happened. Same work as render; named for what the caller means. | 192 | ## anything happened. Same work as render; named for what the caller means. |
| 193 | dup(currentTree()) | 193 | dup(currentTree()) |
| 194 | 194 | ||
| 195 | +proc frq_ui_demo*() {.exportc, dynlib.} = | ||
| 196 | + ## Fill a room with a representative conversation, for a test that wants to | ||
| 197 | + ## lay the chat screen out without a server. | ||
| 198 | + ## | ||
| 199 | + ## It exists because the chat screen is the one a script could not reach: a | ||
| 200 | + ## GUI on Wayland cannot be clicked, so every automated check stopped at the | ||
| 201 | + ## room list and the biggest screen in the app went out unlaid-out. The | ||
| 202 | + ## content is chosen to be awkward on purpose — a long unbroken URL, a very | ||
| 203 | + ## long word, an image, reactions, a reply, a system line, an edited line — | ||
| 204 | + ## because a layout bug is about what does not fit. | ||
| 205 | + app = initState() | ||
| 206 | + app.formNick = "me" | ||
| 207 | + app.rooms.ensureRoom("#test") | ||
| 208 | + var r = app.rooms["#test"] | ||
| 209 | + r.joined = true | ||
| 210 | + r.users = @["me", "alice", "bob"] | ||
| 211 | + r.topic = "a room" | ||
| 212 | + let t0 = 1_700_000_000_000'i64 | ||
| 213 | + r.messages = @[ | ||
| 214 | + Message(id: "1", frm: "*", text: "me joined #test", at: t0, system: true), | ||
| 215 | + Message(id: "2", frm: "alice", text: "hello there", at: t0 + 1000), | ||
| 216 | + Message(id: "3", frm: "bob", | ||
| 217 | + text: "see https://example.com/a/very/long/path/that/will/not/wrap/anywhere/at/all?q=1 for more", | ||
| 218 | + at: t0 + 2000), | ||
| 219 | + Message(id: "4", frm: "alice", | ||
| 220 | + text: "Supercalifragilisticexpialidociousssssssssssssssssssssssssssssssssssss", | ||
| 221 | + at: t0 + 3000), | ||
| 222 | + Message(id: "5", frm: "me", text: "a picture", at: t0 + 4000, | ||
| 223 | + imageUrl: "https://example.com/a.png"), | ||
| 224 | + Message(id: "6", frm: "bob", text: "answering you", at: t0 + 5000, | ||
| 225 | + replyTo: "5"), | ||
| 226 | + Message(id: "7", frm: "me", text: "edited line", at: t0 + 6000, | ||
| 227 | + edited: true, | ||
| 228 | + reactions: @[Reaction(emoji: "👍", nicks: @["me", "alice"]), | ||
| 229 | + Reaction(emoji: "🎉", nicks: @["bob"])]), | ||
| 230 | + # A different day, so a heading has to land between them. | ||
| 231 | + Message(id: "8", frm: "alice", text: "next day", at: t0 + 200_000_000)] | ||
| 232 | + app.rooms["#test"] = r | ||
| 233 | + app.current = "#test" | ||
| 234 | + app.screen = scChat | ||
| 235 | + app.status = "Connected as me" | ||
| 236 | + | ||
| 195 | proc frq_ui_reset*() {.exportc, dynlib.} = | 237 | proc frq_ui_reset*() {.exportc, dynlib.} = |
| 196 | tr.close() | 238 | tr.close() |
| 197 | app = initState() | 239 | app = initState() |