Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

Default style with no text wrap is ignored #829

Open
fwwarr opened this issue Sep 15, 2021 · 1 comment
Open

Default style with no text wrap is ignored #829

fwwarr opened this issue Sep 15, 2021 · 1 comment

Comments

@fwwarr
Copy link

fwwarr commented Sep 15, 2021

Automatic text wrapping due to the presence of new lines in a cell should only be applied if shouldWrapText has not been set on the default style. Indeed the StyleManager does check !$cellStyle->hasSetWrapText() before applying automatic text wrapping, but the following sample shows the opposite behavior:

$defaultStyle = (new StyleBuilder())->setShouldWrapText(false)->build();
$writer = WriterEntityFactory::createXLSXWriter();
$writer->setDefaultRowStyle($defaultStyle)->openToFile('testNoWrap.xlsx');
$writer->addRow(WriterEntityFactory::createRowFromArray(["test\n"]));
$writer->close();

This seems to be because StyleMerger::mergeCellProperties only calls setShouldWrapText on the style to update if the $baseStyle has shouldWrapText true, whereas it probably should call it whenever hasSetWrapText is true.

Current code:

        if (!$style->hasSetWrapText() && $baseStyle->shouldWrapText()) {
            $styleToUpdate->setShouldWrapText();
        }

Possible fix:

        if (!$style->hasSetWrapText() && $baseStyle->hasSetWrapText()) {
            $styleToUpdate->setShouldWrapText($baseStyle->shouldWrapText());
        }
jonnott added a commit to jonnott/spout that referenced this issue Feb 10, 2022
…icitly set as false

- add Common\Entity\Style\CellVerticalAlignment
- duplicate get/set/hasSet/shouldApply methods for CellAlignment in Common\Entity\Style\Style for CellVerticalAlignment instead, plus corresponding properties
- add setCellVerticalAlignment method to Common\Creator\Style\StyleBuilder
- add vertical alignment to StyleMerger:: mergeCellProperties()
- adjust wrapText logic in mergeCellProperties() to fix issue box#829
- apply vertical cell styling for both XLSX and ODS, via corresponding StyleManager classes
- transform vertical alignment  ‘center’ to ‘middle’ for ODS
- fix logic around wrapText such that the choice whether to include wrapping styles depends on hasSetWrapText() being true, and then use shouldWrapText() thereafter to either set wrapping or no wrapping (for XLSX, wrapText=“1” or wrapText=“0”, for ODS, wrap-option=wrap or wrap-option=no-wrap). previously there was no way to set wrapping to be OFF, only to set it to be ON.
- add new tests to StyleBuilderTest for vertical alignment
- add vertical alignment to documentation.md
jonnott added a commit to jonnott/spout that referenced this issue Feb 11, 2022
…icitly set as false

- add Common\Entity\Style\CellVerticalAlignment
- duplicate get/set/hasSet/shouldApply methods for CellAlignment in Common\Entity\Style\Style for CellVerticalAlignment instead, plus corresponding properties
- add setCellVerticalAlignment method to Common\Creator\Style\StyleBuilder
- add vertical alignment to StyleMerger:: mergeCellProperties()
- adjust wrapText logic in mergeCellProperties() to fix issue box#829
- apply vertical cell styling for both XLSX and ODS, via corresponding StyleManager classes
- transform vertical alignment  ‘center’ to ‘middle’ for ODS
- fix logic around wrapText such that the choice whether to include wrapping styles depends on hasSetWrapText() being true, and then use shouldWrapText() thereafter to either set wrapping or no wrapping (for XLSX, wrapText=“1” or wrapText=“0”, for ODS, wrap-option=wrap or wrap-option=no-wrap). previously there was no way to set wrapping to be OFF, only to set it to be ON.
- add new tests to ensure shouldWrapText(false) results in the correct negated wrapText (XLSX) / wrap-option (ODS) styles
- add new tests to StyleBuilderTest for vertical alignment
- add vertical alignment to documentation.md
jonnott added a commit to jonnott/spout that referenced this issue Feb 12, 2022
…icitly set as false

- add Common\Entity\Style\CellVerticalAlignment
- duplicate get/set/hasSet/shouldApply methods for CellAlignment in Common\Entity\Style\Style for CellVerticalAlignment instead, plus corresponding properties
- add setCellVerticalAlignment method to Common\Creator\Style\StyleBuilder
- add vertical alignment to StyleMerger:: mergeCellProperties()
- adjust wrapText logic in mergeCellProperties() to fix issue box#829
- apply vertical cell styling for both XLSX and ODS, via corresponding StyleManager classes
- transform vertical alignment  ‘center’ to ‘middle’ for ODS
- fix logic around wrapText such that the choice whether to include wrapping styles depends on hasSetWrapText() being true, and then use shouldWrapText() thereafter to either set wrapping or no wrapping (for XLSX, wrapText=“1” or wrapText=“0”, for ODS, wrap-option=wrap or wrap-option=no-wrap). previously there was no way to set wrapping to be OFF, only to set it to be ON.
- add new tests to ensure shouldWrapText(false) results in the correct negated wrapText (XLSX) / wrap-option (ODS) styles
- add new tests to StyleBuilderTest for vertical alignment
- add vertical alignment to documentation.md
Slamdunk pushed a commit to Slamdunk/openspout that referenced this issue Mar 28, 2022
…icitly set as false

- add Common\Entity\Style\CellVerticalAlignment
- duplicate get/set/hasSet/shouldApply methods for CellAlignment in Common\Entity\Style\Style for CellVerticalAlignment instead, plus corresponding properties
- add setCellVerticalAlignment method to Common\Creator\Style\StyleBuilder
- add vertical alignment to StyleMerger:: mergeCellProperties()
- adjust wrapText logic in mergeCellProperties() to fix issue box/spout#829
- apply vertical cell styling for both XLSX and ODS, via corresponding StyleManager classes
- transform vertical alignment  ‘center’ to ‘middle’ for ODS
- fix logic around wrapText such that the choice whether to include wrapping styles depends on hasSetWrapText() being true, and then use shouldWrapText() thereafter to either set wrapping or no wrapping (for XLSX, wrapText=“1” or wrapText=“0”, for ODS, wrap-option=wrap or wrap-option=no-wrap). previously there was no way to set wrapping to be OFF, only to set it to be ON.
- add new tests to ensure shouldWrapText(false) results in the correct negated wrapText (XLSX) / wrap-option (ODS) styles
- add new tests to StyleBuilderTest for vertical alignment
- add vertical alignment to documentation.md
@Slamdunk
Copy link

Fixed in openspout/openspout#63

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants