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.Hi All,
If I have a nested table inside a draggable table I get the following error:
cell[0] is undefined
[Break on this error] if (cell[0].colSpan > 1) {Looking at the source, it seems to me the columnIndex and headerIndex calculation should be based in the immediate element of their parent, or not? So the correct selector would be:
var columnIndex = $('> td', cell.parent()).index(cell.get(0)) + 1;
var headerIndex = $('> td:not(:hidden)', cell.parent()).index(cell.get(0)) + 1;(I added the "> " part).
Anything I didn't think off? This seems to work so far.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | tabledrag_after_draggable-329797.js_.patch | 894 bytes | jsagotsky |
| #24 | draggable-329797-4.patch | 2.75 KB | jsagotsky |
| #21 | draggable-329797-3.patch | 2.75 KB | danielb |
| #15 | draggable-329797-2.patch | 2.74 KB | danielb |
| #12 | dragtest.zip | 1.86 KB | danielb |
Comments
Comment #1
berenddeboer CreditAttribution: berenddeboer commentedBTW, the line number where the error occurs is 130.
Comment #2
berenddeboer CreditAttribution: berenddeboer commentedI've attached a patch that even allows for nested dragged tables. Obviously this needs some intricate support on the server side as well.
Comment #3
chx CreditAttribution: chx commentedLet's fix first in D7 and then backport.
Comment #5
danielb CreditAttribution: danielb commentedThis patch works for me mate. I've worked for 3 weeks on a module with the assumption I could make nested drag tables - you can imagine the rude shock I copped when I tried various ways to nest tables.
Using your patch I am able to achieve what I need using the simplest technique - a FAPI array element within the 'value' FAPI array element of one dragtable row.
The problem now I that I cannot distribute my module until this bug is fixed in D6 core, or if I hack together a 2nd JS file that will install with my module - but then I will need to rewrite other tabledrag functions, probably.
Is there a chance this can be added to a D6 release, and how can I help?
Comment #6
danielb CreditAttribution: danielb commentedsorry chx, I type slow
So - the patch failed testing - what is wrong with it?
Comment #7
redndahead CreditAttribution: redndahead commentedPlease don't change the version. We first need to patch core and then we'll backport and apply if necessary. It also needs to pass testing before it can be moved to RTBC.
Comment #8
danielb CreditAttribution: danielb commentedNot my fault mate, looks like this forum has a concurrent update bug.
Comment #9
redndahead CreditAttribution: redndahead commentedI guess I typed slow too. :) It happens. It's not a bug it's a feature. :)
Attached patch for D7
Comment #10
redndahead CreditAttribution: redndahead commentedSon of a monkey I swear I switched that to CNR.
@danielb you can help by testing this works in d7. I haven't tested it I just took the code from before.
Comment #11
danielb CreditAttribution: danielb commentedThanks, I will write a nested drag table in Drupal 7.
Comment #12
danielb CreditAttribution: danielb commentedI can confirm that prior to applying the patch in Drupal 7 I was experiencing the same problem I had in Drupal 6. After applying the patch the behaviour was as originally expected.
For completeness I have attached the module I hacked together for this review, it's a bit rough as I had some trouble with theme_form_element, and may have misused drupal_render() to get around it, but that is just my inexperience with Drupal 7.
Comment #13
Dries CreditAttribution: Dries commentedWould be great if we could document this a bit better in the code. It wasn't immediately obvious... Quick re-roll with some code comments?
Comment #14
danielb CreditAttribution: danielb commentedSomething like "match immediate children of the parent element to allow nesting"? Or more detailed? Should it be put in all four blocks of changed code?
Comment #15
danielb CreditAttribution: danielb commentedPatch with added comments.
Comment #16
danielb CreditAttribution: danielb commentedComment #17
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks. Moving version to D6
Comment #18
moshe weitzman CreditAttribution: moshe weitzman commentedCan we use this anywhere in core? Where are folks thinking of using this? screenshot?
Comment #19
danielb CreditAttribution: danielb commentedI am not sure a need exists to do a nested table in core, but since people like me need it for contrib modules, and the original poster needed it for his website, it shouldn't be prevented by a design that assumes no nesting.
The concept of nesting and hierarchies pops up constantly in development of all sorts of things. My need is for a user to configure a list of things that will happen. Some of these things have a sublist of processes that have to be ordered too. One solution is to have a seperate page for each sublist - but when the sublist is not important enough to have its own page, a nested table seems like the most convenient option for an interface.
Do we need to create a new patch for D6 with the comments, or will it be right?
Comment #20
redndahead CreditAttribution: redndahead commentedA new patch will need to be created against D6
Comment #21
danielb CreditAttribution: danielb commentedThis is the D6 version of the patch.
Comment #22
Gábor HojtsyCommitted to Drupal 6, thanks.
Comment #24
jsagotsky CreditAttribution: jsagotsky commentedThe third version of the patch wasn't working for me in drush. The last bit of it had too high of an offset and was throwing an error even though it applied okay. Here's a new version with fixed line numbers.
Comment #25
jsagotsky CreditAttribution: jsagotsky commentedAnd just because draggable-329797-4.patch breaks tabledrag.js_8.patch, here's a version of tabledrag to apply after you've patched with draggable-329797-4.