Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #280 from ckeditor/i/6430
Browse files Browse the repository at this point in the history
Fix: `TableSelection` plugin should collapse a multi-cell selection when it gets disabled. Closes ckeditor/ckeditor5#6430.
  • Loading branch information
jodator authored Mar 24, 2020
2 parents 7c1d41c + 2bef2e9 commit ba852e3
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 0 deletions.
29 changes: 29 additions & 0 deletions src/tableselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export default class TableSelection extends Plugin {
this._defineSelectionConverter();
this._enableShiftClickSelection();
this._enableMouseDragSelection();
this._enablePluginDisabling(); // sic!
}

/**
Expand Down Expand Up @@ -292,6 +293,34 @@ export default class TableSelection extends Plugin {
}, { priority: 'highest' } );
}

/**
* Creates a listener that reacts to changes in {@link #isEnabled} and, if the plugin was disabled,
* it collapses the multi-cell selection to a regular selection placed inside a table cell.
*
* This listener helps features that disable the table selection plugin bring the selection
* to a clear state they can work with (for instance, because they don't support multiple cell selection).
*/
_enablePluginDisabling() {
const editor = this.editor;

this.on( 'change:isEnabled', () => {
if ( !this.isEnabled ) {
const selectedCells = this.getSelectedTableCells();

if ( !selectedCells ) {
return;
}

editor.model.change( writer => {
const position = writer.createPositionAt( selectedCells[ 0 ], 0 );
const range = editor.model.schema.getNearestSelectionRange( position );

writer.setSelection( range );
} );
}
} );
}

/**
* It overrides the default `model.deleteContent()` behavior over a selected table fragment.
*
Expand Down
39 changes: 39 additions & 0 deletions tests/tableselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,45 @@ describe( 'table selection', () => {
afterEach( async () => {
await editor.destroy();
} );

describe( 'init()', () => {
describe( 'plugin disabling support', () => {
let plugin;

beforeEach( () => {
plugin = editor.plugins.get( TableSelection );
} );

it( 'should collapse multi-cell selection when the plugin gets disabled', () => {
const firstCell = modelRoot.getNodeByPath( [ 0, 0, 0 ] );
const lastCell = modelRoot.getNodeByPath( [ 0, 1, 1 ] );

tableSelection._setCellSelection(
firstCell,
lastCell
);

plugin.forceDisabled( 'foo' );

const ranges = [ ...model.document.selection.getRanges() ];

expect( ranges ).to.have.length( 1 );
expect( ranges[ 0 ].isCollapsed ).to.be.true;
expect( ranges[ 0 ].start.path ).to.deep.equal( [ 0, 0, 0, 0, 0 ] );
} );

it( 'should do nothing if there were no multi-cell selections', () => {
plugin.forceDisabled( 'foo' );

const ranges = [ ...model.document.selection.getRanges() ];

expect( ranges ).to.have.length( 1 );
expect( ranges[ 0 ].isCollapsed ).to.be.true;
expect( ranges[ 0 ].start.path ).to.deep.equal( [ 0, 0, 0, 0, 2 ] );
} );
} );
} );

describe( 'selection by shift+click', () => {
it( 'should...', () => {
// tableSelection.startSelectingFrom( modelRoot.getNodeByPath( [ 0, 0, 0 ] ) );
Expand Down

0 comments on commit ba852e3

Please sign in to comment.