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 images to word export #22

Merged
merged 6 commits into from
Apr 12, 2024
Merged

Add images to word export #22

merged 6 commits into from
Apr 12, 2024

Conversation

aror92
Copy link
Contributor

@aror92 aror92 commented Apr 8, 2024

Create image part for a word doc, create images,
clone images between document fragments,
and track relationship IDs of images.

Export caption text. Formatting of image captions
will be handled separately.


This change is Reviewable

Create image part for a word doc, create images,
clone images between document fragments,
and track relationship IDs of images.

Export caption text. Formatting of image captions
will be handled separately.

Change-Id: I848b9653332204bef01e33a7906be19e4966e743
Change-Id: Iad07186035c3d9ec0513badcfb7c9d42c335eeae
@jasonleenaylor
Copy link
Contributor

Src/xWorks/ConfiguredLcmGenerator.cs line 865 at r1 (raw file):

			}
			return filePath;
			//return settings.UseRelativePaths ? filePath : new Uri(filePath).ToString();

Was this an experiment or for debugging?

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @aror92)


Src/xWorks/LcmWordGenerator.cs line 27 at r1 (raw file):

using A = DocumentFormat.OpenXml.Drawing;
using DW = DocumentFormat.OpenXml.Drawing.Wordprocessing;
using PIC = DocumentFormat.OpenXml.Drawing.Pictures;

More descriptive namespace aliases would be helpful for reading the code. WP for word processing below was tolerable,
using Drawing = DocumentFormat.OpenXml.Drawing;
using DrawingWP = DocumentFormat.OpenXml.Drawing.Wordprocessing;
using Pictures = DocumentFormat.OpenXml.Drawing.Pictures;

I've never looked into if you can use an alias in a following using which could be nice.
using Pictures = Drawing.Pictures

Change-Id: I5cfdfce20b5a3f4346429c4f77e19aedb97343db
Copy link
Contributor Author

@aror92 aror92 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor)


Src/xWorks/ConfiguredLcmGenerator.cs line 865 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Was this an experiment or for debugging?

I meant to start a conversation to ask about this here. Is there a reason we want to create a Uri in the case that we're not using relative paths? (e.g. if sometimes this is a link and not a filepath that would make sense, but I wasn't aware of any situation when it would be.) The word export is using the absolute filepath, but it's simpler to handle the filepath vs Uri.ToString, which adds some extra frontmatter.


Src/xWorks/LcmWordGenerator.cs line 27 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

More descriptive namespace aliases would be helpful for reading the code. WP for word processing below was tolerable,
using Drawing = DocumentFormat.OpenXml.Drawing;
using DrawingWP = DocumentFormat.OpenXml.Drawing.Wordprocessing;
using Pictures = DocumentFormat.OpenXml.Drawing.Pictures;

I've never looked into if you can use an alias in a following using which could be nice.
using Pictures = Drawing.Pictures

Done

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @aror92)


Src/xWorks/ConfiguredLcmGenerator.cs line 865 at r1 (raw file):

Previously, aror92 (Ariel Ror.) wrote…

I meant to start a conversation to ask about this here. Is there a reason we want to create a Uri in the case that we're not using relative paths? (e.g. if sometimes this is a link and not a filepath that would make sense, but I wasn't aware of any situation when it would be.) The word export is using the absolute filepath, but it's simpler to handle the filepath vs Uri.ToString, which adds some extra frontmatter.

I believe this is used currently by the preview windows because the xhtml is generated in the temp folder but we don't copy the images there for the preview. If the previews work well without the Uri wrapping then this change is acceptable.


Src/xWorks/LcmWordGenerator.cs line 1353 at r2 (raw file):

							{
								Id = (UInt32Value)0U,
								Name = name + ".jpg"

Is there a reason for using .jpg here?

Change-Id: Ia25fe2a6ee267ae0649fbd7ed9402639b84d2561
Copy link
Contributor Author

@aror92 aror92 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @jasonleenaylor)


Src/xWorks/ConfiguredLcmGenerator.cs line 865 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

I believe this is used currently by the preview windows because the xhtml is generated in the temp folder but we don't copy the images there for the preview. If the previews work well without the Uri wrapping then this change is acceptable.

The previews appear unchanged without the Uri wrapping.


Src/xWorks/LcmWordGenerator.cs line 1353 at r2 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Is there a reason for using .jpg here?

The image created in the word media file is a jpg (even when the file in FLEx is a png), but there isn't a particular need for the file extension to be in the name, so I've removed it.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aror92)


Src/xWorks/xWorks.csproj line 171 at r3 (raw file):

      <HintPath>..\..\Output\Debug\Newtonsoft.Json.dll</HintPath>
    </Reference>
    <Reference Include="PresentationCore" />

I'm curious about this, I wasn't sure where it was used.

Copy link
Contributor Author

@aror92 aror92 left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aror92)


Src/xWorks/xWorks.csproj line 171 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

I'm curious about this, I wasn't sure where it was used.

It's needed to access the System.Windows.Media.Imaging package, which is used to create the BitmapImage in the Word generator's CreateImage() method.

@aror92 aror92 merged commit b800cf5 into release/9.1 Apr 12, 2024
5 checks passed
@aror92 aror92 deleted the feature/LT-21708 branch April 12, 2024 19:31
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