Create a more ajax Need more choices

samuelsov - March 12, 2009 - 18:28
Project:Decisions
Version:6.x-0.1-beta3
Component:User interface
Category:feature request
Priority:critical
Assigned:urlisse
Status:closed
Description

Use the same kind of UI than Poll with a button Add more choice...

#1

urlisse - March 13, 2009 - 12:56
Assigned to:Anonymous» urlisse

#2

urlisse - April 3, 2009 - 15:24
Status:active» needs work

Please ignore this patch. This isn't working and never will.

I've altered and slightly enhanced advpoll's dynamic Add/Remove choice feature to make the form dynamic and plugged in the jquery script in the form_alter hook of the decisions' edit form. The jquery appears to be working fine, but the form API doesn't seem to receive the newly added form values. I know I must be missing something... Any ideas?

Attached is a patch for the current HEAD and the decisions-form.js to be dropped in the module's folder.

UPDATE : I abandoned this thread for a solution that uses more standard AHAH handling routines.

AttachmentSize
decisions-form.js.txt 4.35 KB
decisions-6.x-0.1-beta3-399952-1.patch 2.29 KB

#3

urlisse - April 3, 2009 - 15:23
Status:needs work» needs review

The following patch adds an ahah-enabled "Add more choices" button for quickly adding choices to the create/edit decision form. It falls back to adding three buttons when JavaScript is disabled.

To install, apply patch and copy decisions.js file to the decisions module folder.

AttachmentSize
decisions-6.x-0.1-beta3-399952-3.patch 10.22 KB
decisions.js 691 bytes

#4

anarcat - April 3, 2009 - 15:43
Status:needs review» needs work

Okay, we have problems with warning, that may not be related with the patch, but that are probably breaking the submission.

At least, for me, the patch doesn't work: the button shows the little cycling circle, but doesn't do anything.

Loading /decisions/add_choices_js directly gives me:

{ "status": true, "data": "\x3cdiv class=\"messages error\"\x3e\n \x3cul\x3e\n \x3cli\x3enotice: Undefined index: form_build_id in /var/hostmaster/drupal-6.x/sites/all/modules/decisions/decisions_node.inc on line 189.\x3c/li\x3e\n \x3cli\x3enotice: Undefined index: form_id in /var/hostmaster/drupal-6.x/sites/all/modules/decisions/decisions_node.inc on line 190.\x3c/li\x3e\n \x3cli\x3enotice: Undefined index: choice in /var/hostmaster/drupal-6.x/sites/all/modules/decisions/decisions_node.inc on line 196.\x3c/li\x3e\n \x3cli\x3enotice: Undefined index: choices in /var/hostmaster/drupal-6.x/sites/all/modules/decisions/decisions_node.inc on line 196.\x3c/li\x3e\n \x3cli\x3enotice: Undefined index: #default_value in /var/hostmaster/drupal-6.x/sites/all/modules/decisions/decisions_node.inc on line 196.\x3c/li\x3e\n \x3cli\x3enotice: Undefined index: #tree in /var/hostmaster/drupal-6.x/includes/form.inc on line 887.\x3c/li\x3e\n \x3cli\x3enotice: Undefined index: #tree in /var/hostmaster/drupal-6.x/includes/form.inc on line 887.\x3c/li\x3e\n \x3c/ul\x3e\n\x3c/div\x3e\n\x3cdiv class=\"form-item\" id=\"edit-label-wrapper\"\x3e\n \x3clabel for=\"edit-label\"\x3eChoice 1: \x3c/label\x3e\n \x3cinput type=\"text\" maxlength=\"128\" name=\"label\" id=\"edit-label\" size=\"60\" value=\"\" class=\"form-text choices\" /\x3e\n\x3c/div\x3e\n" }

Also note that I had to rebuild the menus to make this work, so maybe we want to make an update_N function to make sure that is done...

#5

anarcat - April 3, 2009 - 15:48
Status:needs work» reviewed & tested by the community

So after testing the patch with admin/settings/error-reporting sent to the log (and not to the screen), the patch actually works so I will open a seperate issue about this and commit the patch.

#6

anarcat - April 3, 2009 - 15:58
Status:reviewed & tested by the community» needs work

@@ -723,3 +729,6 @@ function decisions_reset_form_submit($fo
   drupal_set_message('Votes have been reset.');
   drupal_goto('node/'. $nid);
}
+
+
+

Please keep whitespace patches out of fix patches. There was at least one other instance of whitespace change in the patch, this makes patches harder to read and creates more work for the maintainer which in turns makes the patch less likely to be accepted/reviewed.

Also, it seems that some of the warnings *are* indeed coming from this patch (particularly line 196 in decisions_node.inc) so I will ask you to send another patch fixing those warnings.

I have committed a modified version of the patch, thanks!

#7

urlisse - April 4, 2009 - 03:22

Thanks anarcat for the great feedback.

I set up my environment to E_ALL not to miss those insidious E_NOTICE, but I was unable to replicate the bug.

In the meantime, I'd rather leave those notices in instead of blindly getting rid of them. This issue shouldn't affect production sites.

That said, I now receive a bunch of notices form all over the decisions module (decisions.inc, decisions.module) while editing a node. I've made note of this in the issue http://drupal.org/node/423432 .

#8

anarcat - July 27, 2009 - 22:02
Priority:minor» critical

#9

anarcat - July 27, 2009 - 22:25
Status:needs work» fixed

This now works in CVS, there was a .js missing...

#10

System Message - August 10, 2009 - 22:30
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.