Closed (fixed)
Project:
Form Builder
Version:
7.x-1.x-dev
Component:
Form Builder Core
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
1 Feb 2011 at 16:07 UTC
Updated:
28 Jan 2012 at 03:30 UTC
Jump to comment: Most recent file
Comments
Comment #1
james.elliott commentedWrong patch. This one is the correct patch. I'm not sure how those other changes made it in.
Comment #2
quicksketchA lot of this patch has been resolved in other issues, so here's a re-roll with only the necessary changes plus some other changes:
- The variable "fieldConfigureHolder" wasn't ever used, I assume this was actually meant to be "fieldConfigureForm".
- All of the messages and markup being displayed should be run through the theme system. Since this content isn't dynamic in any way, it's easy enough to make these PHP theme functions.
Overall I'm not really confident that this is a good approach. While specifying a custom form location and then letting Form Builder handle everything works, it sure seems like some additional logic may be desired around the tasks of showing the "Loading" indicator or when removing the existing form and replacing with the new form. I'd personally probably just go with an immediate-replace approach. All this up-and-down of the configuration forms seems excessive.
Comment #3
quicksketchTurns out the change from #form-builder-field-configure to .form-builder-field-configure prevented any fields from being saved/updated during editing. I assume there's a reason for the change from a class to an ID, but since only one field can be edited at a time, I don't see the justification here. This update reverts a couple of lines to use IDs again.
Comment #4
quicksketch.
Comment #5
quicksketchWell in the interest of moving this forward, it doesn't seem like we've seen much activity on this front. I backported the changes in #4 to Drupal 6 to result in this patch, which I've committed to both versions of Form Builder.