zulip-flutter icon indicating copy to clipboard operation
zulip-flutter copied to clipboard

Support polls (read-only)

Open PIG208 opened this issue 1 year ago • 5 comments

Stacked on top of #823.

Fixes: #165

PIG208 avatar Aug 12 '24 19:08 PIG208

For ease of cross-reference: the previous PR #823 originally contained these changes too, and had a round of UX review starting with these screenshots: https://github.com/zulip/zulip-flutter/pull/823#issuecomment-2266101540

gnprice avatar Aug 13 '24 05:08 gnprice

Moving this to integration review because these changes went through maintainer review in the original PR.

PIG208 avatar Aug 13 '24 20:08 PIG208

Thanks for the review @gnprice! I have addressed them and updated the PR.

PIG208 avatar Aug 14 '24 08:08 PIG208

Partially updated. The tests reorganization is WIP.

PIG208 avatar Aug 16 '24 23:08 PIG208

The tests from "Message.poll" are rewritten in two destinations. Here's a broad overview:

  • "new message with initial submessages" under "handleMessageEvent":

    Contains all the "parse poll" tests. Rewritten so that we handle new MessageEvents for polls. Because these tests are about processing the initial state of the message, there is no SubmessageEvents to handle.

  • "handleSubmessageEvent":

    Contains all the "applyEvent" tests. Each of them got moved to the corresponding PollEventSubmessageType group.

Extra note: dropped the "no poll if submessages is empty" test because it is a duplicate of the existing "message has no submessages" test.

PIG208 avatar Aug 17 '24 06:08 PIG208

I have updated the PR. In this update:

  • added some tests back to the commit that introduces Poll, before moving them later;
  • rearrange test grouping.

PIG208 avatar Aug 21 '24 07:08 PIG208

Pushed a minor formatting update.

PIG208 avatar Aug 22 '24 23:08 PIG208

Thanks for the revision! Those changes all look good. All the commits before the last one are now ready, so I'm going to go ahead and merge them.

I won't make it to a review of the last commit today, though, before I go on vacation: 798f1b36f poll: Support read-only poll widget UI.

So please send that as a fresh PR. Let's also have @chrisbobbe do the first review on that — I know a similar commit was in #823 which he reviewed before, but it's been a bit of a marathon series of changes :slightly_smiling_face: (which is natural because this is a complex feature), so he may have new comments to make when looking at it with fresh eyes.

gnprice avatar Aug 22 '24 23:08 gnprice