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

Inserting hyperlinked image #499

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Inserting hyperlinked image #499

merged 4 commits into from
Sep 16, 2024

Conversation

jistria
Copy link

@jistria jistria commented Sep 10, 2024

I would like to be able to insert an image that opens a website in my browser when clicked.

This PR adds a new parameter to insertImage() that takes a URL, file path, or mailto string and uses it to add a drawing relationship, turning the image into a hyperlink. Tests are included.

I've also updated the DESCRIPTION and NEWS file with what I understood to be correct for the development version. Let me know if this was incorrect.

Thanks in advance for your time.

Copy link
Collaborator

@JanMarvin JanMarvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jistria , sorry, missed the PR. Looks good, just minor nitpicking to avoid some duplication.

R/WorkbookClass.R Show resolved Hide resolved
R/baseXML.R Outdated Show resolved Hide resolved
sprintf(
'<Relationship Id="rId%s" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/hyperlink" Target="%s" TargetMode="External"/>',
linkRelNo,
address
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I have been wasting the better half of my Sunday on working with hyperlinks in openxlsx2, I doubt that this will survive saving to a file and loading the file again. Afaik loadWorkbook() converts all hyperlink relationships, creates a new index for them and appends an h at the end. So something like rId2 can become rId1h after saving and loading. You could try to count the number of hyperlink objects in the worksheet and use this here or convert straight to a native openxlsx hyperlink object. But I'm not sure how openxlsx handles these native hyperlinks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit I hadn't considered the impact this change would have on saving and loading a file. I have looked into this now and while these hyperlink relationships do survive the process, there is a pre-existing issue that prevents users from inserting an image into a loaded workbook that already has multiple images.

loadWorkbook() reads the drawings xml into one string, regardless of whether there are multiple images. Same for the drawing relationships. But to assign new images an appropriate index (e.g. rId4), openxlsx relies on each image being a separate element in the drawings list. An example of the problematic xml is below.

# Create a workbook.
wb <- createWorkbook()
addWorksheet(wb, "Sheet 1")
# Insert multiple images.
img <- system.file("extdata", "einstein.jpg", package = "openxlsx")
insertImage(wb, "Sheet 1", img)
insertImage(wb, "Sheet 1", img, address = "https://github.com/ycphs/openxlsx")
insertImage(wb, "Sheet 1", img)
# Save workbook.
tf <- tempfile(fileext = ".xlsx")
saveWorkbook(wb, tf)
# Load workbook.
wb_2 <- loadWorkbook(tf)
# Inspect drawings and drawings_rels.
wb_2$drawings[[1]]
# [1] "<xdr:oneCellAnchor xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\"><xdr:from xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">    <xdr:col xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:col>    <xdr:colOff xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:colOff>    <xdr:row xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:row>    <xdr:rowOff xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:rowOff>  </xdr:from><xdr:ext xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\" cx=\"5486400\" cy=\"2743200\"/><xdr:pic xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">      <xdr:nvPicPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <xdr:cNvPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\" id=\"1\" name=\"Picture 1\"/>        <xdr:cNvPicPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">          <a:picLocks xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" noChangeAspect=\"1\"/>        </xdr:cNvPicPr>      </xdr:nvPicPr>      <xdr:blipFill xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <a:blip xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" xmlns:r=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships\" r:embed=\"rId1\">        </a:blip>        <a:stretch xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\">          <a:fillRect/>        </a:stretch>      </xdr:blipFill>      <xdr:spPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <a:prstGeom xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" prst=\"rect\">          <a:avLst xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\"/>        </a:prstGeom>      </xdr:spPr>    </xdr:pic><xdr:clientData/></xdr:oneCellAnchor><xdr:oneCellAnchor xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\"><xdr:from xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">    <xdr:col xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:col>    <xdr:colOff xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:colOff>    <xdr:row xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:row>    <xdr:rowOff xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:rowOff>  </xdr:from><xdr:ext xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\" cx=\"5486400\" cy=\"2743200\"/><xdr:pic xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">      <xdr:nvPicPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <xdr:cNvPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\" id=\"2\" name=\"Picture 2\">              <a:hlinkClick xmlns:r=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships\" r:id=\"rId3\"/>            </xdr:cNvPr>        <xdr:cNvPicPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">          <a:picLocks xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" noChangeAspect=\"1\"/>        </xdr:cNvPicPr>      </xdr:nvPicPr>      <xdr:blipFill xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <a:blip xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" xmlns:r=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships\" r:embed=\"rId2\">        </a:blip>        <a:stretch xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\">          <a:fillRect/>        </a:stretch>      </xdr:blipFill>      <xdr:spPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <a:prstGeom xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" prst=\"rect\">          <a:avLst xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\"/>        </a:prstGeom>      </xdr:spPr>    </xdr:pic><xdr:clientData/></xdr:oneCellAnchor><xdr:oneCellAnchor xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\"><xdr:from xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">    <xdr:col xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:col>    <xdr:colOff xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:colOff>    <xdr:row xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:row>    <xdr:rowOff xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">0</xdr:rowOff>  </xdr:from><xdr:ext xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\" cx=\"5486400\" cy=\"2743200\"/><xdr:pic xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">      <xdr:nvPicPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <xdr:cNvPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\" id=\"3\" name=\"Picture 3\"/>        <xdr:cNvPicPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">          <a:picLocks xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" noChangeAspect=\"1\"/>        </xdr:cNvPicPr>      </xdr:nvPicPr>      <xdr:blipFill xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <a:blip xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" xmlns:r=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships\" r:embed=\"rId4\">        </a:blip>        <a:stretch xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\">          <a:fillRect/>        </a:stretch>      </xdr:blipFill>      <xdr:spPr xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\">        <a:prstGeom xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" prst=\"rect\">          <a:avLst xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\"/>        </a:prstGeom>      </xdr:spPr>    </xdr:pic><xdr:clientData/></xdr:oneCellAnchor>"
wb_2$drawings_rels[[1]]
# [1] "<Relationship Id=\"rId1\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/image\" Target=\"../media/image1.jpg\"/><Relationship Id=\"rId2\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/image\" Target=\"../media/image2.jpg\"/><Relationship Id=\"rId3\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/hyperlink\" Target=\"https://github.com/ycphs/openxlsx\" TargetMode=\"External\"/><Relationship Id=\"rId4\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/image\" Target=\"../media/image3.jpg\"/>"

It looks like this issue was considered at some point (>9 years ago) because there is some code within loadWorkbook() that splits the xml of images into separate list elements, but it has never been uncommented.

openxlsx/R/loadWorkbook.R

Lines 761 to 765 in 06fa7eb

# ptn1 <- "<(mc:AlternateContent|xdr:oneCellAnchor|xdr:twoCellAnchor|xdr:absoluteAnchor)"
# ptn2 <- "</(mc:AlternateContent|xdr:oneCellAnchor|xdr:twoCellAnchor|xdr:absoluteAnchor)>"
## split at one/two cell Anchor
# dXML <- regmatches(dXML, gregexpr(paste0(ptn1, ".*?", ptn2), dXML))

Based on documentation (https://learn.microsoft.com/en-us/dotnet/api/documentformat.openxml.drawing.spreadsheet.worksheetdrawing?view=openxml-3.0.1) this code seems sensible, but I'm no expert. And I'm not familiar with what would create the mc:AlternateContent tag.

I could adapt this code so that it splits the drawings xml by image when it detects one of these expected tags, and does not split otherwise (i.e. resolve the issue in the expected/majority/possibly all situations, and be no worse off otherwise). I'd also need to write comparable code to handle the drawings_rels xml. Does this sound good to you? (sorry for the essay)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, indeed a bummer, I haven't thought about that. But I don't think it's worth going down that rabbit hole ... again. I have fixed many of these annoyances in openxlsx2, but some people still like to use something that is only partially working 🥲
Your PR has inspired me to JanMarvin/openxlsx2#1137, which should provide a clean solution for these shared hyperlinks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understandable! Thanks, @JanMarvin. I really appreciate the work you do in this space on openxlsx and openxlsx2. I'll have to make the time to look into switching to openxlsx2 in the future.

Copy link
Collaborator

@JanMarvin JanMarvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though there remains an issue with loadWorkbook(), it's fine.

@JanMarvin JanMarvin merged commit fae8a9d into ycphs:master Sep 16, 2024
8 checks passed
@JanMarvin
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants