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

Add two cell anchor drawing #2532

Merged
merged 6 commits into from
Mar 10, 2022

Conversation

naotake51
Copy link
Contributor

@naotake51 naotake51 commented Jan 26, 2022

This is:

- [ ] a bugfix
- [x] a new feature

Checklist:

Why this change is needed?

PhpSpreadsheetを使用して、セルの中央に画像を配置したいです。
twoCellAnchorタグによる画像の配置ができると助かります。

I want to use PhpSpreadsheet to center the image in a cell.
It would be helpful if there was an image arrangement by twoCellAnchor tag.

sample code

// A position relative to a cell (top-left) and sized relative to another cell (bottom right)
$drawing = new Drawing();
$drawing->setName('Green Square');
$drawing->setPath('tests/data/Writer/XLSX/green_square.gif');
$drawing->setCoordinates('A1');
$drawing->setOffsetX(30);
$drawing->setOffsetY(10);
$drawing->setCoordinates2('E8');
$drawing->setOffsetX2(-50);
$drawing->setOffsetY2(-20);
$drawing->setWorksheet($sheet);

スクリーンショット 2022-01-27 1 25 07

output xl/drawings/drawing1.xml

<xdr:twoCellAnchor>
    <xdr:from>
        <xdr:col>0</xdr:col>
        <xdr:colOff>285750</xdr:colOff>
        <xdr:row>0</xdr:row>
        <xdr:rowOff>95250</xdr:rowOff>
    </xdr:from>
    <xdr:to>
        <xdr:col>4</xdr:col>
        <xdr:colOff>-476250</xdr:colOff>
        <xdr:row>7</xdr:row>
        <xdr:rowOff>-190500</xdr:rowOff>
    </xdr:to>
    <xdr:pic>
        ...
        <xdr:spPr>
            <a:xfrm>
                <a:off x="25400" y="812800" />
                <a:ext cx="4940300" cy="3765550" />
            </a:xfrm>
            <a:prstGeom prst="rect">
                <a:avLst />
            </a:prstGeom>
        </xdr:spPr>
    </xdr:pic>
    <xdr:clientData />
</xdr:twoCellAnchor>

また、twoCellAnchorタグのxlsxファイルを読み込み、そのまま書き出すとoneCellAnchorに変わってしまうため、画像の配置が変わってしまう問題があります。

Also, if you import an xlsx file with twoCellAnchor tag and export it as is, it will be changed to oneCellAnchor, which will change the placement of the image.

// read twoCellAnchor tag image.
$spreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load('./input.xlsx');

// written oneCellAnchor tag image.
$writer = \PhpOffice\PhpSpreadsheet\IOFactory::createWriter($spreadsheet, "Xlsx");
$writer->save("./output.xlsx");

input.xlsx
スクリーンショット 2022-01-27 14 03 32
output.xlsx
スクリーンショット 2022-01-27 14 03 37

@naotake51 naotake51 changed the title Add two cell anhor drawing. WIP Add two cell anhor drawing. Jan 26, 2022
@naotake51 naotake51 marked this pull request as draft January 26, 2022 15:32
@naotake51 naotake51 force-pushed the add-two-cell-anchor-drawing branch from f387125 to 10d5664 Compare January 26, 2022 16:17
@naotake51 naotake51 changed the title WIP Add two cell anhor drawing. Add two cell anhor drawing. Jan 26, 2022
@naotake51 naotake51 marked this pull request as ready for review January 26, 2022 16:38
@naotake51 naotake51 changed the title Add two cell anhor drawing. Add two cell anchor drawing. Jan 26, 2022
@naotake51 naotake51 changed the title Add two cell anchor drawing. Add two cell anchor drawing Jan 26, 2022
@naotake51 naotake51 closed this Feb 8, 2022
@naotake51 naotake51 deleted the add-two-cell-anchor-drawing branch February 8, 2022 06:33
@oleibman
Copy link
Collaborator

Not sure why you closed this. If it was because there had been no action on it, my apologies for the delay. I am ready to review it now if you wish to reopen it.

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 oleibman mentioned this pull request Mar 1, 2022
5 tasks
@naotake51 naotake51 restored the add-two-cell-anchor-drawing branch March 1, 2022 10:20
@naotake51 naotake51 reopened this Mar 1, 2022
@naotake51
Copy link
Contributor Author

naotake51 commented Mar 1, 2022

@oleibman

I'm sorry.
I didn't have any experience with OSS, so I thought it meant I was rejected.
I would appreciate it if you could take a look at it.

@naotake51 naotake51 force-pushed the add-two-cell-anchor-drawing branch from 1a76099 to fc32b06 Compare March 1, 2022 10:38
@naotake51 naotake51 marked this pull request as draft March 1, 2022 10:42
@naotake51 naotake51 changed the title Add two cell anchor drawing WIP Add two cell anchor drawing Mar 1, 2022
@naotake51 naotake51 force-pushed the add-two-cell-anchor-drawing branch from fc32b06 to 067017f Compare March 1, 2022 11:39
@naotake51 naotake51 force-pushed the add-two-cell-anchor-drawing branch from a2cc2d2 to f240d0a Compare March 1, 2022 13:08
@naotake51 naotake51 changed the title WIP Add two cell anchor drawing Add two cell anchor drawing Mar 1, 2022
@naotake51 naotake51 marked this pull request as ready for review March 1, 2022 13:12
@oleibman
Copy link
Collaborator

oleibman commented Mar 3, 2022

I probably won't be able to get to this for a few days. In the meantime, I would like to suggest an enhancement. In Xlsx, two-cell anchor drawings are associated with an editAs attribute (twocell or onecell or absolute). It would be useful if you could add that to the Drawing class, and to the Xlsx Reader and Writer.

@naotake51
Copy link
Contributor Author

naotake51 commented Mar 4, 2022

@oleibman

I understood it to mean adding absolute.

I will refer to Apache POI.
https://poi.apache.org/apidocs/dev/org/apache/poi/xssf/usermodel/XSSFClientAnchor.html

@oleibman oleibman merged commit 572f4e9 into PHPOffice:master Mar 10, 2022
@oleibman
Copy link
Collaborator

Thank you for your contribution.

@naotake51
Copy link
Contributor Author

naotake51 commented Mar 10, 2022

I probably won't be able to get to this for a few days. In the meantime, I would like to suggest an enhancement. In Xlsx, two-cell anchor drawings are associated with an editAs attribute (twocell or onecell or absolute). It would be useful if you could add that to the Drawing class, and to the Xlsx Reader and Writer.

Sorry, I couldn't take the time for task.

@naotake51 naotake51 deleted the add-two-cell-anchor-drawing branch March 10, 2022 03:38
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.
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
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants