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.

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

Add newly-starred messages live when viewing starred-messages narrow #1006

Open
gnprice opened this issue Oct 18, 2024 · 1 comment
Open
Labels
a-model Implementing our data model (PerAccountStore, etc.) a-msglist The message-list screen, except what's label:a-content
Milestone

Comments

@gnprice
Copy link
Member

gnprice commented Oct 18, 2024

This is like #818, but for StarredMessagesNarrow and a message getting starred, via UpdateMessageFlagsEvent.

On the other hand when a message gets unstarred, we don't want to remove it from view if the user is looking at the starred-messages narrow. Leaving it in view is handy as an "undo" mechanism.

@gnprice gnprice added a-msglist The message-list screen, except what's label:a-content a-model Implementing our data model (PerAccountStore, etc.) labels Oct 18, 2024
@gnprice gnprice added this to the Post-launch milestone Oct 18, 2024
@gnprice
Copy link
Member Author

gnprice commented Oct 18, 2024

In zulip-mobile we do this but there's a bug where if the newly-starred message is one that we don't already have locally on the client, we breach an invariant by adding a dangling message ID to the list of starred messages:

I think we'll naturally avoid any bug like that when implementing this functionality in zulip-flutter, because MessageListView carries a list of Message objects rather than just a list of message IDs. (This ultimately comes down to the fact that zulip-flutter's data structures for the Zulip data model are generally mutable, whereas they're immutable in zulip-mobile.) But in any case, that bug makes a reminder to systematically handle all possible cases when we implement this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-model Implementing our data model (PerAccountStore, etc.) a-msglist The message-list screen, except what's label:a-content
Projects
Status: No status
Development

No branches or pull requests

1 participant