Added keep-together paragraph style #45
Conversation
|
Hello @verheyenkoen , thank you very much for your contribution! When reading the code and comparing the output against a document created by LibreOffice I encountered two issues:
|
src/style/ParagraphProperties.ts
Outdated
| @@ -16,6 +17,7 @@ const DEFAULT_PAGE_BREAK = false; | |||
| export class ParagraphProperties implements IParagraphProperties { | |||
| private horizontalAlignment: HorizontalAlignment; | |||
| private shouldBreakPageBefore: boolean; | |||
| private shouldKeepParagraphTogether: boolean; | |||
There was a problem hiding this comment.
Please rename variable to shouldKeepTogether. The JSDoc of the method explains what this setting is meant for and I try to name the properties like they are named in the schema (makes it easier in the toXml method), if possible.
There was a problem hiding this comment.
Addressed in commit c107b2f
src/style/ParagraphProperties.ts
Outdated
| /** | ||
| * Sets the keep together attribute if paragraph marked as keep together. | ||
| * | ||
| * @param {Element} textPropertiesElement The element which will take the attribute |
There was a problem hiding this comment.
Please fix the name of the attribute (the attribute actually is named paragraphPropertiesElement).
I just discovered it's wrong for the other methods in this class as well. If you don't mind, you may adjust the other JSDocs as well.
There was a problem hiding this comment.
Addressed in 3326a03
src/style/ParagraphProperties.ts
Outdated
| paragraphPropertiesElement.setAttribute( | ||
| OdfAttributeName.FormatKeepTogether, | ||
| "always" | ||
| ); |
There was a problem hiding this comment.
max line length is 120, so you can put the whole in one line
There was a problem hiding this comment.
Yeah, that's my local eslint kicking in.
There was a problem hiding this comment.
Addressed in 8aa67a0
|
@connium About your main remarks:
Anything else you think should be addressed for this PR? |
|
Thank you for improving the code. ✨ I'll merge your pull request this evening. |
No description provided.