jennschiffer / timbles.js

a very simple jQuery plugin for tables, made by the person who literally did not invent tables
MIT License
38 stars 4 forks source link

Multi row headers #37

Closed edelooff closed 8 years ago

edelooff commented 8 years ago

This PR aims to resolve #5.

This multi-row header thing has come up a few times before, and you've mentioned it seems like prime plugin material for reasons of complexity and scope of timbles. However, I find it helps to have code present when having that conversation.

Code

With the recent changes (#36) of how headers are used, only setupExistingTables requires modification. Instead of selecting all <th> elements from the table header, it now runs through a more complete process to determine the cells to use as headers.

Since spanned cells create 'gaps' in the DOM, the first step is to create a full mapping of grid positions to cells. Once this grid exists, header selection is as simple as going column by column and picking its header cell.

This has two new behaviors:

  1. Cells that span two columns will never be selected for header purposes (because of ambiguity)
  2. If a column has multiple <th> cells for it, the lowest one is picked

N.B. Previously, colspanned cells would sort on click (the leftmost column they spanned), but cells to their right would sort the wrong column. Now, the colspanned cell will not sort on click, but cells to their right will sort the correct column. Both are obviously bad, but the new behavior is arguably slightly less bad.

Tests

Two test suites are added:

  1. A sorting suite that is the same as the basic suite but has an additional header row, to ensure that multi-headed tables pass all the same tests as single-headed tables;
  2. A suite specific for multi-row headers, which tests the header selection algorithm.

N.B. A small change was made to the sorting test suite. The selection of the first column header is performed using find( '#name' ) rather than the previous find( 'thead tr th' ).eq( 0 ). This is because the additional row causes the cell count to shift, but doesn't change the test itself.

edelooff commented 8 years ago

@jennschiffer will you have a chance to look at this in the coming week?

jennschiffer commented 8 years ago

i think this looks/works great. and yeah, i think this would be best as a plugin (timbles is meant to be a lightweight foundation for generating interactive table behaviors). i'm wondering how we would structure the json/object of a table that has multiple headers to generate a timble dom object from it.

as for adding the #name selector search, does this mean the user will need to have ids in their headers to point out where sort headers start?

edelooff commented 8 years ago

i think this looks/works great. and yeah, i think this would be best as a plugin (timbles is meant to be a lightweight foundation for generating interactive table behaviors). i'm wondering how we would structure the json/object of a table that has multiple headers to generate a timble dom object from it.

I think reworking this as a plugin is definitely possible, but requires a good bit of groundwork. Currently timbles is a pretty sealed plugin, but opening things up to achieve good plugin behavior sounds like a very good idea. Something to open a new issue for to discuss and think about?

As for creating a multi-row header from a data definition, I have no opinion on how to do that, or a good idea on how to define that. I deliberately left that out of scope here. The good thing I suppose is that the column data itself doesn't have to change, only the header definition requires additional complexity.

as for adding the #name selector search, does this mean the user will need to have ids in their headers to point out where sort headers start?

No, most definitely not. The selector change in the test suite is merely to support the same tests running on the multi-row and single-row header. This is because selecting the first <th> for a .click() would select an (inactive) spanned header for the multi-row header table.

Sorting can still be done programatically using .sortColumn() with either a string ID reference, or a column index. Clicking of course also still works the exact same way.

Multi-row headers add no requirements on the content author

edelooff commented 8 years ago

(err.. closing this was unintentional, though if you're not happy to merge this, feel free to re-close it)

edelooff commented 8 years ago

Right, with the current decision and work done for extensibility by plugins, this PR is no longer relevant.