Before this patch, textareas would expand but not re-collapse. It appears that the callback functions were not called.

I have a hard time grokking jQuery's source code, but some effects ("fx") code did change between 1.0.2 and 1.1.1. In particular, I peered at the "slideDown", "animate", and "fx" functions.

http://jquery.com/dev/svn/tags/1.0.2/src/fx/fx.js
http://jquery.com/dev/svn/tags/1.1.1/src/fx/fx.js

CommentFileSizeAuthor
#6 collapse.js_.patch634 bytesstarbow
collapse_jquery_1-1-1.patch828 bytesnjivy

Comments

joshk’s picture

+1 for this; patch applies cleanly to HEAD and works as advertised.

Code is backwards-compatible to jQuery 1.0, so I suggest committing it.

rszrama’s picture

+1. Solved the advertised issue and another I was having with buggy expansion. I knew it had to be something simple... thanks for posting!

dries’s picture

Version: 6.x-dev » 5.x-dev
Status: Needs review » Reviewed & tested by the community

Committed this to CVS HEAD. Not sure if it needs to be backported to Drupal 5.

m3avrck’s picture

Since HEAD, AFAICT is still running jQuery 1.0.4 this patch will work equally well for Drupal 5 :-)

Steven’s picture

Status: Reviewed & tested by the community » Needs work

About 5.x. For the umpteenth time: WON'T FIX.

Drupal 5 will never be distributed with jQuery 1.1. Because of the lack of backwards compatibility, manually upgrading the jQuery library is a bad idea. It might break modules you downloaded. Or, you'll probably start using jQuery 1.1 features in your own modules, which cause them to break for other users. Keeping Drupal 5 broken under jQuery 1.1

When we included jQuery, this was the policy that was set down. Unless we actually switch over Drupal 5 to jQuery 1.1, none of these 1.1 patches are relevant for Drupal 5.

The only reason people are even considering this is because it's JavaScript. Would anyone think of committing a patch to Drupal 5 "so that it is easier to patch in the Drupal 6 menu system"?

Anyway. None of this really matters because the patch does not work as advertised. It completely removes the "step" callback, with no explanation or excuse given. This is needed to ensure smooth scrolling as the fieldset expands. With this patch applied, the browser jumps down instantly, which destroys the visual continuity and makes the animation useless.

This is even clearly visible if you just look at the patch. It's a mystery to me why this was committed.

starbow’s picture

Status: Needs work » Needs review
StatusFileSize
new634 bytes

The speed/duration just needs to be moved inside the options. I think it's a side effect of of the new easing support.

dries’s picture

Shouldn't we upgrade our jQuery library in core then? Looks like we need this patch + a new version of jQuery.

Roi Danton’s picture

I would recommend that, along with that patch posted a while ago: http://drupal.org/node/118846
jQuery has improved alot since 1.0.4.

webchick’s picture

Having spent the better part of 2 days troubleshooting SimpleFeed module because of bugs in the third-party parser it uses, I would have to emphatically agree with Steven here. Developing against a moving target third-party library is absolutely no fun. It makes sense for HEAD; doesn't make any sort of sense for our stable branch.

m3avrck’s picture

Yes I agree.

jQuery is beginning to slow down and while the latest 1.1.2 is considerably better than 1.0, it doesn't make sense to put this in Drupal till we are closer to a code freeze. jQuery is evolving at a much faster rate than Drupal and the next major releases 1.2/1.3 should be out before D6 code freeze.

Our best bet is to wait till mid-May before the code freeze and then figure out the latest/best jQuery version to use and then upgrade the JS in core -- which for the most part, isn't all that hard.

In the mean time, I've had great success with jQuery 1.1.2 + compat plugin on the Drupal 5 sites I run.

dman’s picture

Damn.
I've had the same issues when trying to upgrade to some nifty jquery effects. (drag-drop shopping cart)
While I agree the moving target is (as ever) annoying, I think the programming philosophy in jquery must allow some graceful degradation. Can't we just stick a functionality-check in there? Or overlay both libraries?

Yeh OK, I guess I'm wrong. :( Forget it. Glad to see the patch exists however, I'll use it locally even if it's not committed.

gábor hojtsy’s picture

The latest, not yet committed and very small change is already included in the catch-all Jquery 1.1.x update patch here: http://drupal.org/node/120291 I don't know whether starbow's patch is still compatible with jquery 1.0 as shipped in Drupal 5, so this patch only makes sense if it is. Otherwise this is a duplicate of #120291.

Gurpartap Singh’s picture

Did you mean http://drupal.org/node/146462 ? :)

this one becomes duplicate as changes are already made to that one, and some more to collapse.js. But maybe it can serve for drupal 5 if people plan to use jquery 1.1.x with it, but still an option is to use jquery_update module which updates that function for you.

drumm’s picture

Status: Needs review » Closed (won't fix)

Only jQuery 1.0.x will be shipped with Drupal 5.x.

codenamerhubarb’s picture

Thanks. This patch really saved me.