Minor Changes for Mpdf, Dompdf #3002
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See discussion in PHPOffice#2999. Mpdf is not acknowledging the styling that we're using to hide table rows. I have opened an issue with them, but enclosing the cells in the hidden row inside a div with appropriate css does seem to be a workaround, and that can be incorporated into PhpSpreadsheet. It's kludgey, and it isn't even valid HTML, but ... Mpdf also doesn't like the addition of the ```file:///``` prefix when using local images from Windows (sample 21). Results are better when that prefix is not added. Dompdf seemed to have problems with sample 21 images on both Windows and Unix, with or without the file prefix. It does, however, support data urls for both, so is changed to embed images. It's still not perfect - the image seems truncated to the row height - but the results are better. I will continue to research, but may proceed as-is if I don't find anything better to do. Html Writer was producing a file with mixed line endings on Windows. This didn't cause any harm, but it seems a bit sloppy. It is changed to always use PHP_EOL as a line ending.
|
No inspiration about a workaround for dompdf truncating image height. I have opened an issue there. |
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Aug 19, 2022
For PR PHPOffice#3002, I noted that there was a problem with Dompdf truncating images. I raised an issue with them (dompdf/dompdf#2980), and they agree that there is a bug; however, they also suggested a workaround, namely omitting background-color from any cells which the image overlays. That did not at first appear to be a solution which could be generalized for PhpSpreasheet. However, investigating further, I saw that Html Writer is generating background-color for all cells, even though most of them use the default Fill type None (which suggests that background-color should not be specified after all). So this PR changes HTML Writer to generate background-color only when the user has actually set Fill type to something other than None. This is not a complete workaround for the Dompdf problem - we will still see truncation if the image overlays a cell which does specify a Fill type - however, it is almost certainly good enough for most use cases. In addition to that change, I made the generated Html a little smaller and the code a little more efficient by combining the TD and TH styles for each cell into a single declaration and calling createCssStyle only once.
7 tasks
oleibman
added a commit
that referenced
this pull request
Aug 25, 2022
* Html Writer - Do Not Generate background-color When Fill is None For PR #3002, I noted that there was a problem with Dompdf truncating images. I raised an issue with them (dompdf/dompdf#2980), and they agree that there is a bug; however, they also suggested a workaround, namely omitting background-color from any cells which the image overlays. That did not at first appear to be a solution which could be generalized for PhpSpreasheet. However, investigating further, I saw that Html Writer is generating background-color for all cells, even though most of them use the default Fill type None (which suggests that background-color should not be specified after all). So this PR changes HTML Writer to generate background-color only when the user has actually set Fill type to something other than None. This is not a complete workaround for the Dompdf problem - we will still see truncation if the image overlays a cell which does specify a Fill type - however, it is almost certainly good enough for most use cases. In addition to that change, I made the generated Html a little smaller and the code a little more efficient by combining the TD and TH styles for each cell into a single declaration and calling createCssStyle only once. * Revamp One Test Look for both td.style and th.style instead of just td.style in test.
MarkBaker
added a commit
that referenced
this pull request
Sep 25, 2022
### Added - Implementation of the new `TEXTBEFORE()`, `TEXTAFTER()` and `TEXTSPLIT()` Excel Functions - Implementation of the `ARRAYTOTEXT()` and `VALUETOTEXT()` Excel Functions - Support for [mitoteam/jpgraph](https://packagist.org/packages/mitoteam/jpgraph) implementation of JpGraph library to render charts added. - Charts: Add Gradients, Transparency, Hidden Axes, Rounded Corners, Trendlines, Date Axes. ### Changed - Allow variant behaviour when merging cells [Issue #3065](#3065) - Merge methods now allow an additional `$behaviour` argument. Permitted values are: - Worksheet::MERGE_CELL_CONTENT_EMPTY - Empty the content of the hidden cells (the default behaviour) - Worksheet::MERGE_CELL_CONTENT_HIDE - Keep the content of the hidden cells - Worksheet::MERGE_CELL_CONTENT_MERGE - Move the content of the hidden cells into the first cell ### Deprecated - Axis getLineProperty deprecated in favor of getLineColorProperty. - Moved majorGridlines and minorGridlines from Chart to Axis. Setting either in Chart constructor or through Chart methods, or getting either using Chart methods is deprecated. - Chart::EXCEL_COLOR_TYPE_* copied from Properties to ChartColor; use in Properties is deprecated. - ChartColor::EXCEL_COLOR_TYPE_ARGB deprecated in favor of EXCEL_COLOR_TYPE_RGB ("A" component was never allowed). - Misspelled Properties::LINE_STYLE_DASH_SQUERE_DOT deprecated in favor of LINE_STYLE_DASH_SQUARE_DOT. - Clone not permitted for Spreadsheet. Spreadsheet->copy() can be used instead. ### Removed - Nothing ### Fixed - Fix update to defined names when inserting/deleting rows/columns [Issue #3076](#3076) [PR #3077](#3077) - Fix DataValidation sqRef when inserting/deleting rows/columns [Issue #3056](#3056) [PR #3074](#3074) - Named ranges not usable as anchors in OFFSET function [Issue #3013](#3013) - Fully flatten an array [Issue #2955](#2955) [PR #2956](#2956) - cellExists() and getCell() methods should support UTF-8 named cells [Issue #2987](#2987) [PR #2988](#2988) - Spreadsheet copy fixed, clone disabled. [PR #2951](#2951) - Fix PDF problems with text rotation and paper size. [Issue #1747](#1747) [Issue #1713](#1713) [PR #2960](#2960) - Limited support for chart titles as formulas [Issue #2965](#2965) [Issue #749](#749) [PR #2971](#2971) - Add Gradients, Transparency, and Hidden Axes to Chart [Issue #2257](#2257) [Issue #2229](#2929) [Issue #2935](#2935) [PR #2950](#2950) - Chart Support for Rounded Corners and Trendlines [Issue #2968](#2968) [Issue #2815](#2815) [PR #2976](#2976) - Add setName Method for Chart [Issue #2991](#2991) [PR #3001](#3001) - Eliminate partial dependency on php-intl in StringHelper [Issue #2982](#2982) [PR #2994](#2994) - Minor changes for Pdf [Issue #2999](#2999) [PR #3002](#3002) [PR #3006](#3006) - Html/Pdf Do net set background color for cells using (default) nofill [PR #3016](#3016) - Add support for Date Axis to Chart [Issue #2967](#2967) [PR #3018](#3018) - Reconcile Differences Between Css and Excel for Cell Alignment [PR #3048](#3048) - R1C1 Format Internationalization and Better Support for Relative Offsets [Issue #1704](#1704) [PR #3052](#3052) - Minor Fix for Percentage Formatting [Issue #1929](#1929) [PR #3053](#3053)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See discussion in #2999. Mpdf is not acknowledging the styling that we're using to hide table rows. I have opened an issue with them, but enclosing the cells in the hidden row inside a div with appropriate css does seem to be a workaround, and that can be incorporated into PhpSpreadsheet. It's kludgey, and it isn't even valid HTML, but ...
Mpdf also doesn't like the addition of the
file:///prefix when using local images from Windows (sample 21). Results are better when that prefix is not added.Dompdf seemed to have problems with sample 21 images on both Windows and Unix, with or without the file prefix. It does, however, support data urls for both, so is changed to embed images. It's still not perfect - the image seems truncated to the row height - but the results are better. I will continue to research, but may proceed as-is if I don't find anything better to do.
Html Writer was producing a file with mixed line endings on Windows. This didn't cause any harm, but it seems a bit sloppy. It is changed to always use PHP_EOL as a line ending.
This is:
Checklist:
Why this change is needed?
Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.