Add javascript 'more' button to Poll (AHAH enable) [#155870]

Easy javascript enabled updates to forms are now (or will be very shorty) possible with the additions to Forms API and ahah.js. This patch is dependent upon the patch in http://drupal.org/node/154398, which adds these abilities to the forms API.

This patch does the following:
- Removes the 'Check this and hit preview form more choices' checkbox (yuk!)
- Adds a 'More choices' button. With javascript disabled this reloads the form with 5 more empty poll choices.
- With javascript enabled, the More choices button immediately adds one more choice to the form via javascript request and HTML insertion directly into the form (AHAH request).

This patch demonstrates additional abilities of the AHAH framework that were not included in the original patch. Rather than replacing form content, it simply appends one more field with the #ahah_method = 'append' property. It uses the #ahah_effect property to slide down the new choice. This exact method can be extended to CCK, making more buttons available for multiple fields without the addition of any new javascript files.

Besides demonstrating additional abilities of the AHAH framework, this patch is also an example of how to use the new abilities of the Forms API. During the AHAH request, poll module retrieves the cached version of the existing form from the database. It updates the cached form and resaves it to the database, so Drupal is aware of the new fields when it handles processing of the form.

This patch also makes changes to poll module's node object. Rather than setting $node->choice as an array of choices, it is renamed to $node->choices for clarity. The $form['choices'] element in the node form previously contained a count of the number of poll choices. It has been removed and replaced with $form['choice_count'], which is no longer a hidden element but just a value set by the Forms API in the non-javascript behavior.

This patch adds a new menu item, which won't immediately be found after applying. To make the javascript menu callback work correctly, I added this hack to index.php to reset the menu on every page load:

require_once './includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
rebuild_menu(); // New line here, rebuild the menu on every page load.

This obviously won't be necessary when this is included as part of poll module initially. Disabling and re-enabling poll module might also do the trick.

Caution again: this patch is dependent on http://drupal.org/node/154398, apply it first, then this one before testing.

Comment	File	Size	Author
#34	poll_ahah_table.patch	10.41 KB	quicksketch
#34
#28	poll_ahah_4.patch	8.16 KB	quicksketch
#28
#22	poll_ahah_3.patch	8.22 KB	quicksketch
#22
#18	poll_ahah_2.patch	9.29 KB	quicksketch
#18
#10	poll_ahah_1.patch	12.92 KB	quicksketch
#10
#3	poll_ahah_0.patch	13.47 KB	quicksketch
#3
	poll_ahah.patch	13.43 KB	quicksketch

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

eaton CreditAttribution: eaton commented 30 June 2007 at 21:20

My heart is warmed. This is exactly the kind of functionality that FormAPI 3 is meant to make possible, and poll module is one of the best candidates. chx and I were able to do a basic compatability update with the FAPI3 patch, but this is what it has been needing. It doesn't fix all the poll.module ugliness, but it does take advantage of this cool new stuff.

I'll take a closer look at the code, but I've been talking back and forth with quicksketch as he worked on the patch and this approach IS in fact FAPI3 best practice.

Comment #2

eaton CreditAttribution: eaton commented 30 June 2007 at 21:22

Status:

Needs review

» Needs work

A quick note -- when resaving things to the form cache it's important that the 'expire' parameter be preserved, or the danger of cached forms building up to fill the table is much greater. I think quicksketch is rolling an update to fix that now.

Comment #3

quicksketch

he/him

CreditAttribution: quicksketch commented 30 June 2007 at 21:27

Status:

Needs work

» Needs review

File	Size
poll_ahah_0.patch	13.47 KB

Patch with $expires properly set on the cache_set() for the updated form.

Comment #4

eaton CreditAttribution: eaton commented 1 July 2007 at 02:16

Status:

Needs review

» Needs work

I've tested this on FF 2.0.0.4 on OSX, and it appears to alternate between *hiding* the choices and *adding another choice*. It's doing the adding correctly, but every other click of the button, all the individual choices disappear, leaving only the 'more choices' button. pressing it again makes them reappear. Pressing it a third time hides the choices, and a fourth time makes them reappear, and so on...

Comment #5

quicksketch

he/him

CreditAttribution: quicksketch commented 1 July 2007 at 02:36

Eaton, that's the behavior before applying the updated patch in http://drupal.org/node/154398. Could you start fresh, apply both patches and try again?

Comment #6

eaton CreditAttribution: eaton commented 1 July 2007 at 02:50

Status:

Needs work

» Reviewed & tested by the community

Misrolled patch for the AHAH patch. That issue has been resolved, and with THAT in place this works perfectly. Very, very cool.

Comment #7

anders.fajerson CreditAttribution: anders.fajerson commented 5 July 2007 at 09:24

Really interesting. We have been doing the exact same thing in Advanced Poll module for Drupal 5 for quite some time now, quite cleanly I if i may say so myself. I have to look into what extra benefits this new AHAH functionality introduces.

Comment #8

quicksketch

he/him

CreditAttribution: quicksketch commented 9 July 2007 at 16:00

The original AHAH patch has now been committed (http://drupal.org/node/154398).

This patch needs no changes for the updated AHAH patch (http://drupal.org/node/157752).

Comment #9

Dries CreditAttribution: Dries commented 9 July 2007 at 16:38

Status:

Reviewed & tested by the community

» Needs work

Are you sure this patch still works? I tried it on my development copy and I got a Javascript error using FF2. Also, the button is now labeled 'more choices'. It's probably better to make it more active: 'Add a choice'. That seems more explanatory to me.

Care to test this patch a bit more, and to rename the button accordingly? Thanks!

Comment #10

quicksketch

he/him

CreditAttribution: quicksketch commented 9 July 2007 at 17:12

Status:

Needs work

» Needs review

File	Size
poll_ahah_1.patch	12.92 KB

Thanks Dries. I updated the patch to change the name to "Add another choice".

I tested further with a fresh D6 checkout, but it works for me regardless. I did notice however that it doesn't work until the menu is rebuilt, and since clearing the cache is no longer adequate I put that menu_rebuild() in my index.php. It worked after that. Let me know if it's still not working. Thanks!

Comment #11

Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commented 11 July 2007 at 11:39

Status:

Needs review

» Needs work

I tested this patch, as you said.
I added the line menu_rebuild(); in my index.php file, flushed the cache and cache_* tables from my database, and got to node/add/poll where I checked the 'Add another option' button.

In FF 2 and Safari 2/3 the progressbar is displayed. saying 'Please wait...', and that's sort of it.
It hangs there, and does not display more choice options...

BugZilla said this:
"Warning: Assignment to undeclared variable reponse
Sourcefile: /head/misc/drupal.js, line 229"

I'm setting this back to 'Code need work'..

Comment #12

quicksketch

he/him

CreditAttribution: quicksketch commented 11 July 2007 at 18:45

Firefox seems to work fine for me. Safari works as you report. This is a problem with that redirectFormButton() function in drupal.js (which is removed in my later patches). You'll notice that upload module doesn't work in Drupal 5 at all. That function just doesn't work in Safari. After applying the new ahah patch, the jquery update patch, and then this one, the scenario improved a little bit but still not quite right in Safari. I'm going to wait on fixing this patch for Safari until the other AHAH patches and the jQuery upgrade go in.

Comment #13

Dries CreditAttribution: Dries commented 2 August 2007 at 10:48

This patch needs to be re-rolled because http://drupal.org/node/162871 went in. :)

Comment #14

yngens CreditAttribution: yngens commented 23 August 2007 at 06:55

Comment #15

alippai CreditAttribution: alippai commented 16 September 2007 at 14:14

Subscribing.

Comment #16

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 2 October 2007 at 10:47

Is this still in development?

Comment #17

quicksketch

he/him

CreditAttribution: quicksketch commented 5 October 2007 at 14:14

Yep, I'm doing it again now that AHAH part 2 went in. I should have an updated patch shortly.

Comment #18

quicksketch

he/him

CreditAttribution: quicksketch commented 5 October 2007 at 17:05

Assigned:	Unassigned	» quicksketch
Status:	Needs work	» Needs review

File	Size
poll_ahah_2.patch	9.29 KB

As promised here's the new patch to work with the various changes made to Poll module. The good news is the patch is now 30% smaller!

I discovered a flaw in my logic in the last ahah.js patch, added in at the very end of that issue. The conditional display of Safari effects causes an issue when using the .ahah-new-content class but no effect. Safari also had an issue with hiding content before it is rendered on the page, so the content is added to the page first and then hidden. Because this patch will not work properly without the change, I've included that change to ahah.js here. I noticed no side effects on the blocks page in any browsers.

I've tested this patch with identical correct behavior on the following browsers: IE 6/7, Firefox 1.5/2 (Mac, Win, Ubuntu), Opera 9 (Mac, Win, Ubuntu), and Safari 2 (Mac).

Comment #19

moshe weitzman CreditAttribution: moshe weitzman commented 6 October 2007 at 23:47

subscribe ... thanks quicksketch.

Comment #20

Dries CreditAttribution: Dries commented 7 October 2007 at 10:00

1. I think 5 initial comment choices is way too much. We can safely reduce that to two choices, IMO. We have an opportunity to clean-up our cockpit navigation so we should take advantage of this.

2. Instead of a 'Please wait' status bar, I expected the throttler. Can we use the throttler instead: it's cleaner and more consistent.

3. It would be nice if we could put the 'Votes for choice x' textfield next to the 'Choice x' textfield. There is no reason why the 'Votes for choice x' textfield should be two miles long. I know it is not part of the AHAH patch, but it would make for an additional cleanup and bonus points. ;)

4. I'd rename 'Settings' to 'Poll settings' and make it a collapsible fieldset.

Would that still work when Javascript is disabled?

Note that I haven't reviewed the code yet. I started with reviewing the user experience.

Comment #21

BioALIEN CreditAttribution: BioALIEN commented 8 October 2007 at 00:02

Thanks quicksketch. It's about time the poll module receives some love.

Comment #22

quicksketch

he/him

CreditAttribution: quicksketch commented 9 October 2007 at 21:06

File	Size
poll_ahah_3.patch	8.22 KB

Thanks Dries, here's an updated patch with most of the concerns addressed.

1. I think 5 initial comment choices is way too much. We can safely reduce that to two choices, IMO. We have an opportunity to clean-up our cockpit navigation so we should take advantage of this.

Yep agreed. Reduced to 2 initial options.

2. Instead of a 'Please wait' status bar, I expected the throttler. Can we use the throttler instead: it's cleaner and more consistent.

The way we're handing this on the block page is a little silly. We've basically included the progress.js file on the page just to add a single div, then over-ridding the css to make it look like the throbber. Because we're realistically going to want the throbber more often than the bar, I wrote this patch: http://drupal.org/node/181741, which will fix that problem here without changing any code.

3. It would be nice if we could put the 'Votes for choice x' textfield next to the 'Choice x' textfield. There is no reason why the 'Votes for choice x' textfield should be two miles long. I know it is not part of the AHAH patch, but it would make for an additional cleanup and bonus points. ;)

The best way to handle this would probably be in a table. I'm wary of it mostly because it's significantly more effort (theme functions, new .tpl.php file, and the PHP update function for the AHAH has to replace the entire table, rather than just a row). Earl wrote a patch recently that did something similar while adding the views-like up and down arrows. I think we could include all that code without the up and down arrows if we wanted.

4. I'd rename 'Settings' to 'Poll settings' and make it a collapsible fieldset.

Easy enough, done. I also set a weight on the Poll settings to make it appear just below the poll choices.

Would that still work when Javascript is disabled?

Yep! The new FAPI is awesome! It works just the same, but to save the poor js-free user time it adds 5 additional choices every time, rather than just 1. It also validates the form when you click the button, which I've been told there isn't much we can do about that.

Comment #23

quicksketch

he/him

CreditAttribution: quicksketch commented 9 October 2007 at 21:10

Also... the patch mentioned in the previous post is a requirement for this patch to work in Safari. I've removed all the ahah.js fixes from this patch and included them all in http://drupal.org/node/181741, which is solely an AHAH update.

Comment #24

dmitrig01 CreditAttribution: dmitrig01 commented 14 October 2007 at 16:09

That's been committed

Comment #25

catch

he/him

English

CreditAttribution: catch commented 14 October 2007 at 17:19

Just tested this on XP - Firefox 2.0.0.x, IE7, IE6, Opera. and the new Safari 3 beta (also windows).

Everything works as expected, no niggles, massive, massive improvement in user experience. I think it's RTBC but might require a Mac/Linux review so not setting as such yet.

Comment #26

JirkaRybka CreditAttribution: JirkaRybka commented 14 October 2007 at 17:51

Status:

Needs review

» Needs work

I tested it on Firefox 2 / Linux (just user experience, as the code is beyond my knowledge to review properly). With JavaScript active, it works fine and is very nice UI improvement.

But however, with JavaScript disabled, I've two problems:

- The button says "Add another choice", but adds five. Would it be possible to change wording, perhaps "Add another choice(s)", or just simply "More choices"? I think the plural is better, as it looks cool in both cases - with JS you just click it more times, but basically you also use it to add (any number of) choices.

- Without JavaScript, clicking on the button to get 5 more, I get warnings:

    * notice: Undefined index: menu_name in /home/jirux/test-webspace/d6poll/modules/menu/menu.module on line 392.
    * notice: Undefined index: plid in /home/jirux/test-webspace/d6poll/modules/menu/menu.module on line 392.

I'm using the 6.x-dev tarball (2007-Oct-12).

Comment #27

catch

he/him

English

CreditAttribution: catch commented 14 October 2007 at 17:55

I agree "more choices" would be a better wording, it'd also make for a shorter button.

Incidentally, I tested in IE5.5 for fun (javascript on), and got the same undefined index error.

Comment #28

quicksketch

he/him

CreditAttribution: quicksketch commented 15 October 2007 at 03:34

File	Size
poll_ahah_4.patch	8.16 KB

Kudos JirkaRybka for the notice of the menu problem when javascript is disabled. I had thought that this was a problem with menu.module, but it looks like I wasn't using the correct method for updating the $form_state variable in the non-javascript version. This caused the fields added by menu module to loose their changes when a user hit 'Add another choice'.

Before:

  $form_state['rebuild'] = TRUE;
  $form_state['node'] = $form_state['values'];

After:

  node_form_submit_build_node($form, $form_state);

node_form_submit_build_node runs all the following:

function node_form_submit_build_node($form, &$form_state) {
  // Unset any button-level handlers, execute all the form-level submit
  // functions to process the form values into an updated node.
  unset($form_state['submit_handlers']);
  form_execute_handlers('submit', $form, $form_state);
  $node = node_submit($form_state['values']);
  $form_state['node'] = (array)$node;
  $form_state['rebuild'] = TRUE;
  return $node;
}

This is the same function used in node.pages.inc when Previewing the form to update the $form_state. It corrected the menu problems and will probably correct quite a bit of problems that would have occurred in the future.

With the feedback from JirkaRybka and catch, I also renamed the button back to 'More choices'.

Comment #29

catch

he/him

English

CreditAttribution: catch commented 15 October 2007 at 10:53

Status:

Needs work

» Needs review

Comment #30

catch

he/him

English

CreditAttribution: catch commented 15 October 2007 at 11:08

Status:

Needs review

» Reviewed & tested by the community

Just tested this on a fresh install, both with and without javascript enabled. The notices are gone, functionality is fine. And as before I support "more choices" for the button so it makes sense in both contexts. I think this is ready.

Comment #31

JirkaRybka CreditAttribution: JirkaRybka commented 15 October 2007 at 17:11

Works fine now.

The only thing I noticed is that if you click "More choices" on a preview page without JavaScript, it returns back to non-preview version of the form (without preview rendered above). But I'm tempted to say that this is no problem: It's quite an edge case (no JavaScript, adding choices after preview), and it's simple enough to click "Preview" again - which was required for the old checkbox to work anyway, so basically it's no change and no problem for this patch being RTBC. Just wanted to point that out.

Comment #32

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 15 October 2007 at 19:07

Status:

Reviewed & tested by the community

» Needs work

Well, I think it is not a good idea UI wise to have 'More choices' return one more choice only. Would it be hard to properly title the button depending on whether we have JS or not? (I think it would improve the experience a lot).

I also played some with putting the Choice 2: and Votes for choice 2: items by each other (as Dries suggested) with using percentage width and floating, and it seems to work nicely in Firefox. I am not sure a table would be required here, but I agree with Dries that this issue is not related, and this patch is big enough anyway.

Comment #33

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 15 October 2007 at 20:50

I agree with Gábor. Although I'm just getting to know the #ahah property (and I can't really find the docs?), I don't think it's possible to change the name of a button with *just* the #ahah FAPI property? It's definitely possible using a separate .js file.

