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

Feature/3676 Conduct Polls #4861

Merged
merged 23 commits into from
Mar 31, 2014

Conversation

JannikStreek
Copy link
Contributor

refs #3676

Still in progress, but I would like to get some feedback. So far I thought the best place to create polls would be directly at the post creation (see screens), so you can add a poll to a post by clicking the poll icon. Any thoughts?
stream

stream2

@Flaburgan
Copy link
Member

Nice design! I think there is no debate about where to display the creation step, but what about the result?

@Flaburgan
Copy link
Member

Oh, please do not commit the db/schema.rb file.

@jhass
Copy link
Member

jhass commented Mar 18, 2014

Oh, please do not commit the db/schema.rb file.

This PR includes a migration.

@JannikStreek
Copy link
Contributor Author

i will create a mock for that as well

@jhass
Copy link
Member

jhass commented Mar 18, 2014

Another thing you might want to consider early in your work is that diaspora is a distributed network, and as such you need to ensure that polls are correctly serialized and deserialized on both ends, as well that poll answers are spread to all recipients of the post.

@goobertron
Copy link

Nice start! Thanks for doing this.

On the design front, just some minor points:

I'd like to see a radius for the corners of the boxes to match that on the Publisher window.

I'd suggest changing the text as follows:

  • Poll creation -> Add a poll
  • Answer -> Option 1, Option 2, ... Option n.
  • Remove answer -> Remove option
    Add answer -> Add option

It would also be great if the poll creation pane only appeared when a user selected to add a poll, so perhaps a 'Poll' button underneath the publisher (although we have to be careful that space isn't going to fill up with too many buttons...).

But a really promising start. I guess the federation aspects might be the most difficult challenges you face.

Look forward to seeing your mock-up of how a user can select an option once the poll has been published, and how results will be displayed.

@Flaburgan
Copy link
Member

@goobertron

It would also be great if the poll creation pane only appeared when a user selected to add a poll

I guess that's why there is a poll icon ;)

@goobertron
Copy link

Oh, I didn't see that (oops). Nice design.

@jaywink
Copy link
Contributor

jaywink commented Mar 19, 2014

Awesome - looking forward to this maturing :)

@JannikStreek
Copy link
Contributor Author

Ok the federation part should work, the only thing what might be missing are notifications like "your friend just answered poll xy with z". Is that worth a message?

Here is how the answering of the poll could look like:
poll

@jhass
Copy link
Member

jhass commented Mar 22, 2014

Hm, it looks quite nice though we don't have that speechbubble design anywhere else, also I think I'd prefer keep the post footer at the bottom. What about something that goes in line with the OpenGraph attachment?

def not_already_participated
if self.poll == nil
return
end
Copy link
Member

Choose a reason for hiding this comment

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

You can simply make this one line: return if poll.nil?

@jhass
Copy link
Member

jhass commented Mar 22, 2014

Nice progress so far! Keep up that great work :)

@JannikStreek
Copy link
Contributor Author

Next try
poll2

poll_answers = params[:poll_answers]
if params[:poll_answers].instance_of? String
poll_answers = [params[:poll_answers]]
end
Copy link
Member

Choose a reason for hiding this comment

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

A little trick here is doing poll_answers = [*params[:poll_answers]], which can also save the local if you chain the each right to it.

@jhass
Copy link
Member

jhass commented Mar 24, 2014

Alright I think we're getting into the final phase here. My open points would be:

  • Did you consider to use the bar-graph or line-graph icon from Entypo? We think about completely switching to it, so it would be good to choose new icons from it.
  • Did you verify poll creation works in the bookmarklet?
  • Does the poll creation form still look like the first screenshot? If so I'd consider making the question input not longer than the right border of the remove answer links
  • Please add some Jasmine specs for the Javascript changes
  • Also consider writing a cucumber scenario or two, so we have some integration tests for the feature.

@goobertron
Copy link

Does the poll creation form still look like the first screenshot? If so I'd consider making the question input not longer than the right border of the remove answer links

I'd agree with this. If you can make it so that the input area extends downwards when text wraps over a line, it would be good to have all the inputs the same width.

One suggestion (I think I might prefer this) would be to replace 'Remove answer' with a small grey 'x', and extending the input areas to the right.

I'd suggest changing the colour of the bars in the results graph to something paler/less strong colour, perhaps $background-grey. But that's a nitpick according to taste.

