Add support for merging cells and autofilter parameter #1

Merged
merged 10 commits into from Aug 28, 2013

2 participants

@jasonshaev

1) Added a new function merge(), example:
[% merge('A1:A2', "Merged content", "format" %]

2) Add support for an "autofilter" parameter in the JSON template which adds a filter dropdown to every column the given row (typically the specified row will be the headers).

@doy
Owner

The merge helper function needs docs (the docs for it should go in Spreadsheet::Template::Processor::Xslate, but you'll also need to add it to the Pod::Coverage ignore list at the bottom of Spreadsheet::Template::Helpers::Xslate. Also, adding some tests for this behavior would be helpful.

As for the autofilter behavior, I think it should probably be named autofilter_row or something along those lines if it's not going to expose the full functionality (I've never used it, but the docs seem to indicate that it can work on two-dimensional ranges, like A1:D12).

@jasonshaev

1) Added documentation for merge() helper function
2) Created test demonstrating merge(). The logic is to create 4 columns with unique contents, merge the columns while inserting different content and then prove that the content has changed.
3) Support for full autofilter() functionality

@doy
Owner

These changes all look good, just a couple more issues:

  • merge and autofilter need to be added to the data format documentation in Spreadsheet::Template. It'd be nice if there were also tests for autofilter, but I don't know if Spreadsheet::ParseXLSX handles that yet, so if not that's fine for now.
  • the merge helper should really take all of the options that the c helper can - there's no reason that you shouldn't be able to store a formula in a merged cell, for instance. You should be able to handle this by using the merge_range_type method instead of the merge_range method, but be sure to pass all of the necessary data along, so that formulas can have both a formula and a value (just like in normal cells).
@doy
Owner

One more thing - merge_range and merge_range_type allow using row and column indices as well as the A1:B2 notation, so there's no reason to not allow this here (since everywhere else in the data format uses row/column indices).

@jasonshaev

1) Added merge and autofilter to documentation in Spreadsheet::Template.
2) Support for formulae in merge helper function. Switched to using merge_range_type instead of merge_range
3) Switched to accepting row and column indices instead of A1:B2 notation in merge helper function

@doy doy added a commit that referenced this pull request Aug 28, 2013
@doy fix up the merge and autofilter stuff (#1) f79aa63
@doy doy merged commit f488bab into doy:master Aug 28, 2013

1 check failed

Details default The Travis CI build failed
@doy
Owner

Okay, I merged this with some modifications (so you'll probably have to change your code a bit). Let me know if everything works, and I'll make a release.

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