Add row detail functionality (issue #173)

[marks]

Addresses issue #173

@timwis - reverted commit that brought in lots of other crud. If you look at Files Changed it should show the net changes

[timwis]

@marks this looks great. The only suggestion I have is perhaps rowDetails makes sense to be a template?

If you haven't used underscore templates before, it's pretty easy: just require() an .html file at the top of the folder (the .html files go in the templates directory for this project). Browserify will compile them at build time and the thing you required will be a function that you can pass arguments to and it will return HTML.

ex:

var RowDetailsTemplate = require('../templates/row-details.html')
var templateMarkup = RowDetailsTemplate({foo: 'bar', baz: 'quz'})
$('#row-details').html(templateMarkup)

And the template can render variables or even execute JS logic (example). Seems like a good fit for the rowDetails function. What do you think?

[marks]

Definitely-- will do. Thanks!

Mark Silverberg
(m) 512 826 7004
http://twitter.com/skram

On Apr 27, 2016, at 7:07 AM, Tim Wisniewski [email protected] wrote:

@marks this looks great. The only suggestion I have is perhaps rowDetails makes sense to be a template?

If you haven't used underscore templates before, it's pretty easy: just require() an .html file at the top of the folder (the .html files go in the templates directory for this project). Browserify will compile them at build time and the thing you required will be a function that you can pass arguments to and it will return HTML.

ex:

var RowDetailsTemplate = require('../templates/row-details.html')
var templateMarkup = RowDetailsTemplate({foo: 'bar', baz: 'quz'})
$('#row-details').html(templateMarkup)
And the template can render variables or even execute JS logic (example). Seems like a good fit for the rowDetails function. What do you think?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[marks]

@timwis as always, good suggestion to keep the code clean. I made the change and think this is ready to be merged. Agree?