-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
LT-21761: Add Table indenting and right justification #35
Conversation
Change-Id: I7f9835e66c6aaa5cb9039cc586683c1cb0311138
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.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mark-sil)
Src/xWorks/WordStylesGenerator.cs
line 966 at r1 (raw file):
/// <param name="tableAlignment">Returns the table alignment.</param> /// <returns>Returns the indentation value.</returns> internal static int GetTableIndentInfo(ReadOnlyPropertyTable propertyTable, ref TableRowAlignmentValues tableAlignment)
It might make more sense to place this function earlier in the code, above some of the helper functions. Perhaps right after "CalculateParagraphIndentsFromAncestors"? But this location works too if it seems better to you.
Src/xWorks/WordStylesGenerator.cs
line 974 at r1 (raw file):
} var projectStyle = styleSheet.Styles[WordStylesGenerator.DictionaryNormal];
Do we know for sure that we'll always want to base this off DictionaryNormal? For most types of dictionary content, Configure Dictionary allows the user to change the default style used for that content, or even specify a custom one. Not sure how it works with tables, though.
Use the table parent as the basis. Change-Id: I9e49a9570c7c9d09696e1bb4c4ba22636e5cb1af
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.
Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @aror92)
Src/xWorks/WordStylesGenerator.cs
line 966 at r1 (raw file):
Previously, aror92 (Ariel Ror.) wrote…
It might make more sense to place this function earlier in the code, above some of the helper functions. Perhaps right after "CalculateParagraphIndentsFromAncestors"? But this location works too if it seems better to you.
I agree. Moved the function.
Src/xWorks/WordStylesGenerator.cs
line 974 at r1 (raw file):
Previously, aror92 (Ariel Ror.) wrote…
Do we know for sure that we'll always want to base this off DictionaryNormal? For most types of dictionary content, Configure Dictionary allows the user to change the default style used for that content, or even specify a custom one. Not sure how it works with tables, though.
No, I shouldn't have used DictionaryNormal. Now it is based on the style of the parent.
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.
Reviewed 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mark-sil)
Src/xWorks/WordStylesGenerator.cs
line 974 at r1 (raw file):
Previously, mark-sil (Mark Kidder) wrote…
No, I shouldn't have used DictionaryNormal. Now it is based on the style of the parent.
One more question: do table nodes ever have a style specified, or will config.Style just always be null for a table? If the first case, I'm thinking trying to use config.Style first and moving to config.Parent?.Style if the node doesn't have a style specified would be the way to go.
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aror92)
Src/xWorks/WordStylesGenerator.cs
line 974 at r1 (raw file):
Previously, aror92 (Ariel Ror.) wrote…
One more question: do table nodes ever have a style specified, or will config.Style just always be null for a table? If the first case, I'm thinking trying to use config.Style first and moving to config.Parent?.Style if the node doesn't have a style specified would be the way to go.
The table could have a style, which should apply to the content of the table. This code is intended to make the entire table line up with the indenting used for the entry, so it should always use the parent style for indenting/justification.
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.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @mark-sil)
Change-Id: I7f9835e66c6aaa5cb9039cc586683c1cb0311138
This change is