Drupal.formBuilder.clickField should not require that the configure link be a descendant of the first child of the wrapper. Right now it works because the default theming adds the form-builder-title-bar as the first child node of the wrapper. If that is not the case, the link that is selected will be incorrect.

My patch firstly grabs the wrapper by using the closest() method which navigates up the DOM tree and returns the first matching element only. Then it filters the configure links that are descendants of the wrapper to make certain that they are not descendants of two (or more) form-builder-elements. This way we can be sure that the configure link selected is the one that belongs to the element that is clicked on.

CommentFileSizeAuthor
#6 form_builder_click_field.patch692 bytesquicksketch
#5 form_builder_click.patch875 bytesjames.elliott
#2 form_builder_clickField.patch859 bytesjames.elliott
form_builder_clickField.patch901 bytesjames.elliott

Comments

quicksketch’s picture

quicksketch
he/him
commented

I'd like to keep using parents(':first') instead of closest(), as at least currently it's possible use jQuery 1.3 in Drupal 6 with Form Builder but closest() does not exist in that version of jQuery.

I haven't tried this out, but couldn't you use a :not() filter with a selector rather than doing another selector for .form-builder-element .form-builder-element a elements?

-  var link = $(this).parents('div.form-builder-wrapper:first').find('a.configure').get(0);
+  var link = $(this).parents('div.form-builder-wrapper:first').find('a.configure:not(.form-builder-element .form-builder-element a)').get(0);
james.elliott’s picture

james.elliott commented
StatusFileSize
new form_builder_clickField.patch859 bytes

I've rerolled this against HEAD with your feedback.

quicksketch’s picture

quicksketch
he/him
commented

This patch looks good but I'll need to do some testing before committing. Looks like it could have some issues when a field is nested inside of multiple fieldsets (and thus multiple form-builder-elements).

james.elliott’s picture

james.elliott commented

I'm pretty sure that the find() method's context will prevent the ".form-builder-element .form-builder-element a" selector from not working correctly with multiple fieldsets.

james.elliott’s picture

james.elliott commented
StatusFileSize
new form_builder_click.patch875 bytes

You were right to be concerned with this patch. The :not() selector wasn't functioning properly in the most recent patch. Form items nested within fieldsets were causing JS errors.

I've altered my patch to fix this issue.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Fixed
StatusFileSize
new form_builder_click_field.patch692 bytes

Thanks finally committed. Comment changes and prefixing the wrapper variable with $ for consistency. Applied to both branches.

Status: Fixed » Closed (fixed)

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