Html Writer Conditional Formatting Inline Css #4541
Merged
Conversation
This file contains hidden or 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
Fix PHPOffice#4539. Conditional Formatting was recently added to Html Writer. It works fine when not using inline Css. However, when using inline Css, the code inadvertently added 2 different `style` attributes (one for the unconditional style and one for the conditional style) to the same cell. This is not valid html, and results in losing the conditional styling. This PR combines the two `style` attributes into one, which will now come after the `class`, `colspan`, and `rowspan` attributes. Aside from the new tests, this PR changes an unusually large number of existing tests. While this might normally be considered a red flag, it is not a problem here. All of the changes involve merely changing the order of attributes within html tags; none of them affect how the generated html would appear in a browser.
|
Confirming this fixes my issue. Thank you for fixing so quickly. |
Xlsx conditional styles cannot be used to change alignment, nor font family, nor font size. Conditional::getStyle currently allocates a new style as an unconditional style; it is changed to do so a conditional ctyle. Border should not be conditionally merged if borderStyle is OMIT. Inline Css was generating `class="gridlines gridlinesp"` for all cells. It is changed to use the worksheet settings to decide which classes to use.
Using baseStyle directly can lead to the by-reference problems associated with Cell and Style.
Do not override base style if conditional style indicates BORDER_OMIT. Do not export BORDER_NONE color 0 for conditional; same approach is already taken for unconditional.
|
@ccchapman Thank you for confirming. As you can see, I have made additional changes to fix pre-existing problems that I've encountered while doing more extensive testing. I think I am done with those now. It is very unlikely that the new changes will adversely affect you, but please test again if you can. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fix #4539. Conditional Formatting was recently added to Html Writer. It works fine when not using inline Css. However, when using inline Css, the code inadvertently added 2 different
styleattributes (one for the unconditional style and one for the conditional style) to the same cell. This is not valid html, and results in losing the conditional styling. This PR combines the twostyleattributes into one, which will now come after theclass,colspan, androwspanattributes.Aside from the new tests, this PR changes an unusually large number of existing tests. While this might normally be considered a red flag, it is not a problem here. All of the changes involve merely changing the order of attributes within html tags; none of them affect how the generated html would appear in a browser.
This is:
Checklist: