Track user status #1629

sm-sayedi · 2025-06-24T21:29:30Z

Support for tracking the user status.

Message List:

Normal display name	Long display name

Recent DMs Page:

Normal display name	Long display name

New DM Sheet

Normal display name	Long display name

User Autocomplete:

Normal display name	Long display name

Profile Page:

Normal display name	Normal display name - Dark	Long display name

Fixes: #197

chrisbobbe · 2025-06-24T21:53:27Z

Thanks!! CI is failing; could you take a look?

chrisbobbe

Thanks, this is exciting! Comments below on the first 4 commits:

5f8cf0e33 api: Add InitialSnapshot.userStatus
1202dbd14 api [nfc]: Add ReactionType.fromApiValue
99b96b76a api: Add user_status event
f333f8935 store: Add UserStore.getUserStatus, with event updates

This is an area where we're reasonably happy with the zulip-mobile implementation, so I recommend reading it. It handles the nuance I mention below, and it discusses the concept of a "zero value" (or perhaps links to discussion; I don't remember 🙂). Could you take a look and see what might be helpful there? For example I think the model code has a name like "user statuses" (plural); it seems like an API wart that the initial snapshot uses the singular user_status for a map about many users.

chrisbobbe · 2025-06-24T22:07:23Z

lib/api/model/model.dart

@@ -158,6 +158,31 @@ class RealmEmojiItem {
  Map<String, dynamic> toJson() => _$RealmEmojiItemToJson(this);
 }

+/// An value in [InitialSnapshot.userStatus].


nit: "A value"

chrisbobbe · 2025-06-24T22:22:48Z

lib/api/model/events.dart

+  static String? _stringFromJson(Object? json) {
+    final value = json as String;
+    return value.isNotEmpty ? value : null;
+  }
+
+  static ReactionType? _reactionTypeFromJson(Object? json) {
+    final value = json as String;
+    return value.isNotEmpty ? ReactionType.fromApiValue(value) : null;
+  }


There's a nuance that's not really clear in the API doc, but that we figured out and implemented in zulip-mobile:

The meaning of user_status is really an update to a user's status; it's not a complete representation of what their status is.

In particular, the event's status_text, emoji_name, emoji_code, reaction_type may be null, and null means that that part of the status didn't change. So for example, if your current status is a house emoji and the text "Working remotely", and we handle an event with status_text: null and non-null emoji fields, then your status should then be "Working remotely" with the different (not-house) emoji. The fields could be the empty string, which is a "zero value": if the event has status_text: "", then that means the status text was cleared (set to "zero"), which is a kind of change.

It's tricky to see this behavior, because I think the current web app always includes all the fields when it makes update-status requests. But some clients do send partial updates (maybe zulip-mobile?), and we should avoid misinterpreting e.g. status_text: null as clearing out the status text, when in fact it means the status didn't change.

So concretely, for this code, we should allow the fields to be null (so no json as String), and preserve the distinction between "" and null (so no conversion of "" to null).

Relatedly, we found that user_status in the initial snapshot isn't a full snapshot of everyone's status either. Each entry represents a change from the baseline of the "zero value" status—

