-
Notifications
You must be signed in to change notification settings - Fork 75
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
Conversation
There was a problem hiding this 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.
sprintf( | ||
'<Relationship Id="rId%s" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/hyperlink" Target="%s" TargetMode="External"/>', | ||
linkRelNo, | ||
address |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
Thanks! |
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.