Add support for adding/removing space before/after paragraphs in content model #1565
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { RibbonButton } from 'roosterjs-react'; | ||
| import { isContentModelEditor, toggleMargins } from 'roosterjs-content-model'; | ||
|
|
||
| const spaceAfterButtonKey = 'buttonNameSpaceAfter'; | ||
| const spaceBeforeButtonKey = 'buttonNameSpaceBefore'; | ||
|
|
||
| /** | ||
| * @internal | ||
| * "Add space after" button on the format ribbon | ||
| */ | ||
| export const spaceAfterButton: RibbonButton<typeof spaceAfterButtonKey> = { | ||
| key: spaceAfterButtonKey, | ||
| unlocalizedText: 'Remove space after', | ||
| iconName: 'CaretDown8', | ||
| isChecked: formatState => !formatState.marginBottom || parseInt(formatState.marginBottom) <= 0, | ||
| onClick: editor => { | ||
| if (isContentModelEditor(editor)) { | ||
| toggleMargins(editor, { | ||
| marginBottom: '8pt', | ||
| }); | ||
| } | ||
| return true; | ||
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * @internal | ||
| * "Add space before" button on the format ribbon | ||
| */ | ||
| export const spaceBeforeButton: RibbonButton<typeof spaceBeforeButtonKey> = { | ||
| key: spaceBeforeButtonKey, | ||
| unlocalizedText: 'Add space before', | ||
| iconName: 'CaretUp8', | ||
| isChecked: formatState => parseInt(formatState.marginTop) > 0, | ||
| onClick: editor => { | ||
| if (isContentModelEditor(editor)) { | ||
| toggleMargins(editor, { | ||
| marginTop: '12pt', | ||
| }); | ||
| } | ||
| return true; | ||
| }, | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { getObjectKeys } from 'roosterjs-editor-dom'; | ||
| import { createParagraphDecorator } from '../../modelApi/creators/createParagraphDecorator'; | ||
| import { MarginFormat } from '../../publicTypes/format/formatParts/MarginFormat'; | ||
| import { IContentModelEditor } from '../../publicTypes/IContentModelEditor'; | ||
| import { formatParagraphWithContentModel } from '../utils/formatParagraphWithContentModel'; | ||
|
|
||
| /** | ||
| * Toggles the current block(s) margin properties. | ||
| * @param editor The editor to operate on | ||
| * @param marginFormat Object containing margin props to be changed. | ||
| * If a margin property is not in this object, it won't be changed. | ||
| */ | ||
| export default function toggleMargins(editor: IContentModelEditor, marginFormat: MarginFormat) { | ||
ianeli1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| formatParagraphWithContentModel(editor, 'toggleMargins', para => { | ||
| if (!para.decorator || para.decorator.tagName !== 'p') { | ||
| para.decorator = createParagraphDecorator('p'); | ||
JiuqingSong marked this conversation as resolved.
Show resolved
Hide resolved
ianeli1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| getObjectKeys(marginFormat).forEach(key => { | ||
|
There was a problem hiding this comment. what if marginFormat only contain marginTop but existing format contains marginBottom? Do we want to delete it? getObjectKeys() can only return keys that exists in marginFormat There was a problem hiding this comment. I was going for a function that let us toggle marginTop and/or marginBottom in one implementation. So if a key isn't passed, it will be ignored There was a problem hiding this comment. Let' just have separate parameters and handle them one by one. Using getObjectKeys() may accidentally overwrite properties that we don't want to if user pass in a larger object then expected parameter type as sub class. |
||
| if ( | ||
| para.format[key] && | ||
| (para.format[key] === marginFormat[key] || parseInt(para.format[key]!) > 0) | ||
|
There was a problem hiding this comment. why delete para.format[key] when format[key] == marginFormat[key]? There was a problem hiding this comment. To answer this and "I think name it "setMargins" is better", if we make it delete the margin format when format is already set, we can toggle the value, no need to get formatState to apply margin. Also removes a positive margin (coming from copy paste for example) There was a problem hiding this comment. Hmm, I think that make cause some confusion to user. If user doesn't want to change the margin, they need to be very careful to make sure not passing in a value that is the same with existing value. So user first need to get the existing value to check if it is the same with the one they want to check. When a function is named "toggle", it means user doesn't need to know the original value, but just call it to turn on or off. So still suggest:
There was a problem hiding this comment. We have an API "toggleHeader" in original roosterjs code which is a bad example. In content model I have renamed it to be "setHeaderLevel" There was a problem hiding this comment. Got it, went ahead with all of your fixes |
||
| ) { | ||
| delete para.format[key]; | ||
| } else { | ||
| para.format[key] = marginFormat[key]; | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| import { ContentModelDocument } from '../../../lib/publicTypes/group/ContentModelDocument'; | ||
| import { paragraphTestCommon } from './paragraphTestCommon'; | ||
| import toggleMargins from '../../../lib/publicApi/block/toggleMargins'; | ||
| import { MarginFormat } from '../../../lib/publicTypes/format/formatParts/MarginFormat'; | ||
|
|
||
| describe('setMargins', () => { | ||
| function runTest( | ||
| model: ContentModelDocument, | ||
| result: ContentModelDocument, | ||
| marginFormat: MarginFormat, | ||
| calledTimes: number = 1 | ||
| ) { | ||
| paragraphTestCommon( | ||
| 'toggleMargins', | ||
| editor => toggleMargins(editor, marginFormat), | ||
| model, | ||
| result, | ||
| calledTimes | ||
| ); | ||
| } | ||
|
|
||
| it('empty content', () => { | ||
| runTest( | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [], | ||
| }, | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [], | ||
| }, | ||
| { | ||
| marginTop: '8px', | ||
| }, | ||
| 0 | ||
| ); | ||
| }); | ||
|
|
||
| it('no selection', () => { | ||
| runTest( | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: {}, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: {}, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| marginTop: '8px', | ||
| }, | ||
| 0 | ||
| ); | ||
| }); | ||
|
|
||
| it('Collapsed selection', () => { | ||
| runTest( | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: {}, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| }, | ||
| { | ||
| segmentType: 'SelectionMarker', | ||
| format: {}, | ||
| isSelected: true, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: { | ||
| marginTop: '8px', | ||
| }, | ||
| decorator: { | ||
| tagName: 'p', | ||
| format: {}, | ||
| }, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| }, | ||
| { | ||
| segmentType: 'SelectionMarker', | ||
| format: {}, | ||
| isSelected: true, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| marginTop: '8px', | ||
| }, | ||
| 1 | ||
| ); | ||
| }); | ||
|
|
||
| it('With selection', () => { | ||
| runTest( | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: {}, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| isSelected: true, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: { | ||
| marginTop: '8px', | ||
| }, | ||
| decorator: { | ||
| tagName: 'p', | ||
| format: {}, | ||
| }, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| isSelected: true, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { marginTop: '8px' }, | ||
| 1 | ||
| ); | ||
| }); | ||
|
|
||
| it('Replaces properties correctly', () => { | ||
| runTest( | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: { | ||
| marginTop: '8px', | ||
| marginRight: '12px', | ||
| }, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| }, | ||
| { | ||
| segmentType: 'SelectionMarker', | ||
| format: {}, | ||
| isSelected: true, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| blockGroupType: 'Document', | ||
| blocks: [ | ||
| { | ||
| blockType: 'Paragraph', | ||
| format: { | ||
| marginRight: '12px', | ||
| }, | ||
| decorator: { | ||
| tagName: 'p', | ||
| format: {}, | ||
| }, | ||
| segments: [ | ||
| { | ||
| segmentType: 'Text', | ||
| text: 'test', | ||
| format: {}, | ||
| }, | ||
| { | ||
| segmentType: 'SelectionMarker', | ||
| format: {}, | ||
| isSelected: true, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| { marginTop: '8px' }, | ||
| 1 | ||
| ); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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.
Do we want to support margin left and right here? If not, you can make the parameter for margin top and bottom only