{ away: false, // (which we're ignoring; it's deprecated) status_text: '', emoji_name: '', reaction_type: '', emoji_code: '' }

—which explains why a user with that "zero value" status doesn't appear in the initial snapshot.

See zulip-mobile: https://github.com/zulip/zulip-mobile/blob/main/src/user-statuses/userStatusesCore.js

chrisbobbe

Thanks for the revision! Looks like some existing tests are failing, and build_runner at one of the first few commits; do you know why that's happening?

chrisbobbe · 2025-06-26T06:24:37Z

lib/api/model/events.dart

@@ -61,6 +61,7 @@ sealed class Event {
          default: return UnexpectedEvent.fromJson(json);
        }
      // case 'muted_topics': … // TODO(#422) we ignore this feature on older servers
+      case 'user_status': return UserStatusEvent.fromJson(json);


api: Add user_status event

tools/check build_runner is failing at this commit; I think that can be fixed with tools/check build_runner --fix.

alya · 2025-06-26T21:23:30Z

The screenshots look great to me!

sm-sayedi · 2025-06-26T23:08:56Z

Thank you @chrisbobbe for the previous review. It's good to know about the intricate detail in the API you mentioned. A new revision is pushed with tests included too, PTAL.

The CI errors are also solved; thanks for mentioning the suggested fix.

chrisbobbe

Thanks! This looks mostly good except I'd like to tighten up a few things:

I'd like to align a bit more with the model code in zulip-mobile. I think it'll be easier for me to show than to write an explanation, so I'll push a WIP commit at the end. If it makes sense, please incorporate it into the earlier commits.
In this PR, the status emoji always appears after the user's name, which is a piece of text. Can we use asWidgetSpan everywhere, and leave the plain UserStatusEmoji to be used later, just for #198?
- With this pattern, it seems like it would be easier to make sure the appearance stays consistent as new callers are added in the future (and among current callers).
- I think it might mean the status emoji gets cut off sometimes, when a super long name is truncated with "…". Depending on the call site, I'd say either:
  - That's OK; the emoji isn't more important than the part of the name we've already decided to cut off. Or,
  - The name actually shouldn't be getting cut off (or should be getting cut off later), and let's add a TODO to fix that. (The sender row in the message list, I think; possibly other places?)

If centralizing on asWidgetSpan makes sense, then the tests will need a way to find the emoji widget inside rich text. Here's a stab at a helper function for that, inspired by similar code for finding UserMentions in WidgetSpans in message content:

      /// Find a [UserStatusEmoji] widget, if present, that is shown
      /// in a [WidgetSpan] along with a user's name.
      ///
      /// Pass either [fullName] or [fullNameFinder] (not both)
      /// to find the [RenderParagraph] that contains the user's name and may
      /// contain the [UserStatusEmoji].
      ///
      /// If [fullName] is passed, the following Finder is used:
      ///   find.textContaining(fullName, findRichText: true)
      UserStatusEmoji? findStatusEmojiOnName({String? fullName, Finder? fullNameFinder}) {
        assert((fullName == null) != (fullNameFinder == null),
          'pass either fullName or fullNameFinder but not both');
        fullNameFinder ??= find.textContaining(fullName!, findRichText: true);

        final nameRootSpan = tester.renderObject<RenderParagraph>(fullNameFinder).text;

        UserStatusEmoji? result;
        nameRootSpan.visitChildren((span) {
          if (span is! WidgetSpan) return true; // keep looking

          result = tester.widgetList<UserStatusEmoji>(
            find.descendant(
              of: find.byWidget(span.child),
              matching: find.byWidgetPredicate((widget) => widget is UserStatusEmoji),
              matchRoot: true,
            )).singleOrNull;

          if (result == null) {
            return true; // keep looking
          } else {
            return false; // found; stop looking
          }
        });
        return result;
      }

chrisbobbe · 2025-06-28T01:35:16Z

lib/api/model/initial_snapshot.dart

+  @JsonKey(name: 'user_status')
+  // In register-queue, the name of this field is the singular "user_status",
+  // even though it actually contains user status information for all the users
+  // that the self-user has access to. Therefore, we prefer to use the plural form.


nit: put the comment above the line that makes it necessary, instead of below

chrisbobbe · 2025-07-01T00:25:01Z

lib/api/model/model.dart

+/// A value in [InitialSnapshot.userStatuses].
+///
+/// For docs, search for "user_status:"
+/// in <https://zulip.com/api/register-queue>.
+@JsonSerializable(fieldRename: FieldRename.snake)
+class UserStatus {
+  // final bool? away; // deprecated in server-6 (FL-148); ignore
+  final String? statusText;
+  final String? emojiName;
+  final String? emojiCode;
+  final ReactionType? reactionType;
+
+  UserStatus({
+    required this.statusText,
+    required this.emojiName,
+    required this.emojiCode,
+    required this.reactionType,
+  });
+
+  factory UserStatus.fromJson(Map<String, dynamic> json) =>
+    _$UserStatusFromJson(json);
+
+  Map<String, dynamic> toJson() => _$UserStatusToJson(this);
+}


The emoji-status information is contained in multiple fields, and you've already spotted a surprise accident where the fields can be inconsistent with each other. Thanks for that!

From that API design thread, we know how to handle that inconsistency, but ideally we'd have some code that handles it explicitly at the edge / the "crunchy shell", making it possible to write the "soft center" (PerAccountStore and its consumers) error-free without even needing to be aware of it. I plan to push a draft commit that does that, along with some other suggested changes in how we model the data.

chrisbobbe · 2025-07-01T00:26:54Z

lib/model/user.dart

+  void handleUserStatusEvent(UserStatusEvent event) {
+    final UserStatusEvent(
+      :userId, :statusText, :emojiName, :emojiCode, :reactionType) = event;
+
+    final oldStatus = _userStatuses[userId];
+    // Here's what the different values of a property in the event mean and how
+    // they affect the corresponding values in the resulting new status:
+    //  - Value is `null` -> property is not changed -> value in the new status
+    //    is the same as in the old status, or `null` if status wasn't set before.
+    //  - Value is empty -> property is cleared -> value in the new status is `null`.
+    //  - Value is not empty -> property is set - value in the new status is the
+    //    same as the value from the event.
+    final newStatus = UserStatus(
+      statusText: statusText == null
+        ? oldStatus?.statusText
+        : statusText.isEmpty
+            ? null
+            : statusText,
+      emojiName: emojiName == null
+        ? oldStatus?.emojiName
+        : emojiName.isEmpty
+            ? null
+            : emojiName,
+      emojiCode: emojiCode == null
+        ? oldStatus?.emojiCode
+        : emojiCode.isEmpty
+            ? null
+            : emojiCode,
+      reactionType: reactionType == null
+        ? oldStatus?.reactionType
+        : reactionType == UserStatusEventReactionType.empty
+          // Currently, if a status emoji is not selected, the server sends
+          // "reaction_type: unicode_emoji", so to keep things clean,
+          // we treat it as `null`.
+          // See discussion:
+          //   https://chat.zulip.org/#narrow/channel/378-api-design/topic/user.20status/near/2203928
+          || (emojiName!.isEmpty)
+            ? null
+            : ReactionType.fromApiValue(reactionType.toJson()),
+      );
+
+    _userStatuses[event.userId] = newStatus;
+  }


This can become much shorter with a change in how we model the data; I plan to push a draft commit to demonstrate, as mentioned :)

(And the "keep things clean" logic will be out at the "crunchy shell", as I hinted in a previous comment.)

chrisbobbe · 2025-07-01T00:31:03Z

lib/widgets/emoji.dart

+  /// Whether to disable the animation for animated emojis.
+  ///
+  /// By default, animation is enabled.
+  final bool noAnimation;


Let's improve this by making it clear that it's an override, that it's not making the caller responsible for checking the accessibility settings, power-saving settings, etc.

How about something like:

/// Whether to show an animated emoji in its "still" variant only, /// even if device settings permit animation. /// /// Defaults to false. final bool neverAnimate;

