lightbox: Prototype lightbox #29

chrisbobbe · 2023-03-16T20:47:16Z

It was great to see how easily we could implement the "hero" animation and pan-zoom!

~~This is stacked on top of dart-lang/sdk#57174, for the copy-link feature.~~ (That's been merged! 🎉)

If this is merged, I plan to file issues for the following:

Animate entrance/exit of the lightbox's top and bottom app bars
Implement drag-down-to-close for the lightbox
Show user's avatar in the lightbox app bar
Add "download" button in lightbox bottom app bar
Add "share" button in lightbox bottom app bar
In the lightbox hero tag, include the "index of the image preview in the message" (discussion)
In lightbox app bar, use a friendlier format for the date, like "Yesterday at 4:47 PM"

then push a commit to main that just updates the corresponding TODOs in the code to point to those new issues.

gnprice

Thanks! Looking forward to having this.

gnprice · 2023-03-16T21:56:11Z

lib/widgets/lightbox.dart

+  );
+}
+
+class _Page extends StatefulWidget {


nit: I think I'd prefer the class names be able to stand alone, even when private to the file: so _LightboxPage.

gnprice · 2023-03-16T21:57:50Z

lib/widgets/lightbox.dart

+    required super.child
+  }) : super(


nit (here and other places in the file): trailing comma when a list item doesn't have the closing punctuation on the same line

gnprice · 2023-03-16T21:59:42Z

lib/widgets/lightbox.dart

+    //   message. Maybe keep `src`, so that on exit the lightbox image doesn't
+    //   fly to an image preview with a different URL, following a message edit
+    //   while the lightbox was open.
+    tag: '${message.id.toString()} $src'


nit: pretty sure the toString is redundant when doing string interpolation

Suggested change

tag: '${message.id.toString()} $src'

tag: '${message.id} $src'

gnprice · 2023-03-16T22:08:30Z

lib/widgets/lightbox.dart

+    // TODO: Add index of the image preview in the message, to not break if
+    //   there are multiple image previews with the same URL in the same
+    //   message. Maybe keep `src`, so that on exit the lightbox image doesn't
+    //   fly to an image preview with a different URL, following a message edit
+    //   while the lightbox was open.
+    tag: '${message.id.toString()} $src'


Instead of a string, this can be an instance of a small ad-hoc class that just contains these two fields and overrides ==. That's what Flutter upstream would definitely do; string concatenation is expensive by comparison.

In the near future we'll be able to simplify that to a record. But the class is quick to write.

gnprice · 2023-03-16T22:09:09Z

lib/widgets/lightbox.dart

+  void _handleRouteEntranceAnimationStatusChange(AnimationStatus status) {
+    if (status == AnimationStatus.completed) {
+      setState(() {
+        _headerFooterVisible = true;


Can the animation status ever go backward — turn from completed to something else?

I think so?

PageRouteBuilder.pageBuilder (where we consume the relevant Animation<double>)
https://github.com/flutter/flutter/blob/2ad6cd72c0/packages/flutter/lib/src/widgets/pages.dart#L94

/// See [ModalRoute.buildPage] for complete definition of the parameters.

ModalRoute.buildPage
https://github.com/flutter/flutter/blob/2ad6cd72c0/packages/flutter/lib/src/widgets/routes.dart#LL1062-L1064C41

/// * [animation]: The animation for this route's transition. When entering, /// the animation runs forward from 0.0 to 1.0. When exiting, this animation /// runs backwards from 1.0 to 0.0.

ModalRoute.buildPage
https://github.com/flutter/flutter/blob/2ad6cd72c0/packages/flutter/lib/src/widgets/routes.dart#L1069-L1071

/// This method is only called when the route is first built, and rarely /// thereafter. In particular, it is not automatically called again when the /// route's state changes unless it uses [ModalRoute.of]. […]

As long as ModalRoute.buildPage is not called synchronously with the beginning of the route exit, I think our listener will observe a transition from completed to something else (AnimationStatus.reverse?) at that time. (I say "as long as" because I think a fresh call to ModalRoute.buildPage would tear down our current listener and we'd make a new listener, and in theory the state change could happen synchronously in between, when we have no listener.)

So maybe we add a case for AnimationStatus.reverse that sets _headerFooterVisible to false.

So maybe we add a case for AnimationStatus.reverse that sets _headerFooterVisible to false.

Sure, except let's make it comprehensive: if the status is completed, we want true, and if not completed, we want false. Right?

gnprice · 2023-03-16T22:34:09Z

lib/widgets/lightbox.dart

+        body: MediaQuery(
+          // Declare the vertical insets unconsumed by `appBar` and
+          // `bottomNavigationBar`, so we can position the image to not overlap
+          // any system intrusions, at least until the user zooms in. The top
+          // and bottom app bars overlay the pan-zoom layer in the Z direction;
+          // they don't affect anything in the pan-zoom layer's Y direction.
+          //
+          // Done by clobbering the ambient MediaQueryData (prepared by Scaffold
+          // for the `body`) with one taken from a BuildContext above the
+          // Scaffold.
+          data: MediaQuery.of(context),


Hmm. We do need this, but this explanation of why doesn't seem right.

As the doc on [Scaffold.extendBody] says, when there's a bottom nav bar, the body will be wrapped in a MediaQuery with bottom padding corresponding to the bottom nav bar. Effectively the bottom nav bar itself is being treated as a system intrusion. This would mean that when we then properly use a SafeArea, we'd shrink the image to avoid the bottom nav bar.

The doc on [Scaffold.extendBodyBehindAppBar] doesn't mention this, but it seems to have much the same behavior, just as one would hope for them to be similar to each other.

I'll give another attempt at this in my next revision.

gnprice · 2023-03-16T22:42:11Z

lib/widgets/lightbox.dart

+
+              // With Material 3, later, try removing these "transparent" overrides
+              shadowColor: Colors.transparent,
+              surfaceTintColor: Colors.transparent,


Does this have a distinct effect from null, the default? I'm not seeing a difference.

Ah yeah, surfaceTintColor does nothing in Material 2, so equivalently we could not pass surfaceTintColor.

In fact, here, I think passing a transparent shadowColor does nothing either. (Using a color picker, I haven't detected a difference from not passing it.) In Material 2, the default shadow color is black (transparent in M3), and the shadow is drawn on the Scaffold's background, which we've set to black.

I'll remove these lines and the comment, here and on the BottomAppBar.

gnprice · 2023-03-16T22:54:57Z

lib/widgets/lightbox.dart

+                      ScaffoldMessenger.of(context).showSnackBar(
+                        const SnackBar(
+                          behavior: SnackBarBehavior.floating,
+                          content: Text('Link copied')));


On modern Android, this ends up looking a bit ridiculous (and potentially frustrating) because the system has its own "something was just copied to the clipboard" popup… and it appears at the bottom left, and completely obscures the words "Link copied". So the user just sees what appears to be a blank snackbar :-P

Looking around for an example to follow, if I go to system settings and long-press on something like the IMEI or the IP address, I get an option to copy it; when I take that option, it shows a toast. The toast is really quite a lot like a floating snackbar, but:

it's rounded into a stadium shape — whatever;

it has a bounded width, roughly 160px. (Longer text wraps onto two lines and then gets ellipsized.)

This feels fine, and it solves the problem — no overlap with the system's own popup.

So perhaps here:

Suggested change

ScaffoldMessenger.of(context).showSnackBar(

const SnackBar(

behavior: SnackBarBehavior.floating,

content: Text('Link copied')));

ScaffoldMessenger.of(context).showSnackBar(

const SnackBar(

behavior: SnackBarBehavior.floating,

width: 160,

content: Text('Link copied')));

the system has its own "something was just copied to the clipboard" popup…

Ah, looks like the Android docs have guidelines for handling this:

https://developer.android.com/develop/ui/views/touch-and-input/copy-paste#duplicate-notifications

In Android 12L (API level 32) and lower, we suggest alerting users that they’ve successfully copied by issuing a visual pop-up in-app widget (like Toasts or Snackbars) after copying.

To avoid duplicate displays of information, we strongly recommend removing any pop-up widget shown after an in-app copy for Android 13 and higher.

How about we follow that? It seems easy.

Sure, sounds good to me.

gnprice · 2023-03-16T22:58:57Z

lib/widgets/lightbox.dart

+                  IconButton(
+                    tooltip: 'Copy link',


Can this IconButton be pulled out as its own widget? So that this would look like:

children: [ _CopyLinkButton(url: widget.src), // TODO … ]

I think that would help keep this build method easier to read, especially when more buttons are added in the future.

gnprice · 2023-03-16T22:59:26Z

lib/widgets/lightbox.dart

+                    icon: const Icon(Icons.copy),
+                    onPressed: () async {
+                      await Clipboard.setData(ClipboardData(text: widget.src));
+                      if (!context.mounted) return; // ignore: use_build_context_synchronously


Hmm interesting. What's the reasoning for the ignore?

An earlier version of this code was "BAD" according to this lint rule: https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html

This revision is "GOOD", but the rule doesn't recognize that.

Huh, weird.

Let's have a comment explaining any lint ignores we use.

This one seems like a bug in the lint rule. Is there an issue for it in the upstream tracker? If so, a link to that would suffice as an explanation.

It looks like this is https://github.com/dart-lang/linter/issues/4007 . It comes with a workaround that's good enough — you rewrite the code as:

if (context.mounted) { // … use context … }

chrisbobbe · 2023-03-28T22:44:10Z

Thanks for the review! Revision pushed.

One interesting thing is that, after dart-lang/sdk#57178, the RealmContentNetworkImage under LightboxHero doesn't have its needed PerAccountStoreWidget ancestor during the hero's "flight", so the image wasn't rendering during the flight.

I've added a PerAccountStoreWidget in this revision, but maybe there's a better way to do so?

gnprice

Thanks! This is getting close.

gnprice · 2023-03-29T02:42:48Z

lib/widgets/content.dart

+    final messageContentWidget = context.findAncestorWidgetOfExactType<MessageContent>();
+    assert(messageContentWidget != null, 'No MessageContent ancestor');
+    final message = messageContentWidget!.message;


Let's encapsulate this as a static method on MessageContent, so that we can say something like:

final message = MessageContent.of(context);

That method is expensive because of the "findAncestor…" call. So it should have a note about that in dartdoc — can copy from and/or refer to the doc of "findAncestor…" itself — and also a TODO comment to use InheritedWidget to fix that. (ISTR having a chat thread about that part; I forget what all our conclusions were.)

gnprice · 2023-03-29T02:44:20Z

lib/widgets/content.dart

+                  alignment: Alignment.center,
+                  color: const Color.fromRGBO(0, 0, 0, 0.03),
+                  child: LightboxHero(
+                    accountId: PerAccountStoreWidget.accountIdOf(context),


Let's pull this sort of call out as a local before the main widget expression.

gnprice · 2023-03-29T02:55:30Z

lib/widgets/lightbox.dart

+      // RealmContentNetworkImage needs an ancestor PerAccountStoreWidget, and
+      // during the hero's "flight", we wouldn't otherwise get one.
+      child: PerAccountStoreWidget(accountId: accountId,


Hmm interesting. I guess it lives directly under the navigator's Stack, and not under either page:

Heroes and the Navigator's Overlay Stack must be axis-aligned for all this to work. The top left and bottom right coordinates of each animated Hero will be converted to global coordinates and then from there converted to that Stack's coordinate space, and the entire Hero subtree will, for the duration of the animation, be lifted out of its original place, and positioned on that stack.

https://api.flutter.dev/flutter/widgets/Hero-class.html

This solution feels a bit kludgy, though. I think a major part of why it feels that way to me is that it's inserting a PerAccountStoreWidget even while we're still within each route, when there's already one as an ancestor.

I think this API's preferred solution for this sort of problem is to use [Hero.flightShuttleBuilder]. See here in the Hero doc:

Additionally, if the Hero subtree changes appearance based on an InheritedWidget (such as MediaQuery or Theme), then the hero animation may have discontinuity at the start or the end of the animation because route A and route B provides different such InheritedWidgets. Consider providing a custom flightShuttleBuilder to ensure smooth transitions. The default flightShuttleBuilder interpolates MediaQuery's paddings. If your Hero widget uses custom InheritedWidgets and displays a discontinuity in the animation, try to provide custom in-flight transition using flightShuttleBuilder.

And here's the type for that builder:
https://api.flutter.dev/flutter/widgets/HeroFlightShuttleBuilder.html

So it gets plenty of contexts, which it can use to find this information and provide its own PerAccountStoreWidget.

gnprice · 2023-03-29T03:03:17Z

lib/widgets/clipboard.dart

+void copyWithPopup({
+  required BuildContext context,
+  required ClipboardData data,
+  required Text successText // TODO(i18n)


Suggested change

required Text successText // TODO(i18n)

required Widget successContent,

In general, APIs that take a widget should avoid caring about what type of widget it is; the type should be just Widget, unless the code is actually going to make use of features of a more specific type.

(And then this todo-i18n comment can go at the call site.)

gnprice · 2023-03-29T03:14:05Z

lib/widgets/clipboard.dart

+    final shouldShowSnackbar =
+      (deviceInfo is IosDeviceInfo)
+
+        // Android 13+ shows its own popup on copying to the clipboard,
+        // so we suppress ours, following the advice at:
+        //   https://developer.android.com/develop/ui/views/touch-and-input/copy-paste#duplicate-notifications
+        // TODO(android-sdk-32) Remove and simplify dartdoc
+        || (deviceInfo is AndroidDeviceInfo && deviceInfo.version.sdkInt <= 32);


This is a bit hard to read because the conditions are separated so widely from each other by the comment, it looks after the "IosDeviceInfo" line as if that's the whole condition.

One solution could be to move the comment above the whole expression.

Or, here's a version using fancy Dart 3 pattern-matching:

Suggested change

final shouldShowSnackbar =

(deviceInfo is IosDeviceInfo)

// Android 13+ shows its own popup on copying to the clipboard,

// so we suppress ours, following the advice at:

// https://developer.android.com/develop/ui/views/touch-and-input/copy-paste#duplicate-notifications

// TODO(android-sdk-32) Remove and simplify dartdoc

|| (deviceInfo is AndroidDeviceInfo && deviceInfo.version.sdkInt <= 32);

final bool shouldShowSnackbar;

switch (deviceInfo) {

case AndroidDeviceInfo(:var version):

// Android 13+ shows its own popup on copying to the clipboard,

// so we suppress ours, following the advice at:

// https://developer.android.com/develop/ui/views/touch-and-input/copy-paste#duplicate-notifications

// TODO(android-sdk-33): Simplify this and dartdoc

shouldShowSnackbar = version.sdkInt <= 32;

default:

shouldShowSnackbar = true;

};

Also reflected in that version: if SDK 32 is the last version with the old behavior, then SDK 33 is the one where this TODO becomes actionable when we bump the minimum that far, which means that's the one we want to mention in the comment.

gnprice · 2023-03-29T03:23:46Z

lib/widgets/lightbox.dart

+    switch (status) {
+      case AnimationStatus.completed:
+        // Route entrance transition complete
+        setState(() {
+          _headerFooterVisible = true;
+        });
+        break;
+      default:
+        setState(() {
+          _headerFooterVisible = false;
+        });
+        break;
+    }


can simplify:

Suggested change

switch (status) {

case AnimationStatus.completed:

// Route entrance transition complete

setState(() {

_headerFooterVisible = true;

});

break;

default:

setState(() {

_headerFooterVisible = false;

});

break;

}

final entranceAnimationComplete = status == AnimationStatus.completed;

setState(() {

_headerFooterVisible = entranceAnimationComplete;

});

(The switch already isn't adding any kind of robustness compared to that, because it has a default case.)

gnprice · 2023-03-29T03:25:38Z

lib/widgets/lightbox.dart

+    final appBarBackgroundColor = Colors.grey.shade900.withOpacity(0.87);
+    const appBarForegroundColor = Colors.white;
+
+    PreferredSizeWidget? appBar;


similarly, just Widget?

Scaffold asks for a PreferredSizeWidget? for its appBar param: https://api.flutter.dev/flutter/material/Scaffold/appBar.html

Huh, I see! 👍, then.

gnprice · 2023-03-29T03:29:31Z

lib/widgets/lightbox.dart

+        centerTitle: false,
+        foregroundColor: appBarForegroundColor,
+        backgroundColor: appBarBackgroundColor,


nit: put these small arguments before the long, "child"-style, argument

gnprice · 2023-03-29T03:34:51Z

lib/widgets/content.dart

+                    src: resolvedSrc)))),
+    );


nit:

Suggested change

src: resolvedSrc)))),

);

src: resolvedSrc)))));

gnprice · 2023-03-29T03:35:42Z

lib/widgets/lightbox.dart

+  bool operator ==(Object other) {
+    return other is _LightboxHeroTag &&
+      other.messageId == messageId &&
+      other.src == other.src;


Suggested change

other.src == other.src;

other.src == src;

🙂

We'll use this to conditionalize on the Android SDK version integer.

A lot of different widgets, some pretty deeply nested, go into rendering a message's content. This way each of those widgets can get data from the relevant Message object as needed, without the need for passing it all the way down, from widget params to widget params.

chrisbobbe · 2023-03-29T21:43:39Z

Thanks for the review! Revision pushed.

gnprice

Thanks! Mostly small things remaining.

gnprice · 2023-03-29T22:05:03Z

lib/widgets/content.dart

+/// Provides access to [message].
+class InheritedMessage extends InheritedWidget {


I think this is a case of https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-useless-documentation 🙂

Not everything in our tree has docs, and that's fine — unlike the Flutter framework, this isn't an API being provided to thousands of developers to consume, so I don't anticipate we'd ever want to turn on a lint that requires all public members to have docs.

gnprice · 2023-03-29T22:08:50Z

lib/widgets/content.dart

+                    src: resolvedSrc,
+                    child: RealmContentNetworkImage(
+                      resolvedSrc,


Hmm, this feels kind of redundant. Can the LightboxHero use src to construct this RealmContentNetworkImage widget for itself?

gnprice · 2023-03-29T22:11:08Z

lib/widgets/lightbox.dart

+          child: RealmContentNetworkImage(src, filterQuality: FilterQuality.medium));
+      },
+      child: child,


And in fact this feels like it's encoding an assumption that the child field is basically the same as this RealmContentNetworkImage widget. So it may as well construct it itself in all cases. Like this, say:

Widget build(BuildContext context) { final child = RealmContentNetworkImage(src, filterQuality: FilterQuality.medium); return Hero( // … child: child); }, child: child);

gnprice · 2023-03-29T22:14:00Z

lib/widgets/content.dart

+  static InheritedMessage of(BuildContext context) {
+    final widget = context.dependOnInheritedWidgetOfExactType<InheritedMessage>();
+    assert(widget != null, 'No InheritedMessage ancestor');
+    return widget!;


Suggested change

return widget!;

return widget!.message;

Much like Theme.of(context) and MediaQuery.of(context) and friends.

gnprice · 2023-03-29T22:15:45Z

lib/widgets/content.dart

@@ -22,7 +23,24 @@ class MessageContent extends StatelessWidget {

  @override
  Widget build(BuildContext context) {
-    return BlockContentList(nodes: content.nodes);
+    return InheritedMessage(message: message, child: BlockContentList(nodes: content.nodes));


nit:

Suggested change

return InheritedMessage(message: message, child: BlockContentList(nodes: content.nodes));

return InheritedMessage(message: message,

child: BlockContentList(nodes: content.nodes));

That way the main payload (the BlockContentList) is kept prominently visible.

gnprice · 2023-03-29T22:17:47Z

lib/widgets/clipboard.dart

+  // Early return on !mounted would be better, but:
+  //   https://github.com/dart-lang/linter/issues/4007
+  if (context.mounted) {


Here's a fun way to express this, which I developed the other day in my draft for #11 :

if (context.mounted) {} // https://github.com/dart-lang/linter/issues/4007 else { return; }

gnprice · 2023-03-29T22:23:07Z

lib/widgets/lightbox.dart

+  void _handleRouteEntranceAnimationStatusChange(AnimationStatus status) {
+    final entranceAnimationComplete = status == AnimationStatus.completed;
+    setState(() {
+      _headerFooterVisible = entranceAnimationComplete;
+    });
+  }
+
+  void _handleTap() {


nit: I think the order reads a bit more cleanly if these come after initState/dispose, so just above build.

gnprice · 2023-03-29T22:23:30Z

lib/widgets/lightbox.dart

+                style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor),
+              ),


nit:

Suggested change

style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor),

),

style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor)),

gnprice · 2023-03-29T22:24:00Z

lib/widgets/lightbox.dart

+        iconTheme: themeData.iconTheme.copyWith(color: appBarForegroundColor),
+      ),


nit:

Suggested change

iconTheme: themeData.iconTheme.copyWith(color: appBarForegroundColor),

),

iconTheme: themeData.iconTheme.copyWith(color: appBarForegroundColor)),

chrisbobbe · 2023-03-29T22:56:33Z

Thanks for the review! Revision pushed.

gnprice · 2023-03-29T23:07:16Z

LGTM! Merging.

chrisbobbe · 2023-03-29T23:09:07Z

Great, thanks for all the reviews! I have this planned followup which I hope to do soon (quoting from PR description):

If this is merged, I plan to file issues for the following:

Animate entrance/exit of the lightbox's top and bottom app bars

Implement drag-down-to-close for the lightbox

Show user's avatar in the lightbox app bar

Add "download" button in lightbox bottom app bar

Add "share" button in lightbox bottom app bar

In the lightbox hero tag, include the "index of the image preview in the message" (discussion)

In lightbox app bar, use a friendlier format for the date, like "Yesterday at 4:47 PM"

then push a commit to main that just updates the corresponding TODOs in the code to point to those new issues.

gnprice · 2023-03-29T23:15:27Z

SGTM

chrisbobbe · 2023-03-31T22:38:27Z

Done: 1381f5e

chrisbobbe requested a review from gnprice March 16, 2023 20:47

chrisbobbe force-pushed the pr-lightbox branch from 70d7f6a to c0cf7e6 Compare March 16, 2023 20:47

gnprice reviewed Mar 16, 2023

View reviewed changes

gnprice mentioned this pull request Mar 24, 2023

store: Move PerAccountStoreWidget down into individual routes #32

Merged

chrisbobbe force-pushed the pr-lightbox branch from c0cf7e6 to 9971951 Compare March 28, 2023 22:41

gnprice reviewed Mar 29, 2023

View reviewed changes

chrisbobbe added 3 commits March 29, 2023 14:35

deps: Add device_info_plus

9b122d4

We'll use this to conditionalize on the Android SDK version integer.

widgets/clipboard: Add, to be used soon in lightbox

f94d942

chrisbobbe force-pushed the pr-lightbox branch from 9971951 to 8888151 Compare March 29, 2023 21:43

gnprice reviewed Mar 29, 2023

View reviewed changes

lightbox: Prototype lightbox

d3a3178

chrisbobbe force-pushed the pr-lightbox branch from 8888151 to d3a3178 Compare March 29, 2023 22:56

gnprice merged commit d3a3178 into zulip:main Mar 29, 2023

chrisbobbe deleted the pr-lightbox branch March 29, 2023 23:08

chrisbobbe mentioned this pull request Jul 19, 2023

Animate lightbox bars #224

Closed

gnprice mentioned this pull request Nov 21, 2024

moving dart-lang/linter issues into this repo dart-lang/sdk#57109

Closed

	tag: '${message.id.toString()} $src'
	tag: '${message.id} $src'

	required Text successText // TODO(i18n)
	required Widget successContent,

		/// Provides access to [message].
		class InheritedMessage extends InheritedWidget {

	return InheritedMessage(message: message, child: BlockContentList(nodes: content.nodes));
	return InheritedMessage(message: message,
	child: BlockContentList(nodes: content.nodes));

		style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor),
		),

		iconTheme: themeData.iconTheme.copyWith(color: appBarForegroundColor),
		),

lightbox: Prototype lightbox #29

lightbox: Prototype lightbox #29

Conversation

chrisbobbe commented Mar 16, 2023 • edited Loading

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented Mar 28, 2023

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented Mar 29, 2023

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe commented Mar 29, 2023

gnprice commented Mar 29, 2023

chrisbobbe commented Mar 29, 2023 • edited Loading

gnprice commented Mar 29, 2023

chrisbobbe commented Mar 31, 2023 • edited Loading

chrisbobbe commented Mar 16, 2023 •

edited

Loading

chrisbobbe commented Mar 29, 2023 •

edited

Loading

chrisbobbe commented Mar 31, 2023 •

edited

Loading