Skip to content

Conversation

@marks
Copy link
Collaborator

@marks marks commented Apr 19, 2016

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

Mark Silverberg and others added 30 commits January 9, 2016 15:17
is quite a hack.. need to understand where to put the code added to
socrata-fields.js
pure copy and paste job, refactoring should come later
timwis#151 show % of filter in balloon of bar chart
timwis#151 fix bug I introduced when working on this feature
Bring vizwit/marks:master into marks:dev
@timwis
Copy link
Owner

timwis commented Apr 27, 2016

@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
Copy link
Collaborator Author

marks commented Apr 27, 2016

Definitely-- will do. Thanks!

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

On Apr 27, 2016, at 7:07 AM, Tim Wisniewski notifications@github.com 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
Copy link
Collaborator Author

marks commented Apr 29, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants