Skip to content

initial workings for adding images to word outputs #1273

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

Merged
merged 17 commits into from
Aug 29, 2023

Conversation

thebioengineer
Copy link
Collaborator

@thebioengineer thebioengineer commented Mar 24, 2023

Summary

Thank you for contributing to gt! To make this process easier for everyone, please explain the context and purpose of your contribution. Also, list the changes made to the existing code or documentation.

Related GitHub Issues and PRs

Checklist

TODO:

  • make image insertion more generally work (right now markdown insertion is the only way)
  • make the styling not corrupt (image inserts, but currently the doc needs to be "Repaired", destroying styling)

@CLAassistant
Copy link

CLAassistant commented Aug 10, 2023

CLA assistant check
All committers have signed the CLA.

@thebioengineer thebioengineer marked this pull request as ready for review August 24, 2023 13:34
@thebioengineer
Copy link
Collaborator Author

@rich-iannone - curious to get your feedback. also, I wanted to comment that fmt_image only has height as an option, not width. That works for html, since it will just scale the image, but word requires both height and width. the code as-is works with square images added, but it will square out any other images.

@rich-iannone
Copy link
Member

@thebioengineer Sorry for the delay in responding here. I will try this out. About having just height as an option: we could also expose width but is possible to pre-calculate the latter given the height and having the image on disk? Unless inspecting an image requires extra packages for different graphics types, this might be worth exploring.

@thebioengineer
Copy link
Collaborator Author

We can definitely do that, but I'd need to sort out how to do that without Magick or other packages like that, since I was trying to not add a dependency

Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

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

LGTM!

@rich-iannone rich-iannone merged commit e61fad4 into rstudio:master Aug 29, 2023
@thebioengineer thebioengineer deleted the word_handling_imgs branch August 29, 2023 17:20
@rich-iannone rich-iannone mentioned this pull request Aug 29, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants