This project is archived and is in readonly mode.

#323 ✓resolved
Tristan

HtmlTable.Sort doesn't work with multiple rows in <thead>

Reported by Tristan | July 9th, 2010 @ 12:41 PM | in 1.3.1.1

It seems that HtmlTable.Sort is unable to cope with multiple rows in the part of a table. I have made a MooShell example to illustrate the problem.

http://mootools.net/shell/CVtYU/

You will see that when you try to sort one of the subheaders that Firebug will tell you this error:

Index or size is negative or greater than the allowed amount" code: "1
var head = document.id(this.head.cells[index]);
mootoo...more.js (line 6967)

Comments and changes to this ticket

  • tmarshall

    tmarshall August 17th, 2010 @ 07:08 PM

    This is caused by the HtmlTable class, which sets this.head = document.id(this.thead.rows[0]);

    So, currently if you have 2+ rows in the theader, the first row will work. Others find a -1 index since their index are not found in this.thead. I would change this.head to be all ths, but I'm not sure how that will affect other classes that use Class.refactor on HtmlTable.

  • Ryan Florence

    Ryan Florence August 24th, 2010 @ 05:08 AM

    Consider this: When I click "Head 2" what is supposed to sort? The first column (1,5,3) or the second (Green, Yellow, Blue)? I'd need a pretty convincing argument that this is in fact within the scope of HTMLTable. You can create whatever wild table header you want and it'd be impossible to guess what is the intended behavior.

    However, maybe there's a valid argument to be able to pass as an option a collection of elements whose indexes match the rows you want to sort.

  • Ryan Florence

    Ryan Florence August 26th, 2010 @ 12:08 AM

    My how quickly my opinion changes.

    I just needed to have multiple th's in a sortable table. I've implemented what I think is a good solution. Just need to get testrunner working on my machine so I can create the tests and then push the changes to my fork.

    Essentially just created a thSelector option to tell the class which th's ought to be clickable that defaults to th. So if you wanted you could do something like

    thSelector: 'thead > tr:first-child > th'

    Another option (I just thought of now) would be to pass in either a selector (like I just explained) or a collection of elements. Those elements have to be in the same order as the columns. Then, put in a check somewhere of the argument type, if it's a collection of Elements, attach the events to the elements (though not sure how delegation fits in here, I'm going to chew on that for a bit.)

  • Arian

    Arian September 20th, 2010 @ 05:02 PM

    • Milestone set to 1.3.0.1 rc1
    • State changed from “new” to “open”
    • Assigned user set to “Aaron Newton”
    • Milestone order changed from “197499” to “0”
  • Tim Wienk

    Tim Wienk September 26th, 2010 @ 10:24 PM

    • Milestone changed from 1.3.0.1 rc1 to 1.3.1.1
    • Milestone order changed from “5” to “0”

    Not sure about this one. Example doesn't work anymore either. Would be nice to see a new example on jsFiddle.net!

    Ryan's solution sounds rather clean. Will leave this up to Aaron for after the first 1.3-release.

  • Aaron Newton

    Aaron Newton September 26th, 2010 @ 11:22 PM

    Ryan, I believe you have the test runner working now. Can you send a pull? We'll aim for 1.3.0.2 as stated.

  • JacobThornton

    JacobThornton December 3rd, 2010 @ 08:50 PM

    • Milestone order changed from “4” to “0”
  • Arian

    Arian December 23rd, 2010 @ 03:35 PM

    • Assigned user changed from “Aaron Newton” to “JacobThornton”
  • Arian

    Arian December 23rd, 2010 @ 09:42 PM

    • State changed from “open” to “resolved”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

The MooTools Extensions

Pages