chrisbobbe · 2025-07-01T00:39:37Z

lib/widgets/content.dart

+    return switch (emojiDisplay) {
+      UnicodeEmojiDisplay() => UnicodeEmojiWidget(
+        size: size, notoColorEmojiTextSize: notoColorEmojiTextSize,
+        emojiDisplay: emojiDisplay),
+      ImageEmojiDisplay() => ImageEmojiWidget(
+        size: size, emojiDisplay: emojiDisplay, noAnimation: noAnimation),
+      // If the status emoji has to be displayed as text because it failed to
+      // load, we ignore it because it would look weird for emojis with longer
+      // text, in certain places, for example in message list.
+      TextEmojiDisplay() => SizedBox.shrink(),
+    };


I think this handles the case where an image emoji's URL doesn't parse, but it doesn't yet handle the case where the network request for the image fails. How about:

// If an image emoji's URL string doesn't parse or the image fails to load, // show nothing. The user-status feature doesn't support a :text_emoji:- // style display. final placeholder = SizedBox.shrink(); return switch (emojiDisplay) { UnicodeEmojiDisplay() => UnicodeEmojiWidget( size: size, notoColorEmojiTextSize: notoColorEmojiTextSize, emojiDisplay: emojiDisplay), ImageEmojiDisplay() => ImageEmojiWidget( size: size, emojiDisplay: emojiDisplay, neverAnimate: noAnimation, errorBuilder: (_, _, _) => placeholder, ), TextEmojiDisplay() => placeholder, };

chrisbobbe · 2025-07-01T01:25:20Z

test/widgets/recent_dm_conversations_test.dart

@@ -182,6 +186,22 @@ void main() {
        }
      }

