Skip to content

Commit

Permalink
Merge pull request #12603 from ckeditor/cf/4746-table-caption-tc
Browse files Browse the repository at this point in the history
Fix (table): Selection should not change when table caption is turned off.

Internal (table, list): Use `model.insertContent()` in table caption instead of using the writer directly.
  • Loading branch information
scofalik authored Oct 17, 2022
2 parents 4c5815f + d8f9054 commit ffe3fe0
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 14 deletions.
4 changes: 2 additions & 2 deletions packages/ckeditor5-list/src/list/converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ export function modelChangePostFixer( model, writer ) {
* @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event.
* @param {Array} args Arguments of {@link module:engine/model/model~Model#insertContent}.
*/
export function modelIndentPasteFixer( evt, [ content, selectable ] ) {
export function modelIndentPasteFixer( evt, [ content, selectable, placeOrOffset ] ) {
// Check whether inserted content starts from a `listItem`. If it does not, it means that there are some other
// elements before it and there is no need to fix indents, because even if we insert that content into a list,
// that list will be broken.
Expand All @@ -775,7 +775,7 @@ export function modelIndentPasteFixer( evt, [ content, selectable ] ) {
if ( !selectable ) {
selection = this.document.selection;
} else {
selection = this.createSelection( selectable );
selection = this.createSelection( selectable, placeOrOffset );
}

if ( item && item.is( 'element', 'listItem' ) ) {
Expand Down
11 changes: 11 additions & 0 deletions packages/ckeditor5-list/tests/list/listediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -4156,6 +4156,17 @@ describe( 'ListEditing', () => {
);
} );

it( 'should work if a selectable element is passed to DataController#insertContent()', () => {
model.change( writer => {
const listItem = writer.createElement( 'listItem', { listType: 'bulleted', listIndent: '0' } );
model.insertContent( listItem, modelRoot, 'in' );
} );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listType="bulleted">[]</listItem>'
);
} );

it( 'should fix indents of pasted list items', () => {
setModelData( model,
'<listItem listType="bulleted" listIndent="0">A</listItem>' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export default class ToggleTableCaptionCommand extends Command {
// Try restoring the caption from the TableCaptionEditing plugin storage.
const newCaptionElement = savedCaptionElement || writer.createElement( 'caption' );

writer.append( newCaptionElement, tableElement );
model.insertContent( newCaptionElement, tableElement, 'end' );

if ( focusCaptionOnShow ) {
writer.setSelection( newCaptionElement, 'in' );
Expand All @@ -114,7 +114,6 @@ export default class ToggleTableCaptionCommand extends Command {
// Store the caption content so it can be restored quickly if the user changes their mind.
tableCaptionEditing._saveCaption( tableElement, captionElement );

writer.setSelection( writer.createRangeIn( tableElement.getChild( 0 ).getChild( 0 ) ) );
writer.remove( captionElement );
model.deleteContent( writer.createSelection( captionElement, 'on' ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import { modelTable } from '../_utils/utils';

import ToggleTableCaptionCommand from '../../src/tablecaption/toggletablecaptioncommand';
import TableCaptionEditing from '../../src/tablecaption/tablecaptionediting';
import { BoldEditing } from '@ckeditor/ckeditor5-basic-styles';

describe( 'ToggleTableCaptionCommand', () => {
let editor, model, command;

beforeEach( () => {
return ModelTestEditor
.create( {
plugins: [ Paragraph, TableEditing, TableCaptionEditing, TableSelection ]
plugins: [ Paragraph, TableEditing, TableCaptionEditing, TableSelection, BoldEditing ]
} )
.then( newEditor => {
editor = newEditor;
Expand Down Expand Up @@ -119,7 +120,7 @@ describe( 'ToggleTableCaptionCommand', () => {
'<table>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph>[11]</paragraph>' +
'<paragraph>11[]</paragraph>' +
'</tableCell>' +
'<tableCell>' +
'<paragraph>12</paragraph>' +
Expand Down Expand Up @@ -177,13 +178,13 @@ describe( 'ToggleTableCaptionCommand', () => {
command.execute();

expect( getData( model ) ).to.equalMarkup(
'<table>' +
'[<table>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph>[]</paragraph>' +
'<paragraph></paragraph>' +
'</tableCell>' +
'</tableRow>' +
'</table>'
'</table>]'
);
} );

Expand Down Expand Up @@ -217,7 +218,7 @@ describe( 'ToggleTableCaptionCommand', () => {
'<table>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph></paragraph>' +
'<paragraph>[]</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'<caption>Foo<$text bold="true">bar</$text></caption>' +
Expand Down Expand Up @@ -255,7 +256,7 @@ describe( 'ToggleTableCaptionCommand', () => {
'<table>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph></paragraph>' +
'<paragraph>A[]bc</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'<caption>Foo</caption>' +
Expand All @@ -269,7 +270,7 @@ describe( 'ToggleTableCaptionCommand', () => {
'<table>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph>[]</paragraph>' +
'<paragraph>A[]bc</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'</table>'
Expand All @@ -294,7 +295,7 @@ describe( 'ToggleTableCaptionCommand', () => {
'<table>' +
'<tableRow>' +
'<tableCell>' +
'<paragraph>[]</paragraph>' +
'<paragraph>A[]bc</paragraph>' +
'</tableCell>' +
'</tableRow>' +

Expand Down

0 comments on commit ffe3fe0

Please sign in to comment.