Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Almost working patch attached - drag and drop poll module!
This patch changes a lot of lines because I needed to add a "weight" field to the form (which *should* have been there before)...
Comment | File | Size | Author |
---|---|---|---|
#18 | poll_drag_and_drop.patch | 20.58 KB | paul.lovvik |
#16 | poll_drag_and_drop.patch | 21.35 KB | paul.lovvik |
#14 | poll_drag_and_drop.patch | 20.58 KB | quicksketch |
#12 | poll_drag_and_drop.patch | 18.6 KB | quicksketch |
#8 | poll_drag_and_drop.patch | 22.45 KB | quicksketch |
Comments
Comment #1
catchsubscribing.
Comment #2
quicksketchWell I hit this one hard to try to get it completed. Unfortunately, adding ordering was not as easy as anticipated. Sorting the table rows was easy, but saving the values was another matter. Because poll module actually used the choice position (chorder column) as the key for saving results, rearranging the choices was not a safe or efficient thing to do.
This patch adds drag and drop, and also the necessary database change to support this rearranging:
- The 'chorder' column is converted to 'weight', since it no longer is the exact position for a choice.
- The 'chid' column is added to the poll_votes table and is used as the key for joining, rather than chorder.
Conveniently, a primary key was added to the poll_choices table when the schema changes went in, even though this key wasn't being used for anything. Now it is. :)
I tested this pretty well, and running an upgrade path from 5 to 6 with several polls and thousands of votes pre-registered. Browser checked in Opera, Safari, and Firefox.
Comment #3
quicksketchThis patch sets the weight column to size = 'tiny' and fixes a spacing issue
'#parents' => array('choice', $key,'chtext'),
becomes'#parents' => array('choice', $key, 'chtext'),
.Comment #4
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedNice! This is ready to... :-)
Comment #5
catchJust to note both this and the upload patch claim system_update_40 - should one or both be rerolled assuming the other also gets in? We're short for time now.
Comment #6
Gábor HojtsyComing with multiple database changes, I hand this off to Dries for consideration. I was asked to stand by the exception list.
Comment #7
quicksketchReroll to make this system_update_6041() after upload module changes.
Comment #8
quicksketchPoll module might have gotten the pocket-veto for D6, but just in case this patch adds table checks for poll_votes and poll_choices in the system update, similar to the followup patch for upload.
Comment #9
Gábor HojtsyUnfortunately, this
- adds new functionality to polls (ordering was never possible)
- changes much more in the database then what the upload did
So I need to postpone this for Drupal 7. Please understand that we try to focus on fixing what is already there in Drupal 6 and we have gone quite far with drag and drop support anyway. Let's focus on what we have there, and do not introduce new features. As far as I know, we added drag and drop to all orderable lists to wherever it was possible, and we even made uploads orderable, although they were never orderable before. Get this in early in Drupal 7.
Polls are in need of other improvements anyways. While you can add options, and even modify poll counts, you cannot remove items them for example. This shows even more when an ordering feature is added.
Comment #10
Gábor HojtsyComment #11
quicksketchNo problems Gábor, I agree with the decision here. It would have been possible to change the schema less and simply add another weight column in poll module as we did with upload, but it wasn't the clean fix. Poll module is still in need of some pretty substantial changes, but we significantly improved it during this release cycle so we can be happy with that. :)
Marking CNW since it'll definitely need changing when development opens for 7 again. Thanks!
Comment #12
quicksketchPoll module is back baby :)
Here's a reroll for Drupal 7. Still fixes the database schema to use (the previously unused) chid auto-increment column instead of the position in the list of questions.
Even if Poll gets removed from core, I wouldn't be offended. But while it's here and we have a fix, might as well get it done.
Comment #13
Dries CreditAttribution: Dries commentedThe poll module is not going to get removed from core.
I don't think the attached patch is correct: the weight field is missing from the database scheme upon a fresh install?
Comment #14
quicksketchThanks Dries. "chorder" definitely needed to be renamed to "weight". I didn't re-roll the patch quite right. Here's one that works on a clean install of Drupal and an upgraded D6 site.
Comment #15
Dries CreditAttribution: Dries commentedI think this looks OK, except for the choice number on the form. It's somewhat odd/silly to show the choice number. Otherwise looks great.
Comment #16
paul.lovvik CreditAttribution: paul.lovvik commentedI have hidden the choice numbers. Note that on a new installation I found it necessary to clear the cached data before this change takes effect.
The drag and drop operation is working great!
Comment #17
quicksketchWe need to remove the JS that affected the reordering of these numbers also.
Comment #18
paul.lovvik CreditAttribution: paul.lovvik commentedGreat point. Now choice number is not being used at all. I have removed the choice number rather than simply hiding it and accordingly I changed the CSS class name to "choice-flag".
Comment #19
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. :-)
Comment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #21
Damien Tournoud CreditAttribution: Damien Tournoud commentedI'm sorry to reopen that issue, but I need some clarifications on changes introduced in this patch.
More precisely,
$poll->choice
is now keyed bychid
and so are poll edit and poll vote forms. That means the key is no longer relative but is now shared between all polls. You can't easily reference the first choice of a poll anymore.This caused issue in testing, because we don't have yet a mechanism to pull form choices outside of a form. A patch was committed to fix poll.test a while ago [1], but it's nothing more than a mere hack: we just hope that the choice ids will come out the way we want them to.
Is there a way to change that behavior and to use node-relative keying instead of absolute keying there? After all the natural primary key of
{poll_choice}
is (nid
,weight
).[1] http://drupal.org/node/260492
Comment #22
quicksketchWe don't have to "hope" that the choice ids come out in the correct order, since they're always sorted by weight. You can easily get the first item in a list by using reset():
Or you could use array_values() if you want to convert them to ordered indexes. The change is more helpful than the old approach since we can easily convert them to meaningless zero-indexed arrays at any time. In any case, I'd suggest opening a new issue rather than asking a question this issue.