I'm waiting for quicksketch's response though, because I have the impression his mission was to not have to add those .js files at all...

Comment #34

quicksketch

he/him

CreditAttribution: quicksketch commented 16 October 2007 at 16:22

Status:

Needs work

» Needs review

File	Size
poll_ahah_table.patch	10.41 KB

Ha, okay it seems a little silly to keep dragging this patch along, but here's a revamped patch with the following changes:

- The choices are now put into a table. We loose the slideDown effect, because jQuery doesn't yet support this effect on the table rows (at least in an acceptable way).
- The button changes it's title depending on the availability of javascript with the following line:

  // If we're using the javascript version (99% use-case), change the button
  // title to 'Add another choice' to reflect the javascript behavior.
  drupal_add_js("if (Drupal.jsEnabled) { $(document).ready(function() { $('#edit-poll-more').val('". t('Add another choice') ."'); }); }", 'inline');

Although I'll miss the slide effect, I'm preferring the table version because it puts us in a good position for adding weights or the views-like sort.js (or table drag and drop). The patch doesn't get much longer, but it does add the necessary theme function to render the choices form as a table so a cache-clear will be necessary.

Comment #35

catch

he/him

English

CreditAttribution: catch commented 16 October 2007 at 16:38

OK I applied the patch, tested with FF2, IE7 and Safari 3 beta on XP with and without javascript.

The button change between js and js-disabled works fine, and the text now makes sense in both contexts, which is great! No notices or anything.

Tables look good (my only tiny comment would be the text field for the number is quite long at my resolution, but this is no big deal). I slightly miss the slideDown effect, but the fade in is fine, and that could always be added in later when jQuery supports it for table rows cleanly.

I think this addresses all of Gabor's concerns, and the table is easier to deal with as well, so tentatively RTBC, again.