Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.When I first started working with form builder I found some issues with using sortable on nested elements. Dragging elements into fieldsets never really worked quite right.
This was because sortable uses intersect to determine where to place the element. If the element overlaps the top 50% it goes above and if it overlaps the bottom 50% it goes below. Fieldsets had the unique situation of also having an internal element that was attempting to follow the same rules. This meant that there was usually a 1px tall region where the fieldset would "jump" and then if I released the mouse button it would place the element within the fieldset.
Another failing I found at the time was that the scoll option wasn't being respected because of a bug in the version of jQuery UI that was being used in D7 at the time. This has been changed in D7 core and is no longer an issue, but the first concern still is.
The solution I arrive at was essentially an implementation of draggable and droppable that works very similar to sortable. A placeholder is put before every form element and then one at the bottom of the form. If the form height is taller than all of it's elements for whatever reason, the last placeholder will fill that space. I've found the interaction with this implementation to be much better than the previous sortable usage. This also adds the ability for users to click a field in the palette and have it added to the end of the form.
Patch is attached and rolled against HEAD.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | form_builder_dnd2-d6.patch | 15.51 KB | quicksketch |
| #13 | form_builder_dnd2-d7.patch | 15.47 KB | quicksketch |
| #12 | form_builder_dnd2-d6.patch | 15.51 KB | quicksketch |
| #12 | form_builder_dnd2-d7.patch | 15.43 KB | quicksketch |
| #10 | form_builder_dnd2.patch | 15.49 KB | quicksketch |
Comments
Comment #1
quicksketchI've tried this patch out several different ways, applying to the latest D6 and D7 branches but even after manual merging, this patch causes quite a few different issues.
- I'm unable to drag downward with an element more than about 1/3 from the bottom of the window.
- Grabbing an element sometimes will grab its parent element too.
- After dropping a field, the edit form opens automatically.
- Plus things are just a bit more jerky than before.
Obviously this code works okay on Gardens, but applied to the project and then trying to just use the example form, this code doesn't work like you'd expect. I tried exporting a Gardens site and then diffing directly against what you guys are using, but that code doesn't seem to work quite right outside of the UI enhancements you guys have on top.
I thought I commented on this issue once before, but it doesn't look like any of the CSS changes are necessary in this patch.
Comment #2
james.elliott CreditAttribution: james.elliott commentedOne of the motivations for this patch was that jQuery UI sortables wasn't properly scrolling the page in the version of jQuery UI that was in D7 at the time. Now, the opposite seems to be true.
I still think this needs improvement. Check out this screencast of the fieldset "flickering". http://screencast.com/t/hGJYO9FYmj
Comment #3
quicksketchTurns out this was just because I had opened and closed Firebug. False alarm on this one.
Comment #4
james.elliott CreditAttribution: james.elliott commentedI've spent some time on this now. The "flickering" is due to the sortable refreshing the position of its drop targets on every mouse move. Especially with tall form items the element they are overlapping jumps so that they are no longer overlapping when the form item's placeholder is expanded and pushes the bottom element down. I tried to clean this up using sortable and wasn't able to get it to work satisfactorily.
This new patch uses Draggable and places Droppable targets in between the form elements. The Droppable targets are absolutely positioned and will become the drop target when the pointer is over them. I found that this made placing a form element much more reliable than using Sortable. It also allowed me to use the same functions to govern the behavior of elements in the form as well as elements being dragged into the form.
I've also trimmed down the CSS changes. There were some unnecessary ones in the last patch. The ones here are for the purpose of handling the hidden drop targets and the class change from ui-sortable to ui-draggable.
Comment #5
quicksketchYeah I definitely agree that some more work is necessary on getting fieldsets working, but so far I just haven't been able to get these patches to work as advertised. I rerolled your latest patch (it had a conflict against 7.x-1.x) and I've attached it here. After applying, drag and drop becomes noticably less reliable. See this video:
http://www.youtube.com/watch?v=vK81kS_jZ3c
Comment #6
james.elliott CreditAttribution: james.elliott commentedWeird that the patch wouldn't apply. I was developing off D7.0 and a fresh checkout of 7.x-1.x. Perhaps it's because I'm using TortoiseGit to create patches.
I think there are two usability issues that still remain with this patch.
I still have a bit of company time to work on this so I'll try to get a patch for at least the first of those issues rolled today.
Comment #7
james.elliott CreditAttribution: james.elliott commentedNew patch attached. Hopefully one that can be applied properly.
For the empty fieldset issue, I set the text color to transparent when hovered to provide visual feedback.
To fix the draggable jump, I set the appendTo property for the draggables so that their helper would be appended to the #form-builder element. This seems to have solved the jumping issue.
Comment #8
james.elliott CreditAttribution: james.elliott commentedoops, forgot the patch.
Comment #9
quicksketchYour patches still aren't applying against a clean 7.x-1.x branch. :(
Comment #10
quicksketchOkay here's a rerolled patch that applies to 1.x-dev, please base your further work off of it as it also fixes the documentation for checkFieldsets and cleans up code style in a few places.
Problems still existing:
- Text still is shown when you hover over fieldsets. (Note sure where your "transparent" text comes in here. I'd probably suggest you use the CSS property for "visibility: hidden" to make the item disappear but keep the space on the page.)
- Mouse still disconnected from dragging item at times.
- Sometimes two drop targets are active at the same time but only one will actually accept the dropped item.
New video:
http://www.youtube.com/watch?v=ltlJ-Vg8uXc
Comment #11
quicksketchComment #12
quicksketchOkay here are some new patches on this. Now that I understand the approach, I really like it! I added some additional fixes especially around dealing with a single field in a fieldset and moving it out of that fieldset. All of the problems in #10 are also fixed.
I'm just about ready to commit these changes but unfortunately this has had a weird side-effect on fields dragged in from the palette. The original fields are drag around fine for re-ordering, but dragging a field that has been added from the palette ends up with a 1px height when re-ordering it. Strange. Anyway this is great progress so I figured I'd post the patches while they're in a stable condition.
Comment #13
quicksketchAh, turns out this was just a minor issue in that I'd tried to do "the right thing" and had added the context variable to the selector of
var = $formbuilder = $('#form-builder');. Turns out this was intentionally omitted since otherwise it doesn't work on new fields added to the form.All in all, this looks great. Thanks so much James for your work, this is definitely a big improvement. I know that not all your changes are in yet (and I did cut some from this issue too), so please open new issues for further changes. Committed to both branches.
A side-effect of all this: The D6 version now requires jQuery UI 1.7 (not 1.6), I'll update the project page to reflect this change.