+      void checkStatusEmoji({required bool isVisible, int? count}) {


recent-dms: Show user status emoji in recent DMs page

It looks like count is never passed; can we simplify that away?

chrisbobbe · 2025-07-01T01:32:33Z

lib/widgets/content.dart

+  static InlineSpan asWidgetSpan({
+    required int userId,
+    required double size,
+    required double notoColorEmojiTextSize,
+    bool noAnimation = true,
+  }) {
+    return WidgetSpan(
+      alignment: PlaceholderAlignment.middle,
+      child: Padding(
+        padding: const EdgeInsetsDirectional.only(start: 4.0),
+        child: UserStatusEmoji(userId: userId, size: size,
+          notoColorEmojiTextSize: notoColorEmojiTextSize, noAnimation: noAnimation)));
+  }


Could you add a dartdoc with some guidance helping callers decide whether to use asWidgetSpan vs. just the UserStatusEmoji constructor? As long as we're putting the status emoji directly after text, it seems like asWidgetSpan is a possible choice, but I don't know whether it's always or only sometimes the right choice. Having the sizing, spacing, and vertical alignment always look right is a goal 🙂

We'll probably need the not-WidgetSpan variant for at least the choose-emoji-status picker, #198, later, but I'm not sure about the sites in this PR.

chrisbobbe · 2025-07-01T04:41:55Z

test/widgets/new_dm_sheet_test.dart

+      final statusEmojiFinder = find.ancestor(of: find.byType(UnicodeEmojiWidget),
+        matching: find.byType(UserStatusEmoji));


(same comment that sometimes the status emoji won't be a Unicode emoji; here and one more below)

chrisbobbe · 2025-07-01T04:45:12Z

test/widgets/new_dm_sheet_test.dart

+    check(findUserChip(user1)).findsNothing();
+    check(findUserChip(user2)).findsNothing();
+    checkChipStatusEmoji(user1, isPresent: false);
+    checkChipStatusEmoji(user2, isPresent: false);


These checkChipStatusEmoji calls are redundant with the findUserChip(…).findsNothing()s.

chrisbobbe · 2025-07-01T04:46:16Z

test/widgets/autocomplete_test.dart

+      final statusEmojiFinder = find.ancestor(of: find.byType(UnicodeEmojiWidget),
+        matching: find.byType(UserStatusEmoji));


(same comment that some status emoji won't be Unicode emoji)

chrisbobbe · 2025-07-01T06:47:22Z

lib/api/model/events.dart

-/// representing an empty string value in [UserStatusEvent] where no status
-/// emoji is selected.
-@JsonEnum(fieldRename: FieldRename.snake)
-enum UserStatusEventReactionType {


This made sense to add, before I thought of the changes in my WIP commit 🙂 but I think the WIP commit gives a nice way to make this unnecessary, in addition to aligning more with the intended API semantics.

gnprice · 2025-07-01T18:42:23Z

lib/api/model/model.dart

+    if (reactionType == null || emojiCode == null || emojiName == null) {
+      return null;
+    } else if (reactionType == '' || emojiCode == '' || emojiName == '') {


Can the null case actually happen? That seems at odds with the API docs:
https://zulip.com/api/get-events#user_status
which say these fields are always present, though sometimes the empty string.

Hmm. We intentionally wrote zulip-mobile to handle the fields being sometimes absent; see type UserStatusUpdate and linked discussion in zulip/zulip-mobile@2106381e4

those should eventually make it into the server API doc

Hmm I see :-)

gnprice · 2025-07-01T18:42:38Z

lib/api/model/model.dart

+  static StatusTextChange? fromApiValue(String? apiValue) =>
+    switch (apiValue) {
+      null => null,
+      '' => StatusTextChange(null),
+      _ => StatusTextChange(apiValue),


Similarly, can this null case actually happen?

gnprice · 2025-07-01T20:02:43Z

lib/api/model/model.dart

+/// The absence of one of these means there is no change.
+class StatusTextChange {
+  final String? newValue;
+  const StatusTextChange(this.newValue);


These two classes, and their apply methods, feel fairly boilerplatey to me. I think what's going on can be made clearer by using a generic "option"/"maybe" type. I've just pushed some additional commits as a demo of that approach.

Other than the commit adding Option, those commits are to be squashed into the "WIP add/use UserStatusChange" commit, and/or into the preceding commits that that one would get squashed into.

gnprice · 2025-07-02T00:25:34Z

OK, let's go with the UserStatusChange approach mentioned at #1629 (review) . I've just pushed a revision which

reworks my commit adding an Option class to something more complete;
squashes my other changes into Chris's "add/use UserStatusChange" commit.

The Option commit should get reordered to earlier in the branch, and the other commit incorporated as Chris mentioned above.

gnprice · 2025-07-02T00:26:46Z

About asWidgetSpan: I think the current approach, where some call sites use that and others use the widget directly, is fine.

sm-sayedi · 2025-07-02T22:08:36Z

Thanks @chrisbobbe and @gnprice for the reviews. I have pushed the new revision, PTAL.

I haven't investigated the suggested solution in #1629 comment yet. I will do that after I get some sleep now. Edit: The result of the investigation is shared in #1629 (comment).

CI is also failing for 9 tests in test/model/test_store.dart. I looked at it to find the reason, but unfortunately, I couldn't. It fails for 83680b1 "api: Add InitialSnapshot.userStatuses", where we just added the API bindings.

sm-sayedi · 2025-07-03T08:13:12Z

Ok, so the CI failure is solved. The following change had to be made:

class InitialSnapshot {
...
- @JsonKey(name: 'user_status', includeToJson: false)
+ @JsonKey(name: 'user_status')
  final Map<int, UserStatusChange> userStatuses;
...
}

In addition to adding UserStatusChange.toJson.

chrisbobbe

Thanks! I think this is close; comments below, and I'll also ask @gnprice to take a look too in case it's almost ready for our release today (or in case he has thoughts on the emoji-size stuff, and lib/api/model tests).

chrisbobbe · 2025-07-03T21:51:47Z

lib/api/model/model.dart

+    } else if (reactionType == '' || emojiCode == '' || emojiName == '') {
+      // Sometimes `reaction_type` is 'unicode_emoji' when the emoji is cleared.
+      // This is an accident, to be handled by looking at `emoji_code` instead:
+      //   https://chat.zulip.org/#narrow/channel/378-api-design/topic/user.20status/near/2203132
+      return OptionSome(null);


api: Add InitialSnapshot.userStatuses

Ideally we'd have a test that this server glitch is tolerated, and for the rest of the nontrivial logic in this commit and the commit that adds the event type. Perhaps this can be a followup; @gnprice, what do you think? 🙂

As discussed in a previous review, it's great that we're handling the glitch just once, here at the edge/"crunchy shell". Since we don't let the glitch affect anything deeper in the app code, we don't have to write tests against this glitch in all the places we consume the data 🙂

Tests are added for the logic in this commit, but for the commit that adds the event type, I think the tests added in 8e7b47a - "store: Add UserStore.getUserStatus, with event updates" cover them. 🙂

For the event, I think actually the nontrivial logic that's there doesn't need to be there: #1629 (comment)

(And for the same reason, I think it's not possible to write a reasonable test for it — with or without that bit of logic, an invalid JSON input would produce an exception, so the test would end up having to specify details like what the stack trace of the exception should look like. And those seem like details we don't actually care about specifying.)

chrisbobbe · 2025-07-03T21:58:22Z

test/model/user_test.dart

+    await store.handleEvent(userStatusEvent((5,
+      'Working remotely', '', '', '')));
+    checkUserStatus(store.getUserStatus(5),
+      ('Working remotely', null, null, null));


Let's add another stanza like this, checking what happens when the event says both the text and emoji were cleared.

chrisbobbe · 2025-07-03T22:49:48Z

lib/widgets/message_list.dart

+                    Padding(
+                      padding: const EdgeInsetsDirectional.only(start: 5.0),
+                      child: UserStatusEmoji(userId: sender.userId,
+                        size: 18, notoColorEmojiTextSize: 15),


Thanks for investigating! (#1629 (comment))

What was your experiment that showed "it worked perfectly fine"? 🙂 I'm imagining something like this:

(Make sure you're testing on Android, with Unicode emoji, since notoColorEmojiTextSize has no effect on iOS or with image emoji.)

Choose a size.

Multiply that size by (17.0 / 14.5) and try the result for notoColorEmojiTextSize.

Open the dev tools to "select widget mode" and confirm that the emoji in fact occupies a size-sized square.

If that's successful for several size values and a particular line-height setting, then it sounds like the widget can compute notoColorEmojiTextSize for itself (using size * (17.0 / 14.5)) as long as it always uses that same line-height setting. From reading the dartdoc it does sound like the actual resulting size can be different for different line-height settings, so we should either:

have the implementation use a constant line height (explicitly overriding a value that might be in an ancestor DefaultTextStyle etc.), or

satisfy ourselves that the dartdoc is wrong and fix it. 🙂

(Or I guess give the widget a line-height param and adjust from that, if we can learn exactly how the line height affects the size.)

See the doc on line heights: https://api.flutter.dev/flutter/painting/TextStyle/height.html

What was the effective line in your testing on the message-list page? I think you can find it in the dev tools:

(Things like Material defaults from Typography and our own DefaultTextStyles might be controlling it, I don't remember all the details offhand.)

chrisbobbe · 2025-07-03T23:05:21Z

lib/widgets/message_list.dart

+                    Padding(
+                      padding: const EdgeInsetsDirectional.only(start: 5.0),
+                      child: UserStatusEmoji(userId: sender.userId,
+                        size: 18, notoColorEmojiTextSize: 15),


It's OK to leave that refactor for later if it's complicated. I wouldn't want to merge something that accidentally makes us forget how to show emojis at a size from our design specs. 🙂

But the PR should at least maintain the invariant that each emoji's actual size matches size. In the current world before the refactor, that'll involve opening the devtools to check the size against size for all the new callsites in the PR, unless you're sure that a given callsite uses the same size and line height as one you've already confirmed experimentally.

chrisbobbe · 2025-07-03T23:18:11Z

test/widgets/message_list_test.dart

+        final senderRowFinder = find.ancestor(
+          of: nameFinder,
+          matching: find.ancestor(
+            of: statusEmojiFinder,
+            matching: find.byType(Row),
+          ),
+        );


nit: can make this more compact, putting parens on the same line

chrisbobbe · 2025-07-03T23:20:19Z

lib/widgets/new_dm_sheet.dart

+              child: Text.rich(
+                TextSpan(
+                  children: [
+                    TextSpan(text: store.userDisplayName(userId)),
+                    UserStatusEmoji.asWidgetSpan(userId: userId, size: 17,
+                      notoColorEmojiTextSize: 14),
+                  ]
+                ),


nit: can be more compact with parens/newlines

chrisbobbe · 2025-07-03T23:21:04Z

lib/widgets/autocomplete.dart

+            Row(
+              children: [
+                Flexible(child: labelWidget),
+                if (option case UserMentionAutocompleteResult(:var userId))
+                  Padding(
+                    padding: const EdgeInsetsDirectional.only(start: 5.0),
+                    child: UserStatusEmoji(
+                      userId: userId,
+                      size: 18,
+                      notoColorEmojiTextSize: 15),
+                  )
+              ],
+            ),


nit: make more compact

gnprice · 2025-07-04T00:22:07Z

I'm just about to prepare today's release, so this is just in time for me to include it as a not-yet-merged PR 🙂. I skimmed the code and also played a bit with the UI, and it looks ready for that.

Then that'll give time to discuss the questions above, and otherwise review a bit more, before merging early next week.

sm-sayedi · 2025-07-04T17:26:49Z

Thanks @chrisbobbe for the review. New revision pushed with tests added for #1629 (comment).

gnprice

Thanks @sm-sayedi for building this, and thanks @chrisbobbe for the previous reviews!

Comments below. I've read through the first 9 commits:
0d9e97a basic: Add a generic Option class
5aa1092 api [nfc]: Add ReactionType.fromApiValue
ea32ee4 api: Add InitialSnapshot.userStatuses
8268c0c api: Add user_status event
a7d9a1d store: Add UserStore.getUserStatus, with event updates
74baf27 emoji [nfc]: Remove UnicodeEmojiWidget.notoColorEmojiTextSize
d27a334 emoji [nfc]: Add ImageEmojiWidget.neverAnimate
b5503f1 content: Add UserStatusEmoji widget
5485c1b test: Add changeUserStatus(es) to PerAccountStoreTestExtension

and all but the tests in the 10th:
8f00a1b msglist: Show user status emoji

That leaves those tests, plus 4 more commits, for a future round:
454b37b recent-dms: Show user status emoji in recent DMs page
de0d257 new-dm: Show user status emoji
d4b7646 autocomplete: Show user status emoji in user-mention autocomplete
43112fd profile: Show user status

gnprice · 2025-07-08T19:48:55Z

lib/api/model/model.dart

+  Map<String, dynamic> _textToJson(Option<String?> text) {
+    return {
+      'status_text': switch (text) {
+        OptionNone<String?>() => null,
+        OptionSome<String?>(:var value) => value ?? '',


This isn't quite accurate — this will lead to 'status_text': null in the JSON, which (according to the API docs, anyway) never occurs:

status_text: string

If present, the text content of the user's status message.

What does occur is for status_text to be absent entirely, or to be a string.

Do we need toJson on this type? On the bulk of the types representing data we get from the server, a toJson is only ever useful in tests, and for most such types we don't end up using it there either.

Since it's a bit complicated to get right for this type, let's just leave it out. That way if we find ourselves wanting it in the future, we can add it then and spend the effort required to make it accurate. (If we do use it in a test, we'll want it to accurately simulate what the server might send us.)

Yeah, I think we need it as UserStatusChange.toJson is needed by JsonSerializable for InitialSnapshot.userStatuses map, when dart run build_runner build --delete-conflicting-outputs is run. If we don’t include InitialSnapshot.userStatuses in toJson by using @JsonKey(includeToJson: false), then some tests in test/model/store_test.dart fail.

Please have a look at the new revision. I think it now handles the absence case.

Ah I see, in places like here:

test('GlobalStore.perAccount loading succeeds; InitialSnapshot has ancient server version', () => awaitFakeAsync((async) async { final globalStore = UpdateMachineTestGlobalStore(accounts: [eg.selfAccount]); final json = eg.initialSnapshot(zulipFeatureLevel: eg.ancientZulipFeatureLevel).toJson(); globalStore.prepareRegisterQueueResponse = (connection) { connection.prepare(json: json); };

I think that would still work if this toJson method just threw UnimplementedError, because the minimal boring initial snapshot being used in those tests doesn't contain any user statuses.

But the new implementation looks right, so that's a good solution 🙂

gnprice · 2025-07-08T21:19:02Z

test/api/model/model_test.dart

+typedef RawStatusChange = (String? statusText, String? emojiName,
+  String? emojiCode, String? reactionType);
+
+typedef RipeStatusChange = (Option<String?> text, Option<StatusEmoji?> emoji);


"ripe" doesn't feel like the right metaphor here — "processed"? "cooked"?

Could also inline both these typedefs, and inline mkStatusChange so that the first type isn't needed in two places. I think that mkStatusChange helper is pretty closely tied to the thing doCheck is doing, where it takes its arguments in a very compact form.

gnprice · 2025-07-08T21:21:15Z

test/api/model/model_test.dart

+      incoming: ('', '', '', 'unicode_emoji'),
+      expected: (OptionSome(null), OptionSome(null)));
+
+    // Hardly to happen from the API standpoint, but we handle it anyways.


Suggested change

// Hardly to happen from the API standpoint, but we handle it anyways.

// Hardly likely to happen from the API standpoint, but we handle it anyways.

?

gnprice · 2025-07-08T21:24:45Z

lib/api/model/events.dart

+    _$UserStatusEventFromJson(json);
+
+  @override
+  Map<String, dynamic> toJson() => _$UserStatusEventToJson(this);


Hmm and I guess this isn't accurate either 🙂

Can this throw UnimplementedError? (It can't just be left out, since the base class calls for it. But I suspect that again nothing really uses it.)

With the new change mentioned in #1629 (comment), and the current revision of UserStatusEvent.toJson, I think this is accurate now. IMO, if we can make it accurate, it will be better to be there than to replace it with throwing an UnimplementedError. 🙂

gnprice · 2025-07-08T21:30:32Z

lib/model/user.dart

+
+  /// The status of the user with the given ID.
+  ///
+  /// If no status is set for a user, returns [UserStatus.zero].


Suggested change

/// If no status is set for a user, returns [UserStatus.zero].

/// If no status is set for the user, returns [UserStatus.zero].

(since it's about this particular user, the one identified by the argument to the method)

gnprice · 2025-07-08T21:44:54Z

lib/widgets/emoji.dart

-            fontSize: notoColorEmojiTextSize, forceStrutHeight: true),
+            fontSize: notoColorEmojiTextSize,
+            // Responsible for keeping the line height constant, even
+            // with ambient DefaultTextStyles.


nit:

Suggested change

// with ambient DefaultTextStyles.

// with ambient DefaultTextStyle.

(If you mean the plural, writing it that way is ambiguous because it looks like it means an identifier DefaultTextStyles. When the plural of some identifier is necessary, can disambiguate with backticks around the identifier part. But usually it works to leave it singular instead — here it ends up meaning "with an ambient DefaultTextStyle", which has effectively the same meaning and in fact is a bit more precisely what we mean anyway.)

gnprice · 2025-07-08T21:50:58Z

lib/widgets/emoji.dart

@@ -105,13 +115,20 @@ class ImageEmojiWidget extends StatelessWidget {

  final ImageErrorWidgetBuilder? errorBuilder;

+  /// Whether to show an animated emoji in its "still (non-animated)" variant


What's this quoting from?

It’s not quoting from a specific place, but I thought it may avoid confusion for the word “still” as it also means “yet”. But I think it causes another confusion, so removed it.

Ah, I see.

I think putting quotes just around the word "still" would also work:

in its "still" (non-animated) variant

But this revision, with no quote marks, works fine too.

gnprice · 2025-07-08T21:53:08Z

lib/widgets/content.dart

+      alignment: PlaceholderAlignment.middle,
+      child: Padding(
+        padding: EdgeInsetsDirectional.only(start: paddingStart, end: paddingEnd),
+        child: UserStatusEmoji(userId: userId, size: size, neverAnimate: noAnimation)));


nit: seems like these parameters have the same meaning; use the same name?

Suggested change

child: UserStatusEmoji(userId: userId, size: size, neverAnimate: noAnimation)));

child: UserStatusEmoji(userId: userId, size: size, neverAnimate: neverAnimate)));

gnprice · 2025-07-08T22:01:53Z

lib/widgets/content.dart

+        // If image emoji fails to load, show nothing.
+        errorBuilder: (_, _, _) => placeholder,


It seems like our other call sites that don't have anything in particular to show in this case just leave this parameter out. What happens if we do that?

If errorBuilder is not provided, an exception will be reported.
Quoting the related part of documentation for Image.errorBuilder:

If this builder is not provided, any exceptions will be reported to [FlutterError.onError]. If it is provided, the caller should either handle the exception by providing a replacement widget, or rethrow the exception.

In debug mode, it causes the Simulator to hang, unless hot restarted. In release mode, nothing happens, the image is shown empty.

Huh. I wonder how it is that we get away with that in the other call sites, then.

I guess maybe if you're in debug mode and manage to hit an error there, then those other call sites have the exact same problem. And we just haven't noticed because in debug mode one is usually at one's desk with a good connection, and in release mode it's quiet.

Yes, it happens in other places as well. For example, if we navigate to a narrow with a compose box, turn off the internet connection, and then start typing "@Someone" to mention a user, the same problem occurs because there is no errorBuilder provided in the implementation of AvatarImage.

gnprice · 2025-07-08T22:08:56Z

lib/widgets/message_list.dart

+                  if (sender != null)
+                    Padding(
+                      padding: const EdgeInsetsDirectional.only(start: 5.0),
+                      child: UserStatusEmoji(userId: sender.userId, size: 18)),


We don't actually need the User object here, right? So we can express the user ID the same way as in the neighboring code:

Suggested change

if (sender != null)

Padding(

padding: const EdgeInsetsDirectional.only(start: 5.0),

child: UserStatusEmoji(userId: sender.userId, size: 18)),

Padding(

padding: const EdgeInsetsDirectional.only(start: 5.0),

child: UserStatusEmoji(userId: message.senderId, size: 18)),

One awkward thing that points at, though, is that we're including the padding even when there's no status emoji to show.

The consequence of that is pretty subtle here. One, it means if there isn't room horizontally for the name, it'll wrap or truncate a bit sooner. Two, if the sender is a bot, there's this extra spacing before the bot icon.

Seems like this might come up in other contexts too, though. Perhaps push this padding inside UserStatusEmoji? The caller can perhaps pass the padding argument, exactly like it would to Padding; but then if there's no emoji to show, it doesn't get padded.

Instead of passing this property each time the widget is used, we now calculate it internally. See discussion: zulip#1629 (comment)

sm-sayedi · 2025-07-09T12:15:11Z

Thanks @gnprice for the review. Revision pushed, please have a look.

Instead of passing this property each time the widget is used, we now calculate it internally. See discussion: zulip#1629 (comment)

gnprice

Thanks for the revision! Those changes all look good, except a few small comments below.

Still ahead for me to read are those last few commits as mentioned at #1629 (review) . I'll try to get to those tomorrow; probably not before today's release.

This has some conflicts with main, I think from #1631 which I just merged. I may try to rebase and merge the finished portion of this PR before the release, and then review on the rest can continue in a follow-up PR (which I'll include in the release unmerged, just like I did with a previous revision of this PR in last week's release).

gnprice · 2025-07-09T21:05:55Z

lib/widgets/content.dart

+/// Use [padding] to control the padding of status emoji from neighboring
+/// widgets.


Suggested change

/// Use [padding] to control the padding of status emoji from neighboring

/// widgets.

/// Use [padding] to control the padding of status emoji from neighboring

/// widgets.

/// When there is no status emoji to be shown, the padding will be omitted too.

gnprice · 2025-07-09T21:07:40Z

lib/widgets/content.dart

+        // If image emoji fails to load, show nothing.
+        errorBuilder: (_, _, _) => placeholder,


Huh. I wonder how it is that we get away with that in the other call sites, then.

I guess maybe if you're in debug mode and manage to hit an error there, then those other call sites have the exact same problem. And we just haven't noticed because in debug mode one is usually at one's desk with a good connection, and in release mode it's quiet.

gnprice · 2025-07-09T21:08:18Z

lib/widgets/content.dart

+        child: UnicodeEmojiWidget(
+          size: size, emojiDisplay: emojiDisplay),
+      ),


nit:

Suggested change

child: UnicodeEmojiWidget(

size: size, emojiDisplay: emojiDisplay),

),

child: UnicodeEmojiWidget(

size: size, emojiDisplay: emojiDisplay)),

gnprice · 2025-07-09T21:08:34Z

lib/widgets/content.dart

+          errorBuilder: (_, _, _) => placeholder,
+        ),


nit:

Suggested change

errorBuilder: (_, _, _) => placeholder,

),

errorBuilder: (_, _, _) => placeholder),

gnprice · 2025-07-09T21:11:25Z

lib/api/model/events.dart

+    _$UserStatusEventFromJson(json);
+
+  @override
+  Map<String, dynamic> toJson() => _$UserStatusEventToJson(this);


gnprice · 2025-07-09T22:10:01Z

I may try to rebase and merge the finished portion of this PR before the release

Or @sm-sayedi if you're online, it'd be quite helpful if you can do that rebase :-)

sm-sayedi · 2025-07-09T22:53:06Z

Sure, doing now.

Co-authored-by: Greg Price <[email protected]> Co-authored-by: Chris Bobbe <[email protected]>

Co-authored-by: Chris Bobbe <[email protected]>

Instead of passing this property each time the widget is used, we now calculate it internally. See discussion: zulip#1629 (comment)

Co-authored-by: Chris Bobbe <[email protected]>

sm-sayedi · 2025-07-09T23:21:02Z

Done rebasing, with previous review addressed too.

gnprice · 2025-07-10T00:31:43Z

Thanks! Those edits look good.

I'm going to shorten this PR branch to just the portion that's completed review, and then merge:
0ee6336 api [nfc]: Add ReactionType.fromApiValue
368df50 api: Add InitialSnapshot.userStatuses
3d814d2 api: Add user_status event
ce1c664 store: Add UserStore.getUserStatus, with event updates
dc686ef emoji [nfc]: Remove UnicodeEmojiWidget.notoColorEmojiTextSize
3df6507 emoji [nfc]: Add ImageEmojiWidget.neverAnimate
85db2af content: Add UserStatusEmoji widget
4b75025 test: Add changeUserStatus(es) to PerAccountStoreTestExtension

Please go ahead and send the remaining commits as a follow-up PR:
7975750 msglist: Show user status emoji
7106581 recent-dms: Show user status emoji in recent DMs page
e6a2275 new-dm: Show user status emoji
c97b8a0 autocomplete: Show user status emoji in user-mention autocomplete
057502d profile: Show user status

I'll also include them in today's release.

sm-sayedi added the maintainer review PR ready for review by Zulip maintainers label Jun 24, 2025

chrisbobbe self-requested a review June 24, 2025 21:44

chrisbobbe self-assigned this Jun 24, 2025

chrisbobbe reviewed Jun 24, 2025

View reviewed changes

sm-sayedi force-pushed the 197-track-user-status branch from a2a9b53 to ae0b51f Compare June 26, 2025 05:57

chrisbobbe reviewed Jun 26, 2025

View reviewed changes

sm-sayedi force-pushed the 197-track-user-status branch from ae0b51f to d51cc98 Compare June 26, 2025 22:53

sm-sayedi requested a review from chrisbobbe June 26, 2025 23:09

chrisbobbe reviewed Jul 1, 2025

View reviewed changes

gnprice reviewed Jul 1, 2025

View reviewed changes

gnprice force-pushed the 197-track-user-status branch from e4dbf32 to 11ba820 Compare July 2, 2025 00:23

sm-sayedi force-pushed the 197-track-user-status branch from 11ba820 to 89b10fa Compare July 2, 2025 21:20

sm-sayedi requested a review from chrisbobbe July 2, 2025 22:08

sm-sayedi force-pushed the 197-track-user-status branch 2 times, most recently from 42dec2d to 8d30ccd Compare July 3, 2025 08:06

chrisbobbe reviewed Jul 3, 2025

View reviewed changes

sm-sayedi force-pushed the 197-track-user-status branch 3 times, most recently from f6d3bf0 to c8bc5dd Compare July 4, 2025 17:20

chrisbobbe assigned gnprice and unassigned chrisbobbe Jul 8, 2025

chrisbobbe requested a review from gnprice July 8, 2025 19:29

gnprice reviewed Jul 8, 2025

View reviewed changes

sm-sayedi force-pushed the 197-track-user-status branch from 43112fd to 11a8aec Compare July 9, 2025 12:06

sm-sayedi requested a review from gnprice July 9, 2025 12:15

sm-sayedi force-pushed the 197-track-user-status branch from 11a8aec to f4e566a Compare July 9, 2025 12:30

gnprice reviewed Jul 9, 2025

View reviewed changes

sm-sayedi and others added 8 commits July 10, 2025 03:37

api [nfc]: Add ReactionType.fromApiValue

0ee6336

api: Add InitialSnapshot.userStatuses

368df50

Co-authored-by: Greg Price <[email protected]> Co-authored-by: Chris Bobbe <[email protected]>

api: Add user_status event

3d814d2

Co-authored-by: Chris Bobbe <[email protected]>

store: Add UserStore.getUserStatus, with event updates

ce1c664

Co-authored-by: Chris Bobbe <[email protected]>

emoji [nfc]: Remove UnicodeEmojiWidget.notoColorEmojiTextSize

dc686ef

Instead of passing this property each time the widget is used, we now calculate it internally. See discussion: zulip#1629 (comment)

emoji [nfc]: Add ImageEmojiWidget.neverAnimate

3df6507

content: Add UserStatusEmoji widget

85db2af

Co-authored-by: Chris Bobbe <[email protected]>

test: Add changeUserStatus(es) to PerAccountStoreTestExtension

4b75025

Co-authored-by: Chris Bobbe <[email protected]>

sm-sayedi force-pushed the 197-track-user-status branch from f4e566a to 057502d Compare July 9, 2025 23:17

gnprice force-pushed the 197-track-user-status branch from 057502d to 4b75025 Compare July 10, 2025 00:32

gnprice merged commit 4b75025 into zulip:main Jul 10, 2025
1 check passed

This was referenced Jul 10, 2025

Show user status #1699

Closed

Set user status #1701

Merged

sm-sayedi mentioned this pull request Jul 10, 2025

Show user status in UI #1702

Merged

gnprice mentioned this pull request Jul 10, 2025

Track user status #197

Closed

		final statusEmojiFinder = find.ancestor(of: find.byType(UnicodeEmojiWidget),
		matching: find.byType(UserStatusEmoji));

	// Hardly to happen from the API standpoint, but we handle it anyways.
	// Hardly likely to happen from the API standpoint, but we handle it anyways.

	/// If no status is set for a user, returns [UserStatus.zero].
	/// If no status is set for the user, returns [UserStatus.zero].

	// with ambient DefaultTextStyles.
	// with ambient DefaultTextStyle.

		@@ -105,13 +115,20 @@ class ImageEmojiWidget extends StatelessWidget {

		final ImageErrorWidgetBuilder? errorBuilder;

		/// Whether to show an animated emoji in its "still (non-animated)" variant

	child: UserStatusEmoji(userId: userId, size: size, neverAnimate: noAnimation)));
	child: UserStatusEmoji(userId: userId, size: size, neverAnimate: neverAnimate)));

		// If image emoji fails to load, show nothing.
		errorBuilder: (_, _, _) => placeholder,

		/// Use [padding] to control the padding of status emoji from neighboring
		/// widgets.

	errorBuilder: (_, _, _) => placeholder,
	),
	errorBuilder: (_, _, _) => placeholder),

Track user status #1629

Track user status #1629

Uh oh!

Conversation

sm-sayedi commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented Jun 24, 2025

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alya commented Jun 26, 2025

Uh oh!

sm-sayedi commented Jun 26, 2025

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice commented Jul 2, 2025

Uh oh!

gnprice commented Jul 2, 2025

Uh oh!

sm-sayedi commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sm-sayedi commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

sm-sayedi commented Jun 24, 2025 •

edited

Loading

chrisbobbe Jul 1, 2025 •

edited

Loading

gnprice Jul 1, 2025 •

edited

Loading

sm-sayedi commented Jul 2, 2025 •

edited

Loading

sm-sayedi commented Jul 3, 2025 •

edited

Loading

chrisbobbe left a comment •

edited

Loading

chrisbobbe Jul 3, 2025 •

edited

Loading

sm-sayedi Jul 9, 2025 •

edited

Loading