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.
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
Inserting hyperlinked image #499
Changes from all commits
bfde11f
9dc6fec
e7847c6
1c71541
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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. AfaikloadWorkbook()
converts all hyperlink relationships, creates a new index for them and appends anh
at the end. So something likerId2
can becomerId1h
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 nativeopenxlsx
hyperlink object. But I'm not sure howopenxlsx
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.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
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
andopenxlsx2
. I'll have to make the time to look into switching toopenxlsx2
in the future.Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.