Add ability to choose the table class on html output and to put a counter on it. #5

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@michelboaventura

I found this two features very useful if you work with big tables, as a left counter helps it a lot! Also with the custom class table one can easily use bootstrap's goodies (class='table').

It would be better implement the counter on a more lower level, so every outputter would benefit for it, but for now I think it works pretty well.

@nathanl

I'm not sure about this one. Some thoughts:

1) I'm not sure if these specific view options will be needed by lots of people, and I worry that this might be a step towards feature bloat for the view.
2) Unlimited view customization can already be done by creating app/views/dossier/reports/show in your own app.

Furthermore, if we do add this, the implementation would need to change. The view should not be dependent on request parameters, but should get any info it needs from the report itself. This is for two reasons.

1) Although Dossier's built-in report controller will hand params[:options] to the report, you might use a report with a different controller (see README) and pass different parameters to your report, then try to render this view. params[:options] might not exist, or worse, might have a different meaning to that controller; maybe they are data export options or something.

2) Getting info from the report itself lets you use methods that can provide defaults. Where you do things like if params[:options] && params[:options][:count] and if params[:options].try(:[],:count), you could just do if report.count. You would have defined def count on your report to use options.fetch(:count) { false } or something, so that you always have default value.

I do like the general idea of being able to easily set the generated table's class without having to write your own view, and it might make sense to allow setting an id, also. Maybe just a table_options hash would be best.

Would you mind holding off on this for a bit until I can chat with Adam?

@michelboaventura

Hi Nathan, this is more like a proof of concept of things we could do to improve the reports. I've put the counter because every single table report I made, the first thing users complain is the lack of a counter. Also, the custom table class is a good thing if we want to support twitter-bootstrap.

@nathanl

Yeah, I have thought more about it and I think those are both sensible things to have in there, but I still want to run it by Adam. Thanks for your work and your patience.

@adamhunter
Tax Management Associates member

Hey @michelboaventura,

Nathan and I have been discussing your pull request and we would like to pull you in to talk with us on it. I am leaning in the direction that a template generator will solve these issues in an easier manner for future implementers, but would like to get your take on it. Are you available to skype sometime?

@adamhunter
Tax Management Associates member

hey all, i've been refactoring this sort of functionality into dossier plugins. If you don't want to provide a custom view, I'll be releasing a dossier-formatter gem that will provide a dsl that will provide options for adding classes and ids and the like in the automatic view.

@adamhunter adamhunter closed this Dec 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment