Show read receipts #1706

sm-sayedi · 2025-07-15T10:22:51Z

Support for viewing read receipts of a message.

Rebased on top of #1700 to borrow some code from. Main commits start from 87518f1 "icons: Add check_check icon, from Figma".

Figma link: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=11367-20647&t=bpmAb3Fr9aJdMIHw-0

Screenshots

Message action sheet	Read receipts - Many

Read receipts - One	Read receipts - Zero	Read receipts - Failed

Screen recording

Read.receipts.-.Screen.recording.mov

Fixes: #667

alya · 2025-07-15T17:00:18Z

We should probably use the same icon as in the web app here:

chrisbobbe · 2025-07-15T17:34:39Z

lib/widgets/read_receipts.dart

+              color: designVariables.title,
+            ).merge(weightVariableTextStyle(context, wght: 600))),
+          if (status == FetchStatus.success && receiptCount > 0)
+            StyledText(


Very interesting, I didn't expect to be able to do this! Is this a solution to #1285?

Looking at the issue, I think this could be a solution for it!

Hmm interesting.

This wouldn't be a satisfactory solution to all of #1285, because this appears to fire up a whole XML parser for each one of these widgets, which is going to be expensive. That's acceptable in this context, where there's only one of these at a time (and it isn't a very common part of the UI anyway). It wouldn't be OK for something that could appear in, say, message content, where you might scroll past a lot of them.

gnprice

Thanks! Here's comments from an initial skim.

gnprice · 2025-07-16T23:34:27Z

lib/widgets/action_sheet.dart

