Allow "Move but don't size with cells" to be set on images. #2237
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
By default, images added to spreadsheets are using oneCellAnchor. However, it is common to want an image in each row of a spreadsheet, and to need those images to be deleted when the row is deleted in Excel. By calling setAnchorType(Drawing::ANCHORTYPE_TWOCELL) and using the new setCoordinate2() method, developers can specify which row(s) contain the image, and when they are deleted the image is too.
|
Hi! @AdamGaskins |
PowerKiKi
requested changes
Oct 31, 2021
There was a problem hiding this comment.
This looks like a nice improvement. There is a few things to improve though.
- the PR would need to be rebased on master
- unit tests are missing
- all new code must be type-hinted (not phpdoc-ed)
- parameters like
$pValueshould not be used anymore, instead use proper names, such as$anchorTypeand so on
|
Hello @AdamGaskins, are you still available to help on this feature? It would be a nice improvement. |
|
Sure I can make the requested changes 👌🏻 |
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Mar 1, 2022
This change was introduced by @naotake51 as PR PHPOffice#2532, but it was closed before I had a chance to review it, and I have been unable to communicate with the author since. I will gladly defer to the original. Please see the original PR for details. Similarly, PR PHPOffice#2237 (@AdamGaskins) is open, but with changes requested for several months. It covers a lot of the same ground as 2532. It makes sense to me to combine them. To those changes I added support for the 'editAs' attribute - I imagine that there are instances of 'absolute' out in the wild. There have been several other tickets referencing two cell anchors, including issue PHPOffice#1159 and PR PHPOffice#1160 (@sgarwood, who also added support for editAs), PR PHPOffice#643, and issue PHPOffice#126, all now closed but not necessarily entirely resolved. I will try to ensure that those tickets are addressed with this one. And, in trying to make sure 1160 is covered, I stumbled upon a bug. If you use the same image resource to create two+ memory drawings, the MemoryDrawing destructor for the first will cause the rest to generate a very long warning message. This is not a problem for Php8+, only for Php7-. I have suppressed the message in the MemoryDrawing constructor. 1160 went stale due to an unresolved test error, but I don't think this was the problem. At any rate, its test works now.
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Mar 13, 2022
This change builds on PR PHPOffice#2532 (@naotake51 as PR PHPOffice#2532), using ideas from PR PHPOffice#2237 (@AdamGaskins), which has had changes requested for several months. It covers a lot of the same ground as 2532. In Excel, two-cell anchor drawings can be edited as "twocell", "onecell", or "absolute". This PR adds support for those options, with a sample file that demonstrates the difference in addition to unit tests. Several other tests are added to improve the spotty coverage for Drawings. There have been several other tickets referencing two cell anchors, including issue PHPOffice#1159 and PR PHPOffice#1160 (@sgarwood, who also added support for editAs), PR PHPOffice#643, and issue PHPOffice#126, all now closed but not necessarily entirely resolved. I will try to ensure that those tickets are addressed with this one. And, in trying to make sure 1160 is covered, I stumbled upon a bug. If you use the same image resource to create two+ memory drawings, the MemoryDrawing destructor for the first will cause the rest to generate a very long warning message. This is not a problem for Php8+, only for Php7-. I have suppressed the message in the MemoryDrawing constructor. 1160 went stale due to an unresolved test error, but I don't think this was the problem. At any rate, its test works now.
5 tasks
oleibman
added a commit
that referenced
this pull request
Mar 16, 2022
* Add editAs Property for 2-cell Anchor Drawings This change builds on PR #2532 (@naotake51 as PR #2532), using ideas from PR #2237 (@AdamGaskins), which has had changes requested for several months. It covers a lot of the same ground as 2532. In Excel, two-cell anchor drawings can be edited as "twocell", "onecell", or "absolute". This PR adds support for those options, with a sample file that demonstrates the difference in addition to unit tests. Several other tests are added to improve the spotty coverage for Drawings. There have been several other tickets referencing two cell anchors, including issue #1159 and PR #1160 (@sgarwood, who also added support for editAs), PR #643, and issue #126, all now closed but not necessarily entirely resolved. I will try to ensure that those tickets are addressed with this one. And, in trying to make sure 1160 is covered, I stumbled upon a bug. If you use the same image resource to create two+ memory drawings, the MemoryDrawing destructor for the first will cause the rest to generate a very long warning message. This is not a problem for Php8+, only for Php7-. I have suppressed the message in the MemoryDrawing constructor. 1160 went stale due to an unresolved test error, but I don't think this was the problem. At any rate, its test works now. * Scrutinizer It reported 1 minor issue (fixed normally), and 2 major. One is fixed with a kludge. The other is a case where Scrutinizer's analysis is just wrong, and I can't figure out a kludge. But I was able to add an annotation (the first time I've managed to get one past phpcs/php-cs-fixer). We'll see.
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.
This is:
Checklist:
Why this change is needed?
See #1159 (and the abandoned PR #1160)
By default when adding a Drawing to a spreadsheet, it is added inside a
oneCellAnchor. This makes it easier to specify a width and height, however in some cases developers may want to change the default value of the option pictured below in Excel. This PR allows developers to add images into spreadsheets with the setting "Move and size with cells."Example use case: imagine a spreadsheet of product data where each row has a column called "Images." When a product row is deleted, it would make sense for the photo to be deleted as well. The only way for this to happen is to have "Move and size with cells" set.
All existing code will work the same, as
oneCellAnchorremains the default. Seesamples/Basic/47_Image_move_and_size_with_Cells.phpfor an example of the new process usingtwoCellAnchor.DrawingtotwoCellAnchormode.setCoordinates2(),setOffsetX2()andsetOffsetY2()methods.setWidth()andsetHeight()only work withoneCellAnchormode)One other note: I also added
getImageWidth()andgetImageHeight()functions to drawing that return the original pixel width/height of the image.getWidth()andgetHeight()contain this value when the image is initially loaded into theDrawing, but if you resize it in the worksheet this info is lost. Having these isn't necessary, but may be helpful due to the increased complexity of inserting images intwoCellAnchormode.