-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding tests and revisions to PR #560 : Provide programmatic column header for table, stock example updated #668
Conversation
Still allows original example to function as-is. Allows non-hard-coded table column headers.
Supplied 3 examples of setting the columns; the original method, which requires the fixed html code for thead/cols; another which uses mostly the data element names which become capitalized as col headings, and another where most of the columns have the column label specified along with an accessor function. There is some code to strip a numberFormat/etc from a function converted to string, an attempt to auto-generate a column header from a function, in data-table.js, columnString().
Restored index.html, no need for a change. Added 4th example of columns.
so both header and value can be overridden
Changed to use just an in-line Object, and allow any Object specification desired, with note to override the internal functions to match. I think this is the cleanest and most flexible version.
…r columns and add jasmine tests
Hey @axiomabsolute, didn't know this was you! Thanks for contributing! If I don't manage to merge this soon, it is just because I am offline.. Just branch off this branch and continue working. Trust me, it'll work better than separate feature branches, since I know you have other plans for the same few files. |
(I'm mostly offline in the woods but can't quite stay away from dc!) |
Devon, thanks for finishing this. On August 8, 2014 9:49:50 AM CST, Devon Terrell notifications@github.com wrote:
Sent from my Android device with K-9 Mail. Please excuse my brevity. |
Thanks @mr23 and @axionabsolute! This is a useful feature. Our linters were complaining about the unused functions in the stock example, so I decided it was best just to keep the most general example, which also meant I could remove the header from index.html. Apparently firefox doesn't support |
Oh. I'm going to have to fix those _doCustom methods. That's not how we do customization points around here! Still, I think the functionality is solid, so it shouldn't be a big change. The code isn't entirely consistent, but the majority of the code has a standard of having _functions be exclusively internal customization points – where base classes/mixins communicate with derived classes. Public methods should not be underscored. Anything that is customizable should have a public getter/setter. |
Modified Object-based column definition to use objects of the form {label: "..", format: function(d){/**/}}
Added Jasmine tests to cover: