Added support for mergeCells, cell height, shrink to fit #738
Conversation
Added support for
mergeCells:
// mergeCells (B2:G2), you may use CellHelper::getColumnLettersFromColumnIndex() to convert from "B2" to "[1,2]"
$writer->mergeCells([1,2], [6, 2]);
cell height:
$row->setHeight(30);
shouldShrinkToFit:
$style->setShouldShrinkToFit();
These changes are implemented for XLSX as that's what I need and test spout on.
|
Should close #529 (comment) & #729 |
|
Seems that one test is failing, any ideea why? I'm not familiar with phpunit or travis |
|
|
|
@jmsche it seems that I'll wait, as my spare time this month is pretty much zero :) |
How to write the code, I also need to merge the cells |
|
Hey guys and thanks for your work @quamis! |
You could provide the missing Unit Tests! |
shrinkToFit was not handled in StyleMerger so it was overwritten by the default cell style StyleManager didn't add the property to the xml if shrinkToFit was used without alignment or text wrap. Unit test
…efact Tests Added 'addOption' to OptionsManagerInterface Moved 'setColumnWidths' and 'mergeCells' methods to Xlsx Writer implementation since the actual feature only works for Xlsx at the moment
|
@jmsche any ideea why "continuous-integration/travis-ci Expected — Waiting for status to be reported " hasn't finished yet? I made the commit 2 weeks ago @ignaczistvan I do not understand. Did you made the unitTests (I'm not seeing anything, but again, I'm not familiar with github's way of mergeing stuff)? Or you would prefer helping with the docs? |
@quamis Yep, I've made the tests and filed a pull request to your fork. You can check my commits here: quamis#1 I'm also open to helping with the docs if it's needed to release a new version. |
Test + minor fixes/refacts
|
@madflow Any update on this one? |
|
@ignaczistvan Not sure what you mean... Are you referring to my hint above, that unit tests where missing? |
Well, I’ve added tests for the new features so basically yes. Is anything missing, something I missed on the tests, docs, etc? |
You asked what you can do to bring this PR forward. After quick glance I noticed the missing unit tests. From my experience: In order to raise chances that a maintainer will consider this PR "mergable" it should
From what I gather - the chances that a maintainer will have a look and give feedback are slim at the moment. |
|
This will add the merged cells on all sheets, not just the current active sheet. |
|
Merged in openspout/openspout#21 |
Added support for
mergeCells:
// mergeCells (B2:G2), you may use CellHelper::getColumnLettersFromColumnIndex() to convert from "B2" to "[1,2]"
$writer->mergeCells([1,2], [6, 2]);
These changes are implemented for XLSX as that's what I need and test spout on.