@MrZyx, what do you think about having something (a title, for e.g.) above the results to indicate that what is below is a poll in Diaspora and not an OpenGraph preview? At the moment, because the design is so similar to OG previews, it doesn't (to me) stand out as something different. I like the streamlining of the designs, that'll look good in the stream, but something to distinguish a Diaspora poll would be good. How about using a slightly larger version of the poll icon from the publisher, placing that to the left of the title (and possibly in $blue)?

@jhass
Copy link
Member

jhass commented Mar 24, 2014

I don't think we need to differentiate, I would even go as far as removing the "hide poll" link. As a user I shouldn't care whether this comes via OpenGraph or via Diaspora, if both are available they just stack. That I can interact is clear through the button in our button design. I'd might consider making the radio boxes a bit bigger.

@goobertron
Copy link

I agree we can remove the 'hide poll' link. Which reminds me of something I meant to suggest earlier: should we (at least by default) hide the poll results until someone has voted? This would help avoid people's votes being biased by the current results (I can't remember the name of this statistical/psychological effect).

One possibility, if we want in some cases for people to be able to see the results, would be to introduce an option in the poll creation form, 'Allow people to see the results before voting'. Current results would be hidden by default, but if this option was selected on creation, there would be a 'show results' link available to people viewing the poll.

I think it needs to be more immediately obvious how to vote in the poll. I hasn't realised the circles on the left were radio buttons. Hiding the results graph by default would, I think, make it more obvious that these are buttons and that's how you vote.

@goobertron
Copy link

@JannikStreek, I've just pulled this into my local copy of Diaspora, and have a few comments. I hope they're useful.

  • In the version I pulled, the poll icon in the publisher is not the same shade of grey as the locator and camera icons. It should be, I think.
  • The tooltip behind the poll icon in the publisher has a missing translation. Change lines 31 & 32 of app/views/publisher/_publisher_blueprint.html.haml to point to config/diaspora/en.yml. Rather than adding a new line to en.yml reading 'Create poll', you could probably point this tooltip to publisher.poll.add_poll_answer so it reads 'Add a poll', which would be fine here.
  • In the poll creation form, the title, 'Add a poll' should align with the left-hand edges of the input boxes.
  • The default text in the inputs, 'Question', 'Option 1', etc, should, I think, be in grey rather than black.
  • When I add some text to Option 1, let's say I add 'Pasta', and click 'Add option', the next input automatically has 'Pasta' inserted into it. This shouldn't happen. (Even if I open a second option before filling in Option 1, any extra options I open after filling out Option 1 have Option 1's text inserted into them.)
  • When I preview the post after creating the poll, the poll isn't shown in the preview. I don't know whether that needs to be the subject of a fresh PR, but I think the poll should be visible in the post preview.
  • When I have your branch checked out, it is not possible to post a message whether or not it contains a poll. The publisher closes and the preview disappears as if it's about to post the message, but nothing appears in the stream, even after a refresh. So something is going wrong there. Possibly you haven't got that far in your coding yet! If you have, and feel that it should be published, I can post the error messages if that would help.

@JannikStreek
Copy link
Contributor Author

@goobertron thanks for testing! The version you just checked out contained some bugs and glitches, so some of your remarks are already fixed, I guess. The correct icon will follow soon.
Regarding the placeholders: They are grey for me (like in the image I posted in the beginning). Also the 'add a poll' title is aligning with the inputs for me, which browser are you using?
Regarding the preview: I also thought about that, however, you can't style polls (just yet :-) ) with markdown, so I don't really see the big advantage for showing the poll in the preview...

By the way: I think the word you were looking for is "suggestive questions" ;) You are right that some people might get influenced. However, I dislike polls where I can't see the result until I voted. Also, I guess most poll questions are not that critical...so if some people are influenced I don't think that's a big issue. But we can discuss that, I just chose here usability over better results.

@MrZyx poll creation in the bookmarklets is not working right now, I will have a look at that. Can you point me to the icon you are referring ?

@Flaburgan
Copy link
Member

About the show / hide results, I think a setting to allow / disallow readers to be able to see the results if they didn't vote is too complicated. In my opinion, simply hide the results and add a link "Show the results" even if the person didn't vote should be enough.

@goobertron
Copy link

