Html Writer - Do Not Generate background-color When Fill is None #3016
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
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.
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)
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Jul 15, 2023
Changed a sample to illustrate how to add header/footer in Mpdf using setHtmlEditCallback. This uses custom Html tags, and, like body, it appears that these must be defined in the first writeHtml. Adjust writeMpdf to permit this by using a new constant `SIMULATED_BODY_START`, defined as an Html comment, as a delimiter. Sample 21c_Pdf, to which the header/footer code is added, had been introduced with PR PHPOffice#2434 to ensure that the body tag was always in the first chunk. However, PR PHPOffice#3016 accidentally invalidated that test by reducing the number of style lines so that the sample now included the body tag in its first 1000 records rather than afterwards. This change puts it past record 1000 again. Inspecting the results of all the Html/Pdf samples after this change, it turns out that sample 25_In_memory_image was accidentally broken by PR PHPOffice#3535 - the combination of `max-width:100%` (already present before that change) with `position:absolute` (introduced with that change) made the memory drawing disappear from the rendered html when the image occurs in a column after the last column with data in it. It appears that there is no need for max-width (drawings which are not memory drawings do not use it), so it is dropped. The sample is changed to add a second page with a memory drawing, one page with the memory drawing after the last data column, and one with it before. The Html results now reflect the Xlsx result, as they should.
11 tasks
oleibman
added a commit
that referenced
this pull request
Jul 24, 2023
* Minor Changes to Writer/Mpdf and Writer/Html Changed a sample to illustrate how to add header/footer in Mpdf using setHtmlEditCallback. This uses custom Html tags, and, like body, it appears that these must be defined in the first writeHtml. Adjust writeMpdf to permit this by using a new constant `SIMULATED_BODY_START`, defined as an Html comment, as a delimiter. Sample 21c_Pdf, to which the header/footer code is added, had been introduced with PR #2434 to ensure that the body tag was always in the first chunk. However, PR #3016 accidentally invalidated that test by reducing the number of style lines so that the sample now included the body tag in its first 1000 records rather than afterwards. This change puts it past record 1000 again. Inspecting the results of all the Html/Pdf samples after this change, it turns out that sample 25_In_memory_image was accidentally broken by PR #3535 - the combination of `max-width:100%` (already present before that change) with `position:absolute` (introduced with that change) made the memory drawing disappear from the rendered html when the image occurs in a column after the last column with data in it. It appears that there is no need for max-width (drawings which are not memory drawings do not use it), so it is dropped. The sample is changed to add a second page with a memory drawing, one page with the memory drawing after the last data column, and one with it before. The Html results now reflect the Xlsx result, as they should. * Minor Performance Improvements Anonymous function.
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Aug 19, 2023
Fix PHPOffice#3678. Problem introduced by PR PHPOffice#3016. Combining `td` and `th` styles into a single declaration greatly reduces file size when `useInlineCss` is false, which is the default. However, generating code with the non-default option was not changed to use the combined declaration, so styling was lost. This PR rectifies that error.
11 tasks
oleibman
added a commit
that referenced
this pull request
Aug 21, 2023
* Html Writer Styles when Using Inline Css Fix #3678. Problem introduced by PR #3016. Combining `td` and `th` styles into a single declaration greatly reduces file size when `useInlineCss` is false, which is the default. However, generating code with the non-default option was not changed to use the combined declaration, so styling was lost. This PR rectifies that error. * Apostrophe Rather Than Quote
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.
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.
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.