+  }
+}
+
+class ViewReadReceiptsButton extends MessageActionSheetMenuItemButton {


An intermediate commit has:

@override void onPressed() { // TODO: open read receipts sheet }

So it shows the button but the button doesn't do anything. That's a buggy state, so let's avoid it.

That commit is quite small, so we can just squash it into the later commit that gives this button something to do.

gnprice · 2025-07-16T23:38:32Z

lib/widgets/action_sheet.dart

+/// A plain text widget for a bottom sheet with a multiline UI string.
+///
+/// Comes with built-in 16px horizontal padding.
+///
+/// Figma:
+///   https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3481-26993&m=dev
+class BottomSheetPlainText extends StatelessWidget {


This name is very general. It's hard for me to tell what sort of situations this is meant for, or what it does.

If I want plain text, that's what Text does, right? It's not clear how the bottom sheet interacts with that.

What about the new name BottomSheetInfoText?

That does suggest more information. Can you explain a bit more in the widget's doc?

Updated its doc.

gnprice · 2025-07-16T23:39:43Z

lib/widgets/read_receipts.dart

+      child: status == FetchStatus.loading
+        ? CircularProgressIndicator()
+        : status == FetchStatus.error
+          ? BottomSheetPlainText(


This chain of ?: would be best as a switch (status). That way it's clear we handle all the possible values.

gnprice · 2025-07-16T23:41:27Z

lib/widgets/read_receipts.dart

+    final localizations = ZulipLocalizations.of(context);
+    final designVariables = DesignVariables.of(context);
+
+    return Center(


Does this Center end up doing anything in the three cases other than CircularProgressIndicator? I suspect it doesn't.

It also centers the plain text in the sheet, especially vertically.🙂

gnprice · 2025-07-16T23:42:54Z

lib/widgets/read_receipts.dart

+// TODO: deduplicate the code with [ViewReactionsUserItem]
+@visibleForTesting
+class ReadReceiptsUserItem extends StatelessWidget {
+  const ReadReceiptsUserItem(this.pageContext, {


nit: widget parameters should always be named (with very rare exceptions like Text)

gnprice · 2025-07-16T23:44:28Z

lib/widgets/read_receipts.dart

+                  padding: EdgeInsets.symmetric(vertical: 8),
+                  itemCount: userIds.length,
+                  itemBuilder: (context, index) =>
+                    ReadReceiptsUserItem(context, userId: userIds[index]))));


The child widget here calls this argument pageContext, but the context we're passing looks like just its own parent's context. If this context works, then I expect the child's own context would work equally well.

(This is also an example of why it's good for widget parameters to be named — if this said pageContext: context, then the contrast would jump out more.)

gnprice · 2025-07-16T23:44:56Z

lib/widgets/read_receipts.dart

+/// https://zulip.com/api/get-read-receipts
+Future<GetReadReceiptsResult> getReadReceipts(ApiConnection connection, {
+  required int messageId,
+}) {
+  return connection.get('getReadReceipts', GetReadReceiptsResult.fromJson,
+    'messages/$messageId/read_receipts', null);


This should go in lib/api/ :slightly_smiling_face:

gnprice · 2025-07-17T00:02:05Z

lib/widgets/read_receipts.dart

+              color: designVariables.title,
+            ).merge(weightVariableTextStyle(context, wght: 600))),
+          if (status == FetchStatus.success && receiptCount > 0)
+            StyledText(


Hmm interesting.

This wouldn't be a satisfactory solution to all of #1285, because this appears to fire up a whole XML parser for each one of these widgets, which is going to be expensive. That's acceptable in this context, where there's only one of these at a time (and it isn't a very common part of the UI anyway). It wouldn't be OK for something that could appear in, say, message content, where you might scroll past a lot of them.

sm-sayedi · 2025-07-30T18:51:18Z

Thanks @gnprice for the previous review. New changes pushed. Marked for Chris's review.

This fixes the two inconsistencies flagged in discussion: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/bottom.20sheet.20.22Cancel.22.2F.22Close.22.20button/near/2216116 > I think it's reasonable to have both labels, but I think we should > choose them differently than now: > > - "Cancel" when the sheet is about doing an action: [etc.] > > - "Close" when the sheet just presents information or nav options: > [etc.]

See Vlad's SVG on CZO: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/View.20Reactions.20ModalBottomSheet/near/2025350

…s test So we can add another test that uses it.

I experimented with using Semantics to help write human-centered tests, and I ended up adding some configuration that actually seemed to make a reasonable experience in the UI, at least in my testing with VoiceOver. Fixes zulip#740.

Figma link: https://www.figma.com/design/jbNOiBWvbtLuHaiTj4CW0G/Zulip-Web-App?node-id=7352-690&t=OXW354YOc17R7B6G-0

This way, it can be used for purposes other than being a header, such as in the next commits, when read receipts fail to load, we use this widget in the center of bottom sheet to give feedback to the user. Additional changes: - Adds a TextAlign property for controlling the alignment. - Removes vertical padding so callers can decide about it. But keeps the horizontal padding of 16 as it is the default padding of our bottoms sheets.

sm-sayedi · 2025-07-31T11:41:04Z

Pushed a new revision with tests included.

Fixes: zulip#667

chrisbobbe

Thanks, this is exciting! Comments below, from reading all the commits except the last, which I've read only partially so far.

chrisbobbe · 2025-08-01T01:03:12Z

lib/widgets/icons.dart

@@ -48,143 +48,146 @@ abstract final class ZulipIcons {
  /// The Zulip custom icon "check".
  static const IconData check = IconData(0xf108, fontFamily: "Zulip Icons");

+  /// The Zulip custom icon "check_check".
+  static const IconData check_check = IconData(0xf109, fontFamily: "Zulip Icons");


icons: Add check_check icon, from Web Figma file

I started a discussion on this; I don't think there's a change to make right now, but please link to it in the commit message for context: #mobile-design > read-receipts icon in #F1706 @ 💬

chrisbobbe · 2025-08-01T01:17:53Z

lib/widgets/action_sheet.dart

-/// A header for a bottom sheet with a multiline UI string.
+/// A plain text widget for a bottom sheet with a multiline UI string.
 ///
-/// Assumes 8px padding below the top of the bottom sheet.
+/// Use it to present short, non-interactive explanatory messages to the user,
+/// such as an error message or other feedback.
+///
+/// Comes with built-in 16px horizontal padding.
 ///
 /// Figma:
 ///   https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3481-26993&m=dev


action_sheet: Rename BottomSheetHeaderPlainText to BottomSheetInfoText

This commit decreases the number of widgets that

declare the job of following one specific source in the Figma,

do that job correctly, and

have an implementation that's easy to compare to that source. 🙂

Let's make a new widget for the new not-header thing, and keep this widget for a header. This would be an example of the principle "prefer duplication over the wrong abstraction", I think 🙂, which Greg taught me about a while ago: https://chat.zulip.org/#narrow/sender/2187-Greg-Price/search/wrong.20abstraction

aside

I could more easily imagine BottomSheetHeaderPlainText growing a "title/body" variant, if that's helpful for following this part of the Figma—

; it would still have the "header" abstraction, which seems right. In fact, that might be helpful—the Figma's 18px horizontal padding kind of looks like a mistake to me; the plain-text version has 16px. A shared widget would be a natural place for a code comment that mentions both kinds of headers. We'd want to make sure the widget still has a clear, coherent interface, with a link to the Figma source for the new behavior.

chrisbobbe · 2025-08-01T02:05:36Z

assets/l10n/app_en.arb

+  "@actionSheetReadReceipts": {
+    "description": "Title for the \"Read receipts\" bottom sheet."
+  },
+  "actionSheetReadReceiptsReadCount": "This message has been <link href=\"https://chat.zulip.org/help/read-receipts\">read</link> by {count} {count, plural, =1{person} other{people}}:",


We shouldn't give translators control of the URL. If the Help Center page moves, we should only have to make a one-line change in our source code, instead of going through and changing language data for all the languages. 🙂

See web's string, can we do something like <z-link> there?

{num_of_people, plural, one {This message has been <z-link>read</z-link> by {num_of_people} person:} other {This message has been <z-link>read</z-link> by {num_of_people} people:}}

Also I think we should link to https://zulip.com/help/read-receipts instead of that page on CZO.

Also IIUC that plural syntax works, but I think we'd normally do

{count, plural, =1{1 person} other{{count} people}}

in case that's a more helpful model when translating into a language that varies the word order among the one/two/few/many/other cases? (I'm not actually sure if such languages exist actually.)

Yeah, in fact best to have the whole sentence inside the plural block — like it is in that example from web.

That way if e.g. the form of the verb needs to change too, the translator has that flexibility.

That way if e.g. the form of the verb needs to change too, the translator has that flexibility.

nit: IIUC the translator has that flexibility already; they can choose to write the translated string however they want, including by re-scoping the plural block (or even omitting it which would probably be wrong). But by putting the whole sentence in the plural block in English, we're potentially helping by minimizing the things a translator has to think through in order to make a correct translation for their language.

True. So it's more that we're giving them a model that makes it easier to see how to apply that flexibility.

chrisbobbe · 2025-08-01T03:23:00Z

lib/widgets/action_sheet.dart

@@ -720,6 +721,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes
      ReactionButtons(message: message, pageContext: pageContext),
    if (hasReactions)
      ViewReactionsButton(message: message, pageContext: pageContext),
+    ViewReadReceiptsButton(message: message, pageContext: pageContext),


We should store and check the org's "Enable read receipts" setting before including the button; see realm_enable_read_receipts in https://zulip.com/api/register-queue

chrisbobbe · 2025-08-01T03:28:05Z

lib/widgets/read_receipts.dart

+      overlayColor: WidgetStateColor.resolveWith((states) =>
+        states.any((e) => e == WidgetState.pressed)
+          ? designVariables.contextMenuItemBg.withFadedAlpha(0.20)
+          : Colors.transparent),


How about

overlayColor: WidgetStateColor.fromMap({ WidgetState.pressed: designVariables.contextMenuItemBg.withFadedAlpha(0.20) }),

?

I updated my #1700 with that in response to #1700 (comment) 🙂 (there might be other things that would be helpful to bring over from ViewReactionsUserItem in my current revision there)

chrisbobbe

And here's a full review of that last commit. Generally looks great; small comments below.

chrisbobbe · 2025-08-01T20:16:48Z

lib/widgets/read_receipts.dart

+  @override
+  Widget build(BuildContext context) {
+    return SizedBox(
+      height: 500,


In the view-reactions sheet, I used 400 with a TODO(design) tune. I'd like to be consistent between the two places unless there's a reason not to, but I don't mind which value is used; 500 is fine if you think it's better.

chrisbobbe · 2025-08-01T20:51:41Z

lib/widgets/read_receipts.dart

+  @override
+  void didChangeDependencies() {
+    super.didChangeDependencies();
+
+    tryFetchReadReceipts();
+  }


This means we can end up fetching multiple times, so I think my ideal version would have generation logic like in lib/model/message_list.dart, to invalidate stale fetches. If results arrive out of order, we don't want to get stuck showing old results when we know we have results from a fetch we made more recently.

I think that can be a followup though—I can write up a more detailed proposal later, or perhaps just send a draft to demonstrate, once this work is merged.

What are the reasons we would want to do a new fetch because a dependency changed?

I think it may only be if there's a new store. So this could go in an onNewStore method instead.

Then the generation-like logic can just be: when getting the response, check if the store we had at the start of the request is still the current store (with identical). If not, ignore the response.

chrisbobbe · 2025-08-01T20:52:47Z

lib/widgets/read_receipts.dart

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


Let's link to the Figma, so it's easier for readers to check the implementation against the spec.

chrisbobbe · 2025-08-01T20:59:37Z

lib/widgets/read_receipts.dart

+    final designVariables = DesignVariables.of(context);
+
+    return Padding(
+      padding: EdgeInsetsDirectional.fromSTEB(18, 16, 18, 8),


I think this is a rare case where I disagree with the Figma (link). Can we change the horizontal padding to 16 and the bottom padding to 4, like in BottomSheetHeaderPlainText (which in main follows this in the Figma)?

Here's a before/after of my proposal (from the current revision):

Before After

You can leave a code comment explaining that we prefer consistency with that other header, with a link to this GitHub comment.

chrisbobbe · 2025-08-01T21:01:49Z

lib/widgets/read_receipts.dart

+
+  @override
+  Widget build(BuildContext context) {
+    final localizations = ZulipLocalizations.of(context);


nit: we generally write zulipLocalizations instead of localizations; that way it's distinct from other localizations like MaterialLocalizations.

chrisbobbe · 2025-08-01T21:05:19Z

lib/widgets/read_receipts.dart

+                  style: TextStyle(
+                    decoration: TextDecoration.underline,
+                    color: designVariables.link,
+                    decorationColor: designVariables.link),


The Figma specifies some other details about the underline style. Let's follow any of those that we can (e.g. decorationThickness, I think) and write TODO(upstream)s for the others.

chrisbobbe · 2025-08-01T21:09:54Z

lib/widgets/theme.dart

@@ -171,6 +171,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
    labelEdited: const HSLColor.fromAHSL(0.35, 0, 0, 0).toColor(),
    labelMenuButton: const Color(0xff222222),
    labelSearchPrompt: const Color(0xff000000).withValues(alpha: 0.5),
+    labelTime: const Color(0x00000000).withValues(alpha: 0.49),
+    link: const Color(0xff066bd0),


Hmm, I didn't find this in the list of Figma variables that I usually look at.

Turns out it's defined in the "Zulip Web UI kit": https://www.figma.com/design/msWyAJ8cnMHgOMPxi7BUvA/Zulip-Web-UI-kit?node-id=0-1&p=f&vars=1&var-id=b9c211a7c212568bc84139413454dd8dd07321fb%2F833-56&m=dev

It has distinct light/dark values; we should use both. And maybe add a comment like

link: const Color(0xff066bd0), // From "Zulip Web UI kit"

chrisbobbe · 2025-08-01T21:21:58Z

test/widgets/read_receipts_test.dart

+  Finder findUserItem(String fullName) => find.ancestor(of: find.text(fullName),
+    matching: find.byType(ReadReceiptsUserItem));


How about:

Suggested change

Finder findUserItem(String fullName) => find.ancestor(of: find.text(fullName),

matching: find.byType(ReadReceiptsUserItem));

Finder findUserItem(String fullName) =>

find.widgetWithText(ReadReceiptsUserItem, fullName);

?

chrisbobbe · 2025-08-01T21:26:14Z

test/widgets/read_receipts_test.dart

+            delay: transitionDurationObserver.transitionDuration
+                   + const Duration(milliseconds: 100));


It takes some nonlocal reasoning to work out which nav-transition this is about, and I notice that we're always passing the same duration here.

How about replacing prepareReceiptsResponse with prepareReceiptsResponseSuccess(List<int> userIds) and prepareReceiptsResponseError(Object? httpException), and control the delay in setupReceiptsSheet?

chrisbobbe · 2025-08-01T21:27:42Z

test/widgets/read_receipts_test.dart

+      check(find.byWidgetPredicate((widget) => switch(widget) {
+        ProfilePage(userId: 1) => true,
+        _                      => false,
+      })).findsOne();


Can be two lines instead of four I think 🙂

check(find.byWidgetPredicate((widget) => widget is ProfilePage && widget.userId == 1)) .findsOne();

gnprice · 2025-08-01T22:30:17Z

lib/widgets/read_receipts.dart

+      final result = await getReadReceipts(store.connection, messageId: widget.messageId);
+      // TODO(i18n): add locale-aware sorting
+      userIds = result.userIds.sortedByCompare(
+        (id) => store.userDisplayName(id),


Relatedly: in general we never want to keep a reference to the store across an await and use it again after, because the old store might already have been disposed. Instead, after an await we should look up the store afresh using a context.

sm-sayedi marked this pull request as draft July 15, 2025 11:23

chrisbobbe reviewed Jul 15, 2025

View reviewed changes

sm-sayedi force-pushed the 667-read-receipts branch from 7c6c52c to d05ac9d Compare July 16, 2025 18:19

gnprice reviewed Jul 17, 2025

View reviewed changes

gnprice mentioned this pull request Jul 26, 2025

msglist: Support viewing who reacted to a message #1700

Merged

sm-sayedi force-pushed the 667-read-receipts branch 2 times, most recently from 2ed033d to 2b769fd Compare July 30, 2025 18:45

sm-sayedi marked this pull request as ready for review July 30, 2025 18:46

sm-sayedi requested a review from chrisbobbe July 30, 2025 18:49

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

sm-sayedi assigned chrisbobbe Jul 30, 2025

chrisbobbe and others added 11 commits July 31, 2025 16:08

icons: Add see_who_reacted, from Vlad on CZO

128f139

See Vlad's SVG on CZO: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/View.20Reactions.20ModalBottomSheet/near/2025350

action_sheet: Implement BottomSheetHeaderPlainText, following Figma

b8136f8

inset_shadow test [nfc]: Pull PaintPatternPredicate helper outside it…

cccb6a8

…s test So we can add another test that uses it.

inset_shadow: Implement start/end shadows, not just top/bottom

a1bea9d

emoji_reaction test [nfc]: Pull out some data for other tests to use

927eabf

msglist: Support viewing who reacted to a message

7aadc11

I experimented with using Semantics to help write human-centered tests, and I ended up adding some configuration that actually seemed to make a reasonable experience in the UI, at least in my testing with VoiceOver. Fixes zulip#740.

icons: Add check_check icon, from Web Figma file

87518f1

Figma link: https://www.figma.com/design/jbNOiBWvbtLuHaiTj4CW0G/Zulip-Web-App?node-id=7352-690&t=OXW354YOc17R7B6G-0

deps: Add styled_text

7fe57ae

api: Add route getReadReceipts

76fe89f

sm-sayedi force-pushed the 667-read-receipts branch from 2b769fd to 24a4e8e Compare July 31, 2025 11:39

action_sheet: Add 'View read receipts' button

961fcd8

Fixes: zulip#667

sm-sayedi force-pushed the 667-read-receipts branch from 24a4e8e to 961fcd8 Compare July 31, 2025 16:17

chrisbobbe reviewed Aug 1, 2025

View reviewed changes

gnprice reviewed Aug 1, 2025

View reviewed changes

		Finder findUserItem(String fullName) => find.ancestor(of: find.text(fullName),
		matching: find.byType(ReadReceiptsUserItem));

		delay: transitionDurationObserver.transitionDuration
		+ const Duration(milliseconds: 100));

Show read receipts #1706

Are you sure you want to change the base?

Show read receipts #1706

Conversation

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

Uh oh!

Screenshots

Screen recording

Uh oh!

alya commented Jul 15, 2025

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 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!

sm-sayedi commented Jul 30, 2025

Uh oh!

sm-sayedi commented Jul 31, 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!

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!

sm-sayedi commented Jul 15, 2025 •

edited

Loading

chrisbobbe Aug 1, 2025 •

edited

Loading