action: Add dialog to markNarrowAsRead for CombinedFeedNarrow view

[chungwwei]

Fixes: #1858

Still picture

narrow	tap when long press

Screencasts

combined feed narrow

combined_feed_sc.mp4

channel narrow

channel_sc.mp4

mentions narrow

mention_sc.mp4

[chrisbobbe]

Thanks! Comments below.

[chungwwei]

@chrisbobbe Thanks for reviewing!

Squash the 4 commits into one.

KeywordSearchNarrow was the other narrow. Was not looking at the code, but when i tested in the app, the Mark all messages as read button didn't appear in the search view when searched. Would you be okay with me filing a follow up issue for KeywordSearchNarrow?

[chrisbobbe]

KeywordSearchNarrow was the other narrow.

That's not accurate. We should show the dialog when the narrow is not a conversation narrow. A "conversation narrow" is a topic narrow or a DM narrow; see e.g. the top of the Zulip Help Center "Reading conversations" article. These details are provided for you in the issue description and discussions linked from there.

In the zulip-flutter codebase, there are multiple kinds of Narrow other than TopicNarrow and DmNarrow. CombinedFeedNarrow and KeywordSearchNarrow are just two of those.

[chrisbobbe]

Thanks! Comments below.

Since the issue is about the mark-as-read button at the bottom of the message list, I'd like to have some tests in the 'MarkAsReadWidget' group in test/widgets/message_list_test.dart that explicitly target the behavior. (I see you've already made some changes to existing tests there, basically as needed to make them pass.)

How about a new group('confirmation dialog', () {}); inside the 'MarkAsReadWidget' group, with tests like these:

• test('CombinedFeedNarrow: show dialog', () {});
• test('MentionsNarrow: show dialog', () {});
• test('ChannelNarrow: show dialog', () {});
• test('DmNarrow: do not show dialog', () {});
• test('TopicNarrow: do not show dialog', () {});

For the 'do not show dialog' tests, you should be able to use the checkNoDialog helper in test/widgets/dialog_checks.dart.

[chrisbobbe]

Why expectDestructiveActionButton: true?

[chungwwei]

Set to false. Read can be reverted.

[chrisbobbe]

(I've also just filed #2030 and #2031 for future work, but those are out of scope for this PR; I don't expect them to require changes here. 🙂)

[chungwwei]

Thanks @chrisbobbe! it's ready for another review.