Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

action_sheet: Add "Mark Topic As Read" button #1274

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

action_sheet: Add "Mark Topic As Read" button #1274

wants to merge 2 commits into from

Conversation

lakshya1goel
Copy link
Contributor

@lakshya1goel lakshya1goel commented Jan 12, 2025 •
edited
Loading

This PR adds a "Mark Topic as Read" button to the topic action sheet. The button appears only when there are unread messages in the topic.

Implementation details

  • Button is shown in the topic action sheet when unread messages exist
  • Uses different API endpoints based on server feature level:
    • For FL < 155: /api/v1/mark_topic_as_read
    • For FL ≥ 155: /api/v1/messages/flags/narrow
  • Shows error dialog with server message if the request fails
  • Added comprehensive tests covering:
    • Button visibility conditions
    • API request handling for both feature levels
    • Error handling

Fixes #1225

Screen Recording

WhatsApp.Video.2025-02-01.at.1.02.03.PM.mp4

Additional

  • Updated icon for "Mark all messages as read button" from Icons.playlist_add_check to Icons.mark_chat_read_outlined

See CZO Discussion

Before After

@lakshya1goel lakshya1goel marked this pull request as draft January 12, 2025 16:17
@lakshya1goel lakshya1goel force-pushed the issue1225 branch 2 times, most recently from 56a877f to af14f10 Compare January 30, 2025 09:08
@lakshya1goel lakshya1goel marked this pull request as ready for review January 30, 2025 09:28
@lakshya1goel
Copy link
Contributor Author

Hello, I want to know that should I commit the change of icon of "Mark all messages as read button" separately?

@PIG208
Copy link
Member

PIG208 commented Jan 30, 2025

Yeah, committing it separately sounds good.

PIG208
PIG208 reviewed Jan 30, 2025
View reviewed changes
Copy link
Member

@PIG208 PIG208 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Took a quick look at the implementation and left a comment.

lib/widgets/action_sheet.dart Outdated Show resolved Hide resolved
@PIG208 PIG208 self-assigned this Jan 30, 2025
@PIG208 PIG208 added the maintainer review PR ready for review by Zulip maintainers label Jan 30, 2025
@alya
Copy link
Collaborator

alya commented Jan 31, 2025

Why is the icon different from the one we have in the web app?

Screenshot 2025-01-31 at 11 01 39

@lakshya1goel
Copy link
Contributor Author

Thanks Alya for pointing. I have updated the correct icon and also updated the PR description with latest screenshots.

@lakshya1goel lakshya1goel changed the title Offer Mark topic as read in topic action sheet action_sheet: Add "Mark Topic As Read" button Feb 1, 2025
@lakshya1goel
action_sheet: Add "Mark Topic As Read" button
59fb9c2 
Adds button to mark all messages in a topic as read. The button:
- Appears only when the topic has unread messages
- Uses mark_topic_as_read API for server feature level < 155
- Uses messages/flags/narrow API for server feature level >= 155
- Shows error dialog if the request fails

fixes: zulip#1225
@lakshya1goel lakshya1goel requested a review from PIG208 February 1, 2025 09:30
@lakshya1goel
Copy link
Contributor Author

Thanks for the review @PIG208, pushed the revision, PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PIG208 PIG208 Awaiting requested review from PIG208

Assignees

@PIG208 PIG208

Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Offer "Mark topic as read" in topic action sheet
3 participants
@lakshya1goel @PIG208 @alya