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

polls: Read-only support #165

Open
gnprice opened this issue Jun 8, 2023 · 2 comments · May be fixed by #823
Open

polls: Read-only support #165

gnprice opened this issue Jun 8, 2023 · 2 comments · May be fixed by #823
Assignees
Labels
a-api Implementing specific parts of the Zulip server API a-content Parsing and rendering Zulip HTML content, notably message contents

Comments

@gnprice
Copy link
Member

gnprice commented Jun 8, 2023

As a first step of poll support (#164), we should render poll messages.

For product context, see https://zulip.com/help/create-a-poll .

For details of this part of the API… unfortunately this is one of the few remaining parts of the Zulip API that is effectively undocumented, and one has to resort to reverse-engineering. Two useful resources for reference are:

  • The implementation in zulip-mobile, which comes from reverse-engineering and embodies notes about the format.
    • This lives mainly in src/webview/html/message.js, in particular widgetBody. (Here "widget" is a term this part of Zulip sometimes uses for an abstraction whose primary example is polls.)
    • See also types Submessage, SubmessageData, WidgetEventData, and WidgetData in src/api/modelTypes.js, and their jsdoc.
  • The implementation in Zulip web.
    • Part of this is in JS code that's shared with zulip-mobile: see web/shared/src/poll_data.js and its neighbor poll_data.js.flow. This is used by the zulip-mobile implementation mentioned above.
    • The web-only code that consumes that poll_data module may also be informative.
@gnprice gnprice added a-content Parsing and rendering Zulip HTML content, notably message contents a-api Implementing specific parts of the Zulip server API labels Jun 8, 2023
@gnprice gnprice added this to the Beta milestone Jun 8, 2023
@gnprice
Copy link
Member Author

gnprice commented Jun 9, 2023

Ideally the information we show here would match what's in Zulip web: in particular for each option it'd show the number of votes and also the list of people who voted for it.

Currently zulip-mobile actually shows less than that:

I think it's probably just about as easy to go ahead and show that information, when we're working on this issue, as to not do so. In that case we should go ahead and show it.

(I think when we added poll support in zulip-mobile I may have been concerned that the list of voters would be too big to comfortably view in the message, the way web does it, on a mobile-size screen. A more mobile-optimized alternative would be to have the list on a separate screen, similar to the list of people who reacted on a message. But given that we never followed up by building that second screen and the interaction to get to it, it would have been better in retrospect to go ahead with the perhaps-unwieldy inline list of voters.)

@gnprice
Copy link
Member Author

gnprice commented May 20, 2024

A small extra implementation note:

There are some ancient messages (2019-01 and earlier) on at least chat.zulip.org which use a different variation of the polls API, which was changed while the feature was still experimental before it was released. When we see such a message, we should just reject it without trying to parse it further.

Specifically that variation involves new_comment rather than new_option. Details at:

@PIG208 PIG208 self-assigned this Jul 17, 2024
@PIG208 PIG208 linked a pull request Jul 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-api Implementing specific parts of the Zulip server API a-content Parsing and rendering Zulip HTML content, notably message contents
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants