A picture fills the window, the box fills the line
Four things, and the last one is the answer to a question I got wrong twice. Clicking a picture already opened a lightbox — the event, the state and the pane were all wired. What it opened was a card in the column with the picture capped at 640 by 480, which on a short window is a thumbnail-and-a-half below the backlog, off the bottom, and not what anyone means by full size. It covers the conversation now: `overlay` learned a second kind of floating child, one that fills rather than sitting at the foot, and `image` learned `expand` so the picture takes the height the panel has. The message box was pinned to 260 points in a Wrap, which is a phone's box on a window four times that with the rest of the line empty beside it. The compose bar is a row now and the box takes what the picture button and Send leave. `Open on bsky.app` was underlined, blue and inert, for exactly the reason the links in a message were: `_wrapTap` hands back a bare child when there is no event to send, and a link's event was never the point. I fixed the inline case yesterday and missed this one, which is the same fix in the other of the two places a link is built. And the pencil. `✏️` is U+270F plus a variation selector *asking* for emoji presentation, and the ask is not binding — CanvasKit answered it out of `notosanssymbols2`, which I watched it fetch. Naming the colour face does not help in a browser: it has none to name, and a named family that is missing is a notdef box. I tried that twice, the second time on the theory that the first result came through a stale service worker. It did not. So the chips use codepoints no text font claims: 📝 where `✏️` was, 💬 where `↩️` was. Both come out in colour, on both hosts, with no font to name — CanvasKit fetches Noto Color Emoji in slices, so one may be a box for a moment before its slice lands. What this does not fix is a reaction somebody else sent. `❤️` carries the same selector and will draw grey on the web until there is a colour emoji face in the bundle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
94d9710 parent: 299008f modified
flutter/lib/nim_renderer.dart +56 -6 | @@ -20,6 +20,19 @@ import 'src/host.dart' as host; | ||
| 20 | 20 | |
| 21 | 21 | import 'nim_theme.dart' as t; |
| 22 | 22 | |
| 23 | +/// A child of an `overlay` that covers it rather than floating at its foot. | |
| 24 | +/// | |
| 25 | +/// A widget rather than a flag, because the thing that has to act on it is | |
| 26 | +/// the `Stack` two levels up, and a tree is the only channel between them. | |
| 27 | +/// It is never built: the overlay unwraps it and uses `child`. | |
| 28 | +class _Filling extends StatelessWidget { | |
| 29 | + const _Filling(this.child); | |
| 30 | + final Widget child; | |
| 31 | + | |
| 32 | + @override | |
| 33 | + Widget build(BuildContext context) => child; | |
| 34 | +} | |
| 35 | + | |
| 23 | 36 | class NimApp extends StatefulWidget { |
| 24 | 37 | const NimApp({super.key}); |
| 25 | 38 | @override |
| @@ -273,6 +286,14 @@ class _NimAppState extends State<NimApp> { | ||
| 273 | 286 | final key = n.prop('key', ''); |
| 274 | 287 | var w = _buildNode(n, axis); |
| 275 | 288 | |
| 289 | + // `_Filling` has to stay outermost: the `Stack` in `overlay` looks for it | |
| 290 | + // by type, and a `KeyedSubtree` around it is a `KeyedSubtree` as far as | |
| 291 | + // that check is concerned — which is how the first version of this ended | |
| 292 | + // up laying a flex child out against an unbounded height. | |
| 293 | + if (w is _Filling && key.isNotEmpty) { | |
| 294 | + return _Filling(KeyedSubtree(key: ValueKey(key), child: w.child)); | |
| 295 | + } | |
| 296 | + | |
| 276 | 297 | // The row a reply's arrow is aiming at. Two keys on one widget is not a |
| 277 | 298 | // thing, so they nest: the ValueKey keeps the element across rebuilds, |
| 278 | 299 | // and the GlobalKey is how this frame finds it afterwards. |
| @@ -358,6 +379,13 @@ class _NimAppState extends State<NimApp> { | ||
| 358 | 379 | children: [ |
| 359 | 380 | Positioned.fill(child: base), |
| 360 | 381 | for (final o in over) |
| 382 | + // Two kinds of floating child. One sits at the bottom on its | |
| 383 | + // own ground — a control over the conversation. The other | |
| 384 | + // covers it: a picture being looked at is not something to | |
| 385 | + // read the backlog through. | |
| 386 | + if (o is _Filling) | |
| 387 | + Positioned.fill(child: o.child) | |
| 388 | + else | |
| 361 | 389 | Positioned( |
| 362 | 390 | left: 0, |
| 363 | 391 | right: 0, |
| @@ -390,6 +418,15 @@ class _NimAppState extends State<NimApp> { | ||
| 390 | 418 | col = _margins(n, col); |
| 391 | 419 | final w = _d(n.props['widthRequest'], 0); |
| 392 | 420 | if (w > 0) col = SizedBox(width: w, child: col); |
| 421 | + // A panel that covers something needs a ground of its own, or what | |
| 422 | + // it covers reads through it. | |
| 423 | + if (n.prop('background', false)) { | |
| 424 | + col = ColoredBox(color: t.bg, child: col); | |
| 425 | + } | |
| 426 | + // Said by a child of an `overlay`: fill it rather than float at the | |
| 427 | + // bottom of it. Carried as a wrapper because the Stack above is the | |
| 428 | + // only thing that can act on it. | |
| 429 | + if (n.prop('fill', false)) return _Filling(col); | |
| 393 | 430 | return expanded(col); |
| 394 | 431 | } |
| 395 | 432 | |
| @@ -482,15 +519,25 @@ class _NimAppState extends State<NimApp> { | ||
| 482 | 519 | return Text(n.prop('text', ''), style: _style(t.textBody, t.onBg)); |
| 483 | 520 | |
| 484 | 521 | case 'link': |
| 485 | - return _wrapTap( | |
| 486 | - n.prop('onClick', ''), | |
| 487 | - Text( | |
| 522 | + { | |
| 523 | + // The same rule the inline links follow: a link with no `onClick` | |
| 524 | + // opens its own `url`. `Open on bsky.app` in the profile panel is | |
| 525 | + // this one, and it was underlined, blue and inert for exactly the | |
| 526 | + // reason the ones in a message were — `_wrapTap` hands back a bare | |
| 527 | + // child when there is no event to send, and a link's event was | |
| 528 | + // never the point. Opening a URL is the platform's job. | |
| 529 | + final url = n.prop('url', ''); | |
| 530 | + final onClick = n.prop('onClick', ''); | |
| 531 | + final label = Text( | |
| 488 | 532 | n.prop('label', ''), |
| 489 | 533 | style: _style(t.textBody, t.accent) |
| 490 | 534 | .copyWith(decoration: TextDecoration.underline, |
| 491 | 535 | decorationColor: t.accent), |
| 492 | - ), | |
| 493 | - ); | |
| 536 | + ); | |
| 537 | + if (onClick.isNotEmpty) return _wrapTap(onClick, label); | |
| 538 | + if (url.isEmpty) return label; | |
| 539 | + return InkWell(onTap: () => host.openUrl(url), child: label); | |
| 540 | + } | |
| 494 | 541 | |
| 495 | 542 | case 'separator': |
| 496 | 543 | return const Divider(height: 1, thickness: 1, color: t.divider); |
| @@ -726,7 +773,10 @@ class _NimAppState extends State<NimApp> { | ||
| 726 | 773 | child: img, |
| 727 | 774 | ); |
| 728 | 775 | } |
| 729 | - return _wrapTap(n.prop('onClick', ''), img); | |
| 776 | + // `expand` on a picture is what makes a lightbox a lightbox: in | |
| 777 | + // the column of a panel that fills the screen, it takes the height | |
| 778 | + // that is left and `BoxFit.contain` does the rest. | |
| 779 | + return expanded(_wrapTap(n.prop('onClick', ''), img)); | |
| 730 | 780 | } |
| 731 | 781 | |
| 732 | 782 | case 'entry': |
| @@ -20,6 +20,19 @@ import 'src/host.dart' as host; | |||
| 20 | 20 | ||
| 21 | import 'nim_theme.dart' as t; | 21 | import 'nim_theme.dart' as t; |
| 22 | 22 | ||
| 23 | +/// A child of an `overlay` that covers it rather than floating at its foot. | ||
| 24 | +/// | ||
| 25 | +/// A widget rather than a flag, because the thing that has to act on it is | ||
| 26 | +/// the `Stack` two levels up, and a tree is the only channel between them. | ||
| 27 | +/// It is never built: the overlay unwraps it and uses `child`. | ||
| 28 | +class _Filling extends StatelessWidget { | ||
| 29 | + const _Filling(this.child); | ||
| 30 | + final Widget child; | ||
| 31 | + | ||
| 32 | + @override | ||
| 33 | + Widget build(BuildContext context) => child; | ||
| 34 | +} | ||
| 35 | + | ||
| 23 | class NimApp extends StatefulWidget { | 36 | class NimApp extends StatefulWidget { |
| 24 | const NimApp({super.key}); | 37 | const NimApp({super.key}); |
| 25 | @override | 38 | @override |
| @@ -273,6 +286,14 @@ class _NimAppState extends State<NimApp> { | |||
| 273 | final key = n.prop('key', ''); | 286 | final key = n.prop('key', ''); |
| 274 | var w = _buildNode(n, axis); | 287 | var w = _buildNode(n, axis); |
| 275 | 288 | ||
| 289 | + // `_Filling` has to stay outermost: the `Stack` in `overlay` looks for it | ||
| 290 | + // by type, and a `KeyedSubtree` around it is a `KeyedSubtree` as far as | ||
| 291 | + // that check is concerned — which is how the first version of this ended | ||
| 292 | + // up laying a flex child out against an unbounded height. | ||
| 293 | + if (w is _Filling && key.isNotEmpty) { | ||
| 294 | + return _Filling(KeyedSubtree(key: ValueKey(key), child: w.child)); | ||
| 295 | + } | ||
| 296 | + | ||
| 276 | // The row a reply's arrow is aiming at. Two keys on one widget is not a | 297 | // The row a reply's arrow is aiming at. Two keys on one widget is not a |
| 277 | // thing, so they nest: the ValueKey keeps the element across rebuilds, | 298 | // thing, so they nest: the ValueKey keeps the element across rebuilds, |
| 278 | // and the GlobalKey is how this frame finds it afterwards. | 299 | // and the GlobalKey is how this frame finds it afterwards. |
| @@ -358,6 +379,13 @@ class _NimAppState extends State<NimApp> { | |||
| 358 | children: [ | 379 | children: [ |
| 359 | Positioned.fill(child: base), | 380 | Positioned.fill(child: base), |
| 360 | for (final o in over) | 381 | for (final o in over) |
| 382 | + // Two kinds of floating child. One sits at the bottom on its | ||
| 383 | + // own ground — a control over the conversation. The other | ||
| 384 | + // covers it: a picture being looked at is not something to | ||
| 385 | + // read the backlog through. | ||
| 386 | + if (o is _Filling) | ||
| 387 | + Positioned.fill(child: o.child) | ||
| 388 | + else | ||
| 361 | Positioned( | 389 | Positioned( |
| 362 | left: 0, | 390 | left: 0, |
| 363 | right: 0, | 391 | right: 0, |
| @@ -390,6 +418,15 @@ class _NimAppState extends State<NimApp> { | |||
| 390 | col = _margins(n, col); | 418 | col = _margins(n, col); |
| 391 | final w = _d(n.props['widthRequest'], 0); | 419 | final w = _d(n.props['widthRequest'], 0); |
| 392 | if (w > 0) col = SizedBox(width: w, child: col); | 420 | if (w > 0) col = SizedBox(width: w, child: col); |
| 421 | + // A panel that covers something needs a ground of its own, or what | ||
| 422 | + // it covers reads through it. | ||
| 423 | + if (n.prop('background', false)) { | ||
| 424 | + col = ColoredBox(color: t.bg, child: col); | ||
| 425 | + } | ||
| 426 | + // Said by a child of an `overlay`: fill it rather than float at the | ||
| 427 | + // bottom of it. Carried as a wrapper because the Stack above is the | ||
| 428 | + // only thing that can act on it. | ||
| 429 | + if (n.prop('fill', false)) return _Filling(col); | ||
| 393 | return expanded(col); | 430 | return expanded(col); |
| 394 | } | 431 | } |
| 395 | 432 | ||
| @@ -482,15 +519,25 @@ class _NimAppState extends State<NimApp> { | |||
| 482 | return Text(n.prop('text', ''), style: _style(t.textBody, t.onBg)); | 519 | return Text(n.prop('text', ''), style: _style(t.textBody, t.onBg)); |
| 483 | 520 | ||
| 484 | case 'link': | 521 | case 'link': |
| 485 | - return _wrapTap( | 522 | + { |
| 486 | - n.prop('onClick', ''), | 523 | + // The same rule the inline links follow: a link with no `onClick` |
| 487 | - Text( | 524 | + // opens its own `url`. `Open on bsky.app` in the profile panel is |
| 525 | + // this one, and it was underlined, blue and inert for exactly the | ||
| 526 | + // reason the ones in a message were — `_wrapTap` hands back a bare | ||
| 527 | + // child when there is no event to send, and a link's event was | ||
| 528 | + // never the point. Opening a URL is the platform's job. | ||
| 529 | + final url = n.prop('url', ''); | ||
| 530 | + final onClick = n.prop('onClick', ''); | ||
| 531 | + final label = Text( | ||
| 488 | n.prop('label', ''), | 532 | n.prop('label', ''), |
| 489 | style: _style(t.textBody, t.accent) | 533 | style: _style(t.textBody, t.accent) |
| 490 | .copyWith(decoration: TextDecoration.underline, | 534 | .copyWith(decoration: TextDecoration.underline, |
| 491 | decorationColor: t.accent), | 535 | decorationColor: t.accent), |
| 492 | - ), | 536 | + ); |
| 493 | - ); | 537 | + if (onClick.isNotEmpty) return _wrapTap(onClick, label); |
| 538 | + if (url.isEmpty) return label; | ||
| 539 | + return InkWell(onTap: () => host.openUrl(url), child: label); | ||
| 540 | + } | ||
| 494 | 541 | ||
| 495 | case 'separator': | 542 | case 'separator': |
| 496 | return const Divider(height: 1, thickness: 1, color: t.divider); | 543 | return const Divider(height: 1, thickness: 1, color: t.divider); |
| @@ -726,7 +773,10 @@ class _NimAppState extends State<NimApp> { | |||
| 726 | child: img, | 773 | child: img, |
| 727 | ); | 774 | ); |
| 728 | } | 775 | } |
| 729 | - return _wrapTap(n.prop('onClick', ''), img); | 776 | + // `expand` on a picture is what makes a lightbox a lightbox: in |
| 777 | + // the column of a panel that fills the screen, it takes the height | ||
| 778 | + // that is left and `BoxFit.contain` does the rest. | ||
| 779 | + return expanded(_wrapTap(n.prop('onClick', ''), img)); | ||
| 730 | } | 780 | } |
| 731 | 781 | ||
| 732 | case 'entry': | 782 | case 'entry': |
modified
flutter/lib/src/host_web.dart +10 -4 | @@ -42,10 +42,16 @@ Widget networkImage(String url, | ||
| 42 | 42 | /// No family is named on the web, and that is the fix rather than the gap. |
| 43 | 43 | /// |
| 44 | 44 | /// A browser is not painting with the machine's fonts: CanvasKit carries its |
| 45 | -/// own, and downloads a Noto face on demand for any glyph it cannot draw — | |
| 46 | -/// emoji included. Naming a family it does not have defeats that, because a | |
| 47 | -/// named family that is missing is a notdef box rather than a search. Every | |
| 48 | -/// reaction chip drew ▯ until this list was empty. | |
| 45 | +/// own and fetches a Noto face on demand for glyphs it cannot draw. Naming a | |
| 46 | +/// family it does not have defeats that — a named family that is missing is | |
| 47 | +/// a notdef box rather than a search, and every chip drew ▯. | |
| 48 | +/// | |
| 49 | +/// That was tried twice. The second time was on the theory that the first | |
| 50 | +/// result had come through a stale service worker; it had not, and the boxes | |
| 51 | +/// came straight back. What a glyph gets here is whatever CanvasKit's own | |
| 52 | +/// fallback finds, which for an emoji-presentation codepoint is the colour | |
| 53 | +/// face — and for a text codepoint wearing a variation selector is not. See | |
| 54 | +/// `screens/chat`, where the chips are chosen accordingly. | |
| 49 | 55 | const List<String> emojiFonts = <String>[]; |
| 50 | 56 | |
| 51 | 57 | @JS('window.open') |
| @@ -42,10 +42,16 @@ Widget networkImage(String url, | |||
| 42 | /// No family is named on the web, and that is the fix rather than the gap. | 42 | /// No family is named on the web, and that is the fix rather than the gap. |
| 43 | /// | 43 | /// |
| 44 | /// A browser is not painting with the machine's fonts: CanvasKit carries its | 44 | /// A browser is not painting with the machine's fonts: CanvasKit carries its |
| 45 | -/// own, and downloads a Noto face on demand for any glyph it cannot draw — | 45 | +/// own and fetches a Noto face on demand for glyphs it cannot draw. Naming a |
| 46 | -/// emoji included. Naming a family it does not have defeats that, because a | 46 | +/// family it does not have defeats that — a named family that is missing is |
| 47 | -/// named family that is missing is a notdef box rather than a search. Every | 47 | +/// a notdef box rather than a search, and every chip drew ▯. |
| 48 | -/// reaction chip drew ▯ until this list was empty. | 48 | +/// |
| 49 | +/// That was tried twice. The second time was on the theory that the first | ||
| 50 | +/// result had come through a stale service worker; it had not, and the boxes | ||
| 51 | +/// came straight back. What a glyph gets here is whatever CanvasKit's own | ||
| 52 | +/// fallback finds, which for an emoji-presentation codepoint is the colour | ||
| 53 | +/// face — and for a text codepoint wearing a variation selector is not. See | ||
| 54 | +/// `screens/chat`, where the chips are chosen accordingly. | ||
| 49 | const List<String> emojiFonts = <String>[]; | 55 | const List<String> emojiFonts = <String>[]; |
| 50 | 56 | ||
| 51 | @JS('window.open') | 57 | @JS('window.open') |
modified
flutter/test/nim_layout_test.dart +27 -0 | @@ -380,6 +380,33 @@ void main() { | ||
| 380 | 380 | }); |
| 381 | 381 | }); |
| 382 | 382 | |
| 383 | + group('the lightbox', () { | |
| 384 | + testWidgets('a picture being looked at covers the conversation', | |
| 385 | + (tester) async { | |
| 386 | + // It used to be a card in the column with the picture capped at 640 by | |
| 387 | + // 480 — a thumbnail-and-a-half below the backlog, off the bottom of a | |
| 388 | + // short window, called full size. | |
| 389 | + core.demoUi(); | |
| 390 | + core.dispatch('window.size', '1280x800'); | |
| 391 | + await layOut(tester, sizes['desktop']!); | |
| 392 | + expect(find.text('Picture'), findsNothing); | |
| 393 | + | |
| 394 | + core.dispatch('lightbox:https://example.com/a.png'); | |
| 395 | + await tester.pump(const Duration(milliseconds: 150)); | |
| 396 | + expectLaidOut(tester, 'the lightbox'); | |
| 397 | + expect(find.text('Picture'), findsOneWidget); | |
| 398 | + | |
| 399 | + // As wide as the conversation it covers, rather than a card in it. | |
| 400 | + final pane = tester.getRect(find.text('Picture').first); | |
| 401 | + expect(pane.left, lessThan(200)); | |
| 402 | + | |
| 403 | + core.dispatch('lightbox.close'); | |
| 404 | + await tester.pump(const Duration(milliseconds: 150)); | |
| 405 | + expect(find.text('Picture'), findsNothing); | |
| 406 | + expectLaidOut(tester, 'the conversation after closing it'); | |
| 407 | + }); | |
| 408 | + }); | |
| 409 | + | |
| 383 | 410 | group('identity', () { |
| 384 | 411 | // Duplicate keys among siblings are an error Flutter throws at build |
| 385 | 412 | // time, so this is mostly a guard on the tree the core emits: every |
| @@ -380,6 +380,33 @@ void main() { | |||
| 380 | }); | 380 | }); |
| 381 | }); | 381 | }); |
| 382 | 382 | ||
| 383 | + group('the lightbox', () { | ||
| 384 | + testWidgets('a picture being looked at covers the conversation', | ||
| 385 | + (tester) async { | ||
| 386 | + // It used to be a card in the column with the picture capped at 640 by | ||
| 387 | + // 480 — a thumbnail-and-a-half below the backlog, off the bottom of a | ||
| 388 | + // short window, called full size. | ||
| 389 | + core.demoUi(); | ||
| 390 | + core.dispatch('window.size', '1280x800'); | ||
| 391 | + await layOut(tester, sizes['desktop']!); | ||
| 392 | + expect(find.text('Picture'), findsNothing); | ||
| 393 | + | ||
| 394 | + core.dispatch('lightbox:https://example.com/a.png'); | ||
| 395 | + await tester.pump(const Duration(milliseconds: 150)); | ||
| 396 | + expectLaidOut(tester, 'the lightbox'); | ||
| 397 | + expect(find.text('Picture'), findsOneWidget); | ||
| 398 | + | ||
| 399 | + // As wide as the conversation it covers, rather than a card in it. | ||
| 400 | + final pane = tester.getRect(find.text('Picture').first); | ||
| 401 | + expect(pane.left, lessThan(200)); | ||
| 402 | + | ||
| 403 | + core.dispatch('lightbox.close'); | ||
| 404 | + await tester.pump(const Duration(milliseconds: 150)); | ||
| 405 | + expect(find.text('Picture'), findsNothing); | ||
| 406 | + expectLaidOut(tester, 'the conversation after closing it'); | ||
| 407 | + }); | ||
| 408 | + }); | ||
| 409 | + | ||
| 383 | group('identity', () { | 410 | group('identity', () { |
| 384 | // Duplicate keys among siblings are an error Flutter throws at build | 411 | // Duplicate keys among siblings are an error Flutter throws at build |
| 385 | // time, so this is mostly a guard on the tree the core emits: every | 412 | // time, so this is mostly a guard on the tree the core emits: every |
modified
nim/src/frq/screens/chat.nim +41 -20 | @@ -37,18 +37,26 @@ func actionChips(m: Message, mine: bool): Node = | ||
| 37 | 37 | ## the line with the text they took width off every line under them and |
| 38 | 38 | ## wrapped a message that had the room to sit on one. |
| 39 | 39 | ## |
| 40 | - ## A row laid out from the right lays its first child furthest right, so | |
| 41 | - ## reacting comes first here and this reads ✏️ then ↩️ then 🙂 on screen. | |
| 42 | - ## Only our own lines carry a pencil: the server refuses an edit of somebody | |
| 43 | - ## else's, and a chip that always fails is a chip that lies. | |
| 40 | + ## Only our own lines carry the edit chip: the server refuses an edit of | |
| 41 | + ## somebody else's, and a chip that always fails is a chip that lies. | |
| 42 | + ## | |
| 43 | + ## The glyphs are all emoji-presentation codepoints, and that is a | |
| 44 | + ## constraint rather than a preference. `✏️` is U+270F plus a variation | |
| 45 | + ## selector *asking* for emoji presentation, and the ask is not binding: | |
| 46 | + ## some font claims the bare U+270F and draws the monochrome pencil the | |
| 47 | + ## text era had. On a desktop that font is DejaVu Sans, which naming the | |
| 48 | + ## colour face gets around; in a browser CanvasKit has no such face to name | |
| 49 | + ## — naming one gives notdef boxes — so the only reliable answer is a | |
| 50 | + ## codepoint no text font claims. Hence 📝 where ✏️ was, and 💬 where ↩️ | |
| 51 | + ## was: both are emoji-only, and both come out in colour everywhere. | |
| 44 | 52 | result = n("hbox", %*{"key": "actions", "align": "end", "spacing": chipGap}, @[ |
| 45 | 53 | n("reaction", %*{"key": "react", "emoji": "🙂", "size": pillSize, |
| 46 | 54 | "onClick": "react.open:" & rowId(m)}), |
| 47 | - n("reaction", %*{"key": "reply", "emoji": "↩️", "size": pillSize, | |
| 55 | + n("reaction", %*{"key": "reply", "emoji": "💬", "size": pillSize, | |
| 48 | 56 | "onClick": "reply.to:" & rowId(m)})]) |
| 49 | 57 | if mine: |
| 50 | 58 | result.children.add n("reaction", |
| 51 | - %*{"key": "edit", "emoji": "✏️", "size": pillSize, | |
| 59 | + %*{"key": "edit", "emoji": "📝", "size": pillSize, | |
| 52 | 60 | "onClick": "edit.start:" & rowId(m)}) |
| 53 | 61 | |
| 54 | 62 | func reactionRow(m: Message, me: string): Node = |
| @@ -58,8 +66,8 @@ func reactionRow(m: Message, me: string): Node = | ||
| 58 | 66 | ## takes yours off, which is the same gesture that put it there. `reaction` |
| 59 | 67 | ## rather than a button with the emoji as its label — the renderer draws a |
| 60 | 68 | ## `reaction` in the colour emoji font, where a label gets whatever ordinary |
| 61 | - ## fallback finds, which for ✏️ and ↩️ is a monochrome glyph out of a text | |
| 62 | - ## font. | |
| 69 | + ## fallback finds — see `actionChips` on why the glyphs here avoid the | |
| 70 | + ## variation-selector kind entirely. | |
| 63 | 71 | result = n("hbox", %*{"key": "pills", "spacing": chipGap}) |
| 64 | 72 | var emojis: seq[string] |
| 65 | 73 | for r in m.reactions: emojis.add r.emoji |
| @@ -296,17 +304,24 @@ proc overviewPane(s: State): Node = | ||
| 296 | 304 | result.children.add body |
| 297 | 305 | |
| 298 | 306 | proc lightboxPane(s: State): Node = |
| 299 | - ## The picture being looked at, full size. | |
| 307 | + ## The picture being looked at, as large as the window allows. | |
| 300 | 308 | ## |
| 301 | 309 | ## A panel over the conversation rather than a screen of its own: closing it |
| 302 | 310 | ## should put the reader back exactly where they were, and a screen would |
| 303 | 311 | ## have to remember where that was. |
| 304 | - card( | |
| 305 | - hbox(%*{"spacing": 8}, | |
| 312 | + ## | |
| 313 | + ## It used to be a card in the column with the picture capped at 640 by 480 | |
| 314 | + ## — which put a thumbnail-and-a-half below the backlog, off the bottom of | |
| 315 | + ## a short window, and called it full size. Now it covers the conversation | |
| 316 | + ## and the picture takes all of it. | |
| 317 | + n("vbox", %*{"key": "lightbox-card", "spacing": 8, "margin": 12, | |
| 318 | + "background": true, "expand": true}, @[ | |
| 319 | + hbox(%*{"spacing": 8, "wrap": false}, | |
| 306 | 320 | title2("Picture"), |
| 307 | - button("Close", "lightbox.close")), | |
| 308 | - image(s.lightbox.url, maxWidth = 640, maxHeight = 480), | |
| 309 | - dimLabel(s.lightbox.url)) | |
| 321 | + n("button", %*{"label": "Close", "onClick": "lightbox.close", | |
| 322 | + "expand": true})), | |
| 323 | + n("image", %*{"src": s.lightbox.url, "expand": true}), | |
| 324 | + dimLabel(s.lightbox.url)]) | |
| 310 | 325 | |
| 311 | 326 | proc profilePane(s: State): Node = |
| 312 | 327 | ## Who someone is, behind the nick on a line. |
| @@ -476,7 +491,9 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 476 | 491 | if s.profileViewing.has: |
| 477 | 492 | profile.children.add profilePane(s) |
| 478 | 493 | |
| 479 | - var lightbox = vbox(%*{"key": "lightbox-pane"}) | |
| 494 | + # In the overlay rather than the column, and marked to fill it: a picture | |
| 495 | + # being looked at should cover the conversation, not sit under it. | |
| 496 | + var lightbox = vbox(%*{"key": "lightbox-pane", "fill": true}) | |
| 480 | 497 | if s.lightbox.has: |
| 481 | 498 | lightbox.children.add lightboxPane(s) |
| 482 | 499 | |
| @@ -514,7 +531,7 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 514 | 531 | var editing = vbox(%*{"key": "editing"}) |
| 515 | 532 | if s.editing.has: |
| 516 | 533 | editing.children.add hbox(%*{"spacing": 8}, |
| 517 | - emoji("✏️", ""), | |
| 534 | + emoji("📝", ""), | |
| 518 | 535 | dimLabel("Editing your message"), |
| 519 | 536 | button("✕", "edit.cancel")) |
| 520 | 537 | banners.children.add editing |
| @@ -530,11 +547,16 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 530 | 547 | |
| 531 | 548 | # The compose bar. The picture button is a tile rather than an emoji: the |
| 532 | 549 | # emoji was a colour photo that matched nothing else in the bar. |
| 533 | - var compose = hbox(%*{"spacing": 8, "align": "center", "marginBottom": 12}, | |
| 550 | + # `wrap: false`, so this is a row and the box can take what the picture | |
| 551 | + # button and Send leave. It was a Wrap with the box pinned to 260 points, | |
| 552 | + # which is a message box the width of a phone's on a window four times | |
| 553 | + # that — and the rest of the line empty beside it. | |
| 554 | + var compose = hbox(%*{"spacing": 8, "align": "center", "marginBottom": 12, | |
| 555 | + "wrap": false}, | |
| 534 | 556 | image("asset:assets/insert-image.png", maxWidth = 36, maxHeight = 36, |
| 535 | 557 | onClick = "image.pick"), |
| 536 | 558 | entry("draft", s.draft, "Message " & name, "draft.change", |
| 537 | - width = 260, onSubmit = "send"), | |
| 559 | + onSubmit = "send"), | |
| 538 | 560 | button("Send", "send", "primary")) |
| 539 | 561 | |
| 540 | 562 | vbox(%*{"spacing": 8, "margin": 12, "expand": true}, |
| @@ -550,10 +572,9 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 550 | 572 | # conversation between them taking whatever is left. |
| 551 | 573 | n("hbox", %*{"spacing": 8, "wrap": false, "expand": true}, |
| 552 | 574 | @[chatListPane, messages, peoplePane]), |
| 553 | - jump), | |
| 575 | + jump, lightbox), | |
| 554 | 576 | overview, |
| 555 | 577 | profile, |
| 556 | - lightbox, | |
| 557 | 578 | returnRow, |
| 558 | 579 | banners, |
| 559 | 580 | separator(), |
| @@ -37,18 +37,26 @@ func actionChips(m: Message, mine: bool): Node = | |||
| 37 | ## the line with the text they took width off every line under them and | 37 | ## the line with the text they took width off every line under them and |
| 38 | ## wrapped a message that had the room to sit on one. | 38 | ## wrapped a message that had the room to sit on one. |
| 39 | ## | 39 | ## |
| 40 | - ## A row laid out from the right lays its first child furthest right, so | 40 | + ## Only our own lines carry the edit chip: the server refuses an edit of |
| 41 | - ## reacting comes first here and this reads ✏️ then ↩️ then 🙂 on screen. | 41 | + ## somebody else's, and a chip that always fails is a chip that lies. |
| 42 | - ## Only our own lines carry a pencil: the server refuses an edit of somebody | 42 | + ## |
| 43 | - ## else's, and a chip that always fails is a chip that lies. | 43 | + ## The glyphs are all emoji-presentation codepoints, and that is a |
| 44 | + ## constraint rather than a preference. `✏️` is U+270F plus a variation | ||
| 45 | + ## selector *asking* for emoji presentation, and the ask is not binding: | ||
| 46 | + ## some font claims the bare U+270F and draws the monochrome pencil the | ||
| 47 | + ## text era had. On a desktop that font is DejaVu Sans, which naming the | ||
| 48 | + ## colour face gets around; in a browser CanvasKit has no such face to name | ||
| 49 | + ## — naming one gives notdef boxes — so the only reliable answer is a | ||
| 50 | + ## codepoint no text font claims. Hence 📝 where ✏️ was, and 💬 where ↩️ | ||
| 51 | + ## was: both are emoji-only, and both come out in colour everywhere. | ||
| 44 | result = n("hbox", %*{"key": "actions", "align": "end", "spacing": chipGap}, @[ | 52 | result = n("hbox", %*{"key": "actions", "align": "end", "spacing": chipGap}, @[ |
| 45 | n("reaction", %*{"key": "react", "emoji": "🙂", "size": pillSize, | 53 | n("reaction", %*{"key": "react", "emoji": "🙂", "size": pillSize, |
| 46 | "onClick": "react.open:" & rowId(m)}), | 54 | "onClick": "react.open:" & rowId(m)}), |
| 47 | - n("reaction", %*{"key": "reply", "emoji": "↩️", "size": pillSize, | 55 | + n("reaction", %*{"key": "reply", "emoji": "💬", "size": pillSize, |
| 48 | "onClick": "reply.to:" & rowId(m)})]) | 56 | "onClick": "reply.to:" & rowId(m)})]) |
| 49 | if mine: | 57 | if mine: |
| 50 | result.children.add n("reaction", | 58 | result.children.add n("reaction", |
| 51 | - %*{"key": "edit", "emoji": "✏️", "size": pillSize, | 59 | + %*{"key": "edit", "emoji": "📝", "size": pillSize, |
| 52 | "onClick": "edit.start:" & rowId(m)}) | 60 | "onClick": "edit.start:" & rowId(m)}) |
| 53 | 61 | ||
| 54 | func reactionRow(m: Message, me: string): Node = | 62 | func reactionRow(m: Message, me: string): Node = |
| @@ -58,8 +66,8 @@ func reactionRow(m: Message, me: string): Node = | |||
| 58 | ## takes yours off, which is the same gesture that put it there. `reaction` | 66 | ## takes yours off, which is the same gesture that put it there. `reaction` |
| 59 | ## rather than a button with the emoji as its label — the renderer draws a | 67 | ## rather than a button with the emoji as its label — the renderer draws a |
| 60 | ## `reaction` in the colour emoji font, where a label gets whatever ordinary | 68 | ## `reaction` in the colour emoji font, where a label gets whatever ordinary |
| 61 | - ## fallback finds, which for ✏️ and ↩️ is a monochrome glyph out of a text | 69 | + ## fallback finds — see `actionChips` on why the glyphs here avoid the |
| 62 | - ## font. | 70 | + ## variation-selector kind entirely. |
| 63 | result = n("hbox", %*{"key": "pills", "spacing": chipGap}) | 71 | result = n("hbox", %*{"key": "pills", "spacing": chipGap}) |
| 64 | var emojis: seq[string] | 72 | var emojis: seq[string] |
| 65 | for r in m.reactions: emojis.add r.emoji | 73 | for r in m.reactions: emojis.add r.emoji |
| @@ -296,17 +304,24 @@ proc overviewPane(s: State): Node = | |||
| 296 | result.children.add body | 304 | result.children.add body |
| 297 | 305 | ||
| 298 | proc lightboxPane(s: State): Node = | 306 | proc lightboxPane(s: State): Node = |
| 299 | - ## The picture being looked at, full size. | 307 | + ## The picture being looked at, as large as the window allows. |
| 300 | ## | 308 | ## |
| 301 | ## A panel over the conversation rather than a screen of its own: closing it | 309 | ## A panel over the conversation rather than a screen of its own: closing it |
| 302 | ## should put the reader back exactly where they were, and a screen would | 310 | ## should put the reader back exactly where they were, and a screen would |
| 303 | ## have to remember where that was. | 311 | ## have to remember where that was. |
| 304 | - card( | 312 | + ## |
| 305 | - hbox(%*{"spacing": 8}, | 313 | + ## It used to be a card in the column with the picture capped at 640 by 480 |
| 314 | + ## — which put a thumbnail-and-a-half below the backlog, off the bottom of | ||
| 315 | + ## a short window, and called it full size. Now it covers the conversation | ||
| 316 | + ## and the picture takes all of it. | ||
| 317 | + n("vbox", %*{"key": "lightbox-card", "spacing": 8, "margin": 12, | ||
| 318 | + "background": true, "expand": true}, @[ | ||
| 319 | + hbox(%*{"spacing": 8, "wrap": false}, | ||
| 306 | title2("Picture"), | 320 | title2("Picture"), |
| 307 | - button("Close", "lightbox.close")), | 321 | + n("button", %*{"label": "Close", "onClick": "lightbox.close", |
| 308 | - image(s.lightbox.url, maxWidth = 640, maxHeight = 480), | 322 | + "expand": true})), |
| 309 | - dimLabel(s.lightbox.url)) | 323 | + n("image", %*{"src": s.lightbox.url, "expand": true}), |
| 324 | + dimLabel(s.lightbox.url)]) | ||
| 310 | 325 | ||
| 311 | proc profilePane(s: State): Node = | 326 | proc profilePane(s: State): Node = |
| 312 | ## Who someone is, behind the nick on a line. | 327 | ## Who someone is, behind the nick on a line. |
| @@ -476,7 +491,9 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 476 | if s.profileViewing.has: | 491 | if s.profileViewing.has: |
| 477 | profile.children.add profilePane(s) | 492 | profile.children.add profilePane(s) |
| 478 | 493 | ||
| 479 | - var lightbox = vbox(%*{"key": "lightbox-pane"}) | 494 | + # In the overlay rather than the column, and marked to fill it: a picture |
| 495 | + # being looked at should cover the conversation, not sit under it. | ||
| 496 | + var lightbox = vbox(%*{"key": "lightbox-pane", "fill": true}) | ||
| 480 | if s.lightbox.has: | 497 | if s.lightbox.has: |
| 481 | lightbox.children.add lightboxPane(s) | 498 | lightbox.children.add lightboxPane(s) |
| 482 | 499 | ||
| @@ -514,7 +531,7 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 514 | var editing = vbox(%*{"key": "editing"}) | 531 | var editing = vbox(%*{"key": "editing"}) |
| 515 | if s.editing.has: | 532 | if s.editing.has: |
| 516 | editing.children.add hbox(%*{"spacing": 8}, | 533 | editing.children.add hbox(%*{"spacing": 8}, |
| 517 | - emoji("✏️", ""), | 534 | + emoji("📝", ""), |
| 518 | dimLabel("Editing your message"), | 535 | dimLabel("Editing your message"), |
| 519 | button("✕", "edit.cancel")) | 536 | button("✕", "edit.cancel")) |
| 520 | banners.children.add editing | 537 | banners.children.add editing |
| @@ -530,11 +547,16 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 530 | 547 | ||
| 531 | # The compose bar. The picture button is a tile rather than an emoji: the | 548 | # The compose bar. The picture button is a tile rather than an emoji: the |
| 532 | # emoji was a colour photo that matched nothing else in the bar. | 549 | # emoji was a colour photo that matched nothing else in the bar. |
| 533 | - var compose = hbox(%*{"spacing": 8, "align": "center", "marginBottom": 12}, | 550 | + # `wrap: false`, so this is a row and the box can take what the picture |
| 551 | + # button and Send leave. It was a Wrap with the box pinned to 260 points, | ||
| 552 | + # which is a message box the width of a phone's on a window four times | ||
| 553 | + # that — and the rest of the line empty beside it. | ||
| 554 | + var compose = hbox(%*{"spacing": 8, "align": "center", "marginBottom": 12, | ||
| 555 | + "wrap": false}, | ||
| 534 | image("asset:assets/insert-image.png", maxWidth = 36, maxHeight = 36, | 556 | image("asset:assets/insert-image.png", maxWidth = 36, maxHeight = 36, |
| 535 | onClick = "image.pick"), | 557 | onClick = "image.pick"), |
| 536 | entry("draft", s.draft, "Message " & name, "draft.change", | 558 | entry("draft", s.draft, "Message " & name, "draft.change", |
| 537 | - width = 260, onSubmit = "send"), | 559 | + onSubmit = "send"), |
| 538 | button("Send", "send", "primary")) | 560 | button("Send", "send", "primary")) |
| 539 | 561 | ||
| 540 | vbox(%*{"spacing": 8, "margin": 12, "expand": true}, | 562 | vbox(%*{"spacing": 8, "margin": 12, "expand": true}, |
| @@ -550,10 +572,9 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 550 | # conversation between them taking whatever is left. | 572 | # conversation between them taking whatever is left. |
| 551 | n("hbox", %*{"spacing": 8, "wrap": false, "expand": true}, | 573 | n("hbox", %*{"spacing": 8, "wrap": false, "expand": true}, |
| 552 | @[chatListPane, messages, peoplePane]), | 574 | @[chatListPane, messages, peoplePane]), |
| 553 | - jump), | 575 | + jump, lightbox), |
| 554 | overview, | 576 | overview, |
| 555 | profile, | 577 | profile, |
| 556 | - lightbox, | ||
| 557 | returnRow, | 578 | returnRow, |
| 558 | banners, | 579 | banners, |
| 559 | separator(), | 580 | separator(), |
modified
nim/tests/tchat.nim +3 -2 | @@ -79,8 +79,9 @@ suite "the chat screen": | ||
| 79 | 79 | test "only our own lines get a pencil": |
| 80 | 80 | let chips = cs.chatScreen(s, true).find("reaction") |
| 81 | 81 | .mapIt(it.props{"emoji"}.getStr()) |
| 82 | - # 🙂 and ↩️ on both messages, ✏️ on ours alone. | |
| 83 | - check chips.countIt(it == "✏️") == 1 | |
| 82 | + # React and reply on both messages, edit on ours alone. Emoji-only | |
| 83 | + # codepoints on purpose — see `actionChips`. | |
| 84 | + check chips.countIt(it == "📝") == 1 | |
| 84 | 85 | check chips.countIt(it == "🙂") == 2 |
| 85 | 86 | |
| 86 | 87 | test "a line with no msgid gets a spacer where the chips would be": |
| @@ -79,8 +79,9 @@ suite "the chat screen": | |||
| 79 | test "only our own lines get a pencil": | 79 | test "only our own lines get a pencil": |
| 80 | let chips = cs.chatScreen(s, true).find("reaction") | 80 | let chips = cs.chatScreen(s, true).find("reaction") |
| 81 | .mapIt(it.props{"emoji"}.getStr()) | 81 | .mapIt(it.props{"emoji"}.getStr()) |
| 82 | - # 🙂 and ↩️ on both messages, ✏️ on ours alone. | 82 | + # React and reply on both messages, edit on ours alone. Emoji-only |
| 83 | - check chips.countIt(it == "✏️") == 1 | 83 | + # codepoints on purpose — see `actionChips`. |
| 84 | + check chips.countIt(it == "📝") == 1 | ||
| 84 | check chips.countIt(it == "🙂") == 2 | 85 | check chips.countIt(it == "🙂") == 2 |
| 85 | 86 | ||
| 86 | test "a line with no msgid gets a spacer where the chips would be": | 87 | test "a line with no msgid gets a spacer where the chips would be": |