I've just pulled the latest code. The post is now made successfully, but the poll still doesn't appear.

The poll icon is still a much lighter grey than the other icons in the publisher.

One thing I forgot to mention before: I think there should be two options available by default. In fact, I think it shouldn't be possible to publish a poll with fewer than two options. That's totalitarianism! ;) Maybe we could allow polls with one option just for fun, but the default should be a minimum of two options so that one doesn't have to add an option just to give people two options to choose from.

@JannikStreek
Copy link
Contributor Author

Actually its already not possible to submit a poll with only 1 option..but i guess you are right with the 2 standard options, i will fix that.

@goobertron
Copy link

Thanks. I really like your work!

@goobertron
Copy link

And not just because you obey me... ;)

@JannikStreek
Copy link
Contributor Author

:P Thanks

@JannikStreek
Copy link
Contributor Author

Ok some pictures of the most recent version (tested on firefox+chrome):

Creating a poll:
createpoll2

Voting:
vote

Show me the results:
vote2

Already voted:
vote3

The bookmarklets thing and tests are still missing.

@jhass
Copy link
Member

jhass commented Mar 31, 2014

git rebase -i should help you in that.

@JannikStreek
Copy link
Contributor Author

ok fixed, I guess I pulled in the wrong moment ;)

@jhass
Copy link
Member

jhass commented Mar 31, 2014

Okay, great, let's just wait for Travis one last time.

jhass added a commit that referenced this pull request Mar 31, 2014
@jhass jhass merged commit faad980 into diaspora:develop Mar 31, 2014
@jhass
Copy link
Member

jhass commented Mar 31, 2014

Thank you very much, looking forward to the follow up PRs :)

@@ -65,3 +65,6 @@ dump.rdb

#Rubinius's JIT
*.rbc

#IDE
diaspora.iml
Copy link
Member

Choose a reason for hiding this comment

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

Oh and I accepted this but as a little tip for the future: You can have a clone local ignore file by editing .git/info/exclude

@jaywink
Copy link
Contributor

jaywink commented Mar 31, 2014

Awesome to see this merged in - nice to have new big features land to users! :) Thanks!

@Flaburgan
Copy link
Member

Really nice contribution! I hope we are still going to see great work like that coming from you :)

@Flaburgan
Copy link
Member

I'm going to pull that in diaspora-fr.org, if you want to test it in production, you can log in using testaccount / testaccount. Do podmins need to do something special or a simple pull / asset precompile / database migration / bundle is enough? If something is needed, it has to be add to the changelog.

@goobertron
Copy link

Yes, thanks a lot @JannikStreek. Hope you've got the Diaspora bug now and will be contributing more in the future.

@svbergerem
Copy link
Member

@JannikStreek Awesome! Thank you for your work. I agree with @Flaburgan and hope that you will continue contributing. :)

@goobertron
Copy link

@Flaburgan, pulling into my test setup it was the normal procedure you've listed. Hopefully the same for production setups.

@JannikStreek
Copy link
Contributor Author

thanks, I'll try to contribute more in the future :-)

@JannikStreek JannikStreek deleted the feature/3676-conduct_polls branch March 31, 2014 19:17
@Flaburgan
Copy link
Member

Just a little idea here, I want your opinion before opening an issue. Should at least the question of the polls be added to the notification email when someone comment on the post?

@jhass
Copy link
Member

jhass commented Mar 31, 2014

Hm, no I think that's not useful.

@JannikStreek
Copy link
Contributor Author

It might be if somebody asks a question in the comments referring to the poll, but you are unaware that the post included a poll. But in that case you could check that on diaspora directly, i guess.

@jhass
Copy link
Member

jhass commented Mar 31, 2014

And I guess you'd recognize your own post that had a poll.

@Flaburgan
Copy link
Member

Oh, another point: the poll is not present in the single post view.

@JannikStreek
Copy link
Contributor Author

Damn :D I will create a new PR for that in the next couple of days.

@svbergerem
Copy link
Member

@JannikStreek The SPV uses Bootstrap so these progress bars might help: http://getbootstrap.com/2.3.2/components.html#progress

@Flaburgan
Copy link
Member

@JannikStreek sorry I didn't find another way to contact you, we have our IRC monthly meeting today at 19h30 UTC on #diaspora-meeting on freenode, if you wanna join us :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants