There's a javascript mechanism that shows/hides only the form fields relevant to the selected link-type.

However, while working on a link-type for the #871064: Making flaggings fieldable feature, I discovered that this mechanism has some bugs:

  • It only works for <input> with one parent .form-item (i.e, no support for textareas and radios/checboxes).
  • The settings aren't grouped in fieldsets, so if javascript isn't enabled, all will be displayed in one big mess.
  • The initialization code (that hides fields on page load) doesn't work correctly.

I've replaced that mechanism with a much simpler one. It doens't try to be smart with "rel" attrributes. It's a simple mechanism that works. A programmer providing a link-type should put its settings fields inside a fieldset with an appropriate HTML id.

And I moved the settings "Confirmation form" fieldset into the "Display options" fieldest, where it belongs.

The patch is for D7 (because it uses #id, not #attributes[id]; I'll backport it when this one gets committed).

CommentFileSizeAuthor
#8 fixdep6.diff4.02 KBmooffie
#8 fixdep7.diff4.07 KBmooffie
#2 ScreenFlow.mov680.4 KBwebchick
link-options.diff4.36 KBmooffie

Comments

quicksketch’s picture

quicksketch
he/him
commented

Sounds good to me. Could you go ahead and provide the Drupal 6 patch also? I prefer to have both of them maintained at the same time so that patches keep as much consistency as possible between D6/D7.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new ScreenFlow.mov680.4 KB

The animation is pretty jumpy for me (FF 3.6, OSX). See attached video.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

Also, is there a reason we're not using the D7 #states system for this? See http://drupalcode.org/viewvc/drupal/contributions/modules/examples/form_... for an example.

And I guess that's a "needs work". :)

mooffie’s picture

mooffie commented

The animation is pretty jumpy for me (FF 3.6, OSX). See attached video.

Thanks for the effort.

I actually don't see a problem in that video. I'll take your word that something bad is happening there.

So I guess one solution is to get rid of the slideUp()/slideDown() and just use hide()/show()? Nate, are you fine with this?

(BTW, it's not this patch that introduced slideUp/slideDown, so it's possible the animation was jumpty before too.)

Also, is there a reason we're not using the D7 #states system for this?

I had a look at D7's #states today. Seems nice. But I also need to hide/show the introduction line ("The selected link type may require these additional settings:"), and I haven't yet figured out how to do this (I opened a new issue, #875740: #states: documentation for "remote conditions" is wrong).

mooffie’s picture

mooffie commented

I actually don't see a problem in that video. I'll take your word that something bad is happening there.

BTW, the "smoosh down" happens because the page gets shorter (because the fieldset shrinks). So the page is scrolled up. This will happen even if we use hide() instead of slideUp().

But maybe there's something elementary I don't get. I can be dense sometimes.

mooffie’s picture

mooffie commented
Status: Needs work » Needs review

Nate, are you ok with my proposal?

I proposed to use the same patch but to get rid of slideUp/slideDown (so that nobody sees jumpy animation).

I can't use D7's #state because I don't know how to easily hide/show the introduction line.

I want us to resolve this issue because the #871064: Making flaggings fieldable feature needs to expose its link options and it's not quite feasible with the current dependency code.

quicksketch’s picture

quicksketch
he/him
commented

That patch looks good, getting rid of sliding is also a preferred by me. However we need to apply the changes to Drupal 6 and Drupal 7 at the same time, so I'm not at all concerned about not using #states since it's not available in D6.

mooffie’s picture

mooffie commented
StatusFileSize
new fixdep7.diff4.07 KB
new fixdep6.diff4.02 KB

Here's the patch, with versions for both D6 and D7.

There are two differences between the two:
- '#value' vs '#markup'
- ['#attributes']['id'] vs ['#id']

mooffie’s picture

mooffie commented

Nate? Are you fine with the patch?

If so, do you want me to commit it? And every other RTBC patch?

My work on #871064: Making flaggings fieldable is almost done, but it won't be easy for reviewers to to handle its patch while these issues are pending.

quicksketch’s picture

quicksketch
he/him
commented

Hi Mooffie, sorry I've been busy (as usual). Please use your own discretion and make changes as necessary. Drupal 7 patches are particularly welcome, as I've stopped actively testing against Drupal 7 because it kept changing on me and I got fed up with it (I'm not using it yet for any projects so I don't have much incentive to chase HEAD). Just be sure all patches are tested and applied against D6 at the same time (if applicable). The 2.x versions should be feature (and API)-equivalent between the two branches whenever possible.

mooffie’s picture

mooffie commented
Status: Needs review » Fixed

Committed.
http://drupal.org/cvs?commit=407826
http://drupal.org/cvs?commit=407828

The 2.x versions should be feature (and API)-equivalent between the two branches whenever possible.

OK.

quicksketch’s picture

quicksketch
he/him
commented

Sweet, thanks again mooffie :D

Status: Fixed » Closed (fixed)

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