I'd like to see an optional question field, that could be used to fill in the poll question instead of the title field.
In my current use of binary polls we always use the title as an ordinary title, like "Taxes for 2007" and than enter the actual poll question at the end of the body, which could be something like "Should the tax be raised by 50 cents for 2007?". It works but it would be more elegant to store the question in a separate field, and allow for better control of theming etc. We also have polls that always have the same question, like "should we vote in this issue". That fits pretty bad as a title.
Here is my suggestion for implementation:
* Store as 'question' in {advpoll}, default is empty.
* Make it a setting per content type, e.g. "Use title as poll question", checked by default (our current behavior).
* If the setting is unchecked, a new field "poll question" is introduced at the node edit page above the choices. It's still optional.
This way, are current way of doing it is default, no upgrade path is needed.
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | advpoll_question_6.patch | 8.28 KB | anders.fajerson |
| #21 | advpoll_question_5.patch | 8.58 KB | anders.fajerson |
| #14 | advpoll_question_4.patch | 8.39 KB | anders.fajerson |
| #12 | advpoll_question_3.patch | 8.23 KB | anders.fajerson |
| #11 | advpoll_question_2.patch | 8.17 KB | anders.fajerson |
Comments
Comment #1
ChrisKennedy commentedI think it sounds like a fair idea.
Comment #2
anders.fajerson commentedOk, I start working on it.
Comment #3
anders.fajerson commentedHere it is. Most headache was which tag to use around the question. I settled for
<p class="question">.<strong>or<h3>didn't seem semantically correct, although they have the benefit that the question "pops out". Since we don't have our own stylesheet (we use poll.css) I let it be up to the user to style it if wanted.Please also test the upgrade path.
I wait with the "Use title as poll question" content type setting until http://drupal.org/node/164887 gets in, so setting as "code needs work".
Comment #4
ChrisKennedy commentedThis looks solid.
I tested the upgrade path and it worked - nice, I didn't realize you could use db_add_column in mysql.
advpoll_node_info() should change the title_field from "question" to "title".
Should we allow HTML in the question field in the same way we do for the choices (use the body filter)?
In an Ajax vote the status result is displayed after the question. It might look better to show it before the question... or maybe not.
We need to manually set the maxlength of the question field to 255 (since it defaults to 128).
Comment #5
anders.fajerson commented1. I'm not sure about this. As "Question" will be the default, my idea was to make it up to the user tho change it (it's done on the same page, something i fixed in a previous patch), maybe add a note about it in the description under "Use title as poll question" (which is checked by default).
2. I think I prefer plain text, just as the original title. The description should be enough? If we use HTML the database field should be changed to 'text'.
3. Yeah, I just didn't want to add a check for this...
4. Fixed.
Comment #6
ChrisKennedy commentedAh gotcha, that makes sense. The only other thing is that it would probably be good to add a brief doxygen header to the two new functions.
Comment #7
anders.fajerson commenteddb_add_column()is only used for pgsql in core after further investigation. While it seems to work for both we are probably best off usingupdate_sql()for mysql.Comment #8
anders.fajerson commentedClose tag...
Comment #9
ChrisKennedy commentedYeah I was trying to use it for the write-in patch but I couldn't find any non-pgsql file in which it was declared.
Comment #10
anders.fajerson commentedI want this in before the beta. Added some missing stuff and fixed the upgrade path.
Comment #11
anders.fajerson commentedWait, we have a CSS file now! (Lets see if someone complains about the generic class name)
Comment #12
anders.fajerson commentedRe-rolled after biggest and most religious patch ever.
Comment #13
ChrisKennedy commentedYay religion :P
On this patch:
1. I think the question field should go before the description.
2. For the CSS classname, as you say it seems likely that we will get conflicts with certain random modules. What about putting some advpoll class container around the poll to ensure that we're styling the right question?
3. There should be a comma at the end of the question field in advpoll.install.
I've been testing it and seriously cannot get it to add the question to the database. I have no idea why this is; I've checked out different things and it seems like it should work. It works for you?
Comment #14
anders.fajerson commented1. Are you sure? Hmm... I'm sort of listing them in them in the order they are rendered. I like having the choices in context.
2. Agree. I just don't a good way of doing that. As said before it more an more feels like we should define our own node.tpl.php. But there is probably another way of doing it. In the meantime I made a little more specific: div.node div.content p.question
3. Fixed
4. Strange. Yes it works here from what I can tell.
Comment #15
ChrisKennedy commentedOkay, I tested it again it's working - not sure what the deal was last time. I was probably on the wrong site or something.
1. If I disable the "use separate question" option for a content type, and a node already has a question defined, it still shows up on the node page, but not on the edit screen. Should we handle this case somehow? We could a) not display the question in the theme if the content type doesn't have it enabled, b) delete all custom questions when a content type's question support is removed, or c) assume that users should handle this situation appropriately. I think option "a" might be best.
2. Need to fix that
if(spacing error. I meant to point this out earlier but forgot.3. I see how the current question field location makes sense. Originally I had been thinking that it would be displayed before the description, but I think the current way is also good.
4. For the question CSS we should probably support it being displayed in a block also.
5. I think we should consider making this turned on by default. What do you think?
Comment #16
anders.fajerson commented1. The nice thing of doing it the way we do now ("option d: Do nothing") is that you can safely jump between the two "modes" without risking that a previous poll will loose context. But I agree that it can be confusing if someone comes in later and want to edit a previous poll, and finds that the question is inaccessible. Maybe option e then, "Always show the question field on the edit page if a question has been (previously) defined". In that case it could be confusing if someone removes a question, saves, edits again and then the question field magically has disappeared (seem like a rare case though). Option f is to always show the question field (remove the setting on the content type page).
4. True, I tend to forget that we have a block.
5. Hard. Personally I will use it, but I don't know about other user cases. If we do this we could probably go with option f above. The negative thing is yet another setting on the edit page...
Comment #17
ChrisKennedy commentedHmmm, in that case maybe we should opt for option G! This would be to add a "use_question" column for each poll that is set to the mode at the time the poll is created, that way clearing the question field won't cause it to disappear. Otherwise I think option A is the simplest - if the admin changes the setting for the content type shouldn't they want old polls to not show the question any more?
Comment #18
anders.fajerson commentedNo columns if possible... I actually mostly think this is a non issue. Either you use the option or not, you don't flip back and forth on a site. So our solution (if any) should be as simple as possible.
An example where it would be a bad idea to hide the question field: Earlier a lot of polls are created with the optional question field turned on. Here is one example poll:
Title: Issue number 1
Question: Should we install metal detectors in our shool?
Choices: 1. Yes
2. No
2 years later it is decided that the title field would be enough when creating polls. So the optional question field is turned off. This specific poll, e.g found in the "archive", would then look like this:
Title: Issue number 1
Choices: 1. Yes 80%
2. No 20%
Still, it's a rare case. Let's make whatever we choose a themable function (if possible) so it can be overridden.
Comment #19
ChrisKennedy commentedThen maybe we should do option e, because the question clearing issue is the rarest case of them all. Otherwise when you disable the question field you have no way to edit the question on old polls.
Comment #20
anders.fajerson commentedI prefer option e. I'll add that in the next patch.
Comment #21
anders.fajerson commentedSpace error and block fixed. Added option e.
Comment #22
ChrisKennedy commentedOkay this sounds good. Three last comments:
1. That switch in advpoll.install has a spacing problem.
2. The !empty() check is not optimal because it ends up unnecessarily stopping someone from entering "0" as the question. It seems to me that $node->question will always be defined since it's added in advpoll_load, so we should just check the value, e.g. $node->question !== ''.
3. Since we're going with option e, what about adding a drupal_set_message() to notify the user IF the message is cleared and the content type is set to not show questions? That way we cover even the rarest case. But maybe we don't need to even worry about this.
Comment #23
anders.fajerson commented1. Fixed
2. Replaced !empty() with $node->question !== ''.
3. drupal_set_message() seems too unobtrusive unless displayed _after_ the form is saved, and that seemed too complicated to do. I tried to add a description, e.g "Note: if cleared.." but (a) no one will read it and (b) I couldn't find a straightforward way to describe it. I really feel it's a rare one the more I think about it.
4. Went with your suggestion and changed the class name to
poll-question. It's not perfect (.poll .question would be better) but I prefer it to the "specific" approach which is a pain to override for a themer.Comment #24
ChrisKennedy commentedLooks good. I'd just take the period off the end of settings title "Use question field." when committing.
Comment #25
anders.fajerson commentedThanks, fixed. http://drupal.org/cvs?commit=83145
Comment #26
igorik commentedHi
I am really missunderstood from new question field. I think that that it will be better if in access rule there will be new rule - "use separate question as title".
I am not sure how many users will be use this new question field, but I think that it will be minority. So I suppose let by default poll with title only, and who will be want separate question, he/she can check this in access roles, and new field will be shown for his/her.
Currently - I don't need this new question field, and when I checked "Use question" checkbox and then I submit it, any progress for me. You wrote that I have to rename title field - but rename for what? I used various names for title field but I have still both fields, title and question. I am completly missunderstood from it. It will be probably my problem, but I think that it can be done more usable as it is now.
I would like really to have just one question field - title field, no other question field. I really don't need it. Title is great, it can be used with token module, or automatic node titles or various other modules.
Thanks
Igor
somvprahe.sk
Comment #27
anders.fajerson commentedHmm, not sure if I understand. Read my initial post if you don't understand the motivation behind this new field. From what I understand from your post you simply don't want to use this new feature, and it's turned off by default so no need for you to do anything really.
Comment #28
igorik commentedMy idea is by default to let the title only, and who will be want new question field, he/she can check it (in access rules or in node type settings)
I think that users who want new question field is really less as who want title field only.
But so I really need is more explanation about rename title field, (or what to do) because now I don't know how to got on poll creating page only title field and no other question field.
Thanks
Igor
Comment #29
igorik commentedand after I upgrade to latest advpoll version, I have by default title AND question fields (both) so I think that is not you wrote that by default is only title field.
Comment #30
anders.fajerson commentedI see. I can confirm that the question field is enabled by default: http://drupal.org/node/181298
Comment #31
(not verified) commented