widgets: Polish poll widget.

[rheaparekh]

I added this PR mainly to familiarize myself with the widget structure, so that I can work on a TODO widget.

Tasks:

• Add basic styling for poll widget
• Add question in the widget
• Restrict question to one user
• Add edit question feature
• /poll question
• Don't vote while adding options

commit 1:

Before and After styling for poll widget:

commit 2

Add question in the widget itself

[showell]

Looks good so far, can you rebase to get the build passing? I think I fixed the test flake.

[rheaparekh]

@showell Updated this! The tests are failing because of lint errors, I'll update it again.

[showell]

The first commit looks great!

For asking the question, do we want to restrict this feature to the person that created the poll?

Also, I think it will be useful to say "/poll What do you want for dinner?" so that the question can be created from the compose box.

[rheaparekh]

Hmm, now that I think about it, restricting question to one user does seem right. Also, /poll What do you want for dinner? sounds good. Maybe we can have both: If the question is not mentioned, we prompt using the input box, and if it is mentioned, we show it directly. What are your thoughts?

[showell]

I provided a bit of feedback on chat:

• structure code more to clearly divide the is-my-poll and is-not-my-poll logic
• add comments about why we show disabled input boxes
• squash two commits adding poll

[showell]

The build seems to be failing here. It may be a test flake.

[showell]

You can probably still squash the questions changes a bit more, so that this PR just has one commit related to the UI side of adding question (so basically squash the "Restrict poll question..." into the prior commit). I'd still leave the other commit by itself (the one that parses what you typed in the compose box).

[rheaparekh]

@showell I have squashed the commit. (I kept the commits atomic to make it easier to review). And yes, the tests were failing because of test flake.

[rheaparekh]

@showell the build is passing now.

[showell]

I think you need to handle that tokens[0] could theoretically throw IndexError or whatever.

I would consider something like this.

def get_widget_type() -> Optional[str]:
    valid_widget_types = ['tictactoe', 'poll']
    tokens = content.split(' ')
    if not tokens:
        return None

    if tokens[0].startswith('/'):
        widget_type = tokens[0][1:]
        if widget_type in valid_widget_types:
            return widget_type
    return None

widget_type = get_widget_type()
if widget_type is None:
    return

[showell]

I tried testing this out, and I got some errors. Sorry, I don't have the errors handy, but basically extra_data was null and it confused some JS module. I think you can reproduce this as follows:

• go to master
• make some widgets (poll, tictactoe, etc.)
• go to your branch
• run the app

I'd also re-work the last commit a bit. I think you can take that function I gave you and extend it to return both widget_type and extra_data as a tuple, so that all the parsing more or less happens up front. And it will return (None, None) if it's not a slash command. I think this will be slightly cleaner and may fix the bug.

[rheaparekh]

@showell Thanks for pointing that bug out. It was basically being caused by no question being set in from the extra_data of the polls started in the master. I have added these lines to fix that. Also refracted the last commit.

[showell]

Merged, thanks @rheaparekh! I really like the experience here for the "other" people. For the person creating the poll, I could see us getting some feedback, like maybe we should leave the question in the input box next to "Edit question" to make it easy to edit typos or something. I'm looking forward to the next deploy!

[veerjainATgmail]

Hi, Any pointers to make it (widgets/poll) work on the mobile app? Thanks, Mahaveer

…

On Mon, Jul 2, 2018, 5:27 AM showell ***@***.***> wrote: Merged, thanks @rheaparekh <https://github.com/rheaparekh>! I really like the experience here for the "other" people. For the person creating the poll, I could see us getting some feedback, like maybe we should leave the question in the input box next to "Edit question" to make it easy to edit typos or something. I'm looking forward to the next deploy! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9833 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMhhrtx-KM5r1HaL3X6_IGEnO8ZwWgolks5uCWICgaJpZM4U3v_q> .