Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Html.php #3535

Merged
merged 10 commits into from
May 12, 2023
Merged

Update Html.php #3535

merged 10 commits into from
May 12, 2023

Conversation

dubox
Copy link
Contributor

@dubox dubox commented Apr 23, 2023

Add position attr for image whitch driving by MemoryDrawing

This is:

  • a bugfix
  • a new feature
  • refactoring
  • additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Why this change is needed?

#3529

Add position attr for image whitch driving by MemoryDrawing
@oleibman
Copy link
Collaborator

oleibman commented May 9, 2023

I have updated your description to point to issue 3529 rather than 3530. If you feel it should be 3530, please explain why.

You need to include a unit test to demonstrate that your code is working as desired. It might also be helpful to include a sample so that we can see the practical effect of your change on the output file.

@dubox
Copy link
Contributor Author

dubox commented May 11, 2023

I have updated your description to point to issue 3529 rather than 3530. If you feel it should be 3530, please explain why.

You need to include a unit test to demonstrate that your code is working as desired. It might also be helpful to include a sample so that we can see the practical effect of your change on the output file.

I have added a unittest at tests/PhpSpreadsheetTests/Writer/Html/MemoryDrawingOffsetTest.php

@oleibman
Copy link
Collaborator

Thank you for supplying the test. I have a few minor changes that I would like you to make to it, but won't be able to document those until tonight.

@oleibman oleibman merged commit 40203ff into PHPOffice:master May 12, 2023
@oleibman
Copy link
Collaborator

Thank you for your contribution.

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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants