Browse Source

Merge pull request #6727 from ckeditor/i/6641

Fix (table): Table keyboard navigation should not alter the native Shift+Arrow behavior inside table cells. Closes #6641.
Maciej 5 years ago
parent
commit
88543374bc

+ 14 - 3
packages/ckeditor5-table/src/tablenavigation.js

@@ -160,7 +160,8 @@ export default class TableNavigation extends Plugin {
 			return;
 		}
 
-		const wasHandled = this._handleArrowKeys( getDirectionFromKeyCode( keyCode, this.editor.locale.contentLanguageDirection ) );
+		const direction = getDirectionFromKeyCode( keyCode, this.editor.locale.contentLanguageDirection );
+		const wasHandled = this._handleArrowKeys( direction, domEventData.shiftKey );
 
 		if ( wasHandled ) {
 			domEventData.preventDefault();
@@ -174,9 +175,10 @@ export default class TableNavigation extends Plugin {
 	 *
 	 * @private
 	 * @param {'left'|'up'|'right'|'down'} direction The direction of the arrow key.
+	 * @param {Boolean} expandSelection If the current selection should be expanded.
 	 * @returns {Boolean} Returns `true` if key was handled.
 	 */
-	_handleArrowKeys( direction ) {
+	_handleArrowKeys( direction, expandSelection ) {
 		const model = this.editor.model;
 		const selection = model.document.selection;
 		const isForward = [ 'right', 'down' ].includes( direction );
@@ -242,7 +244,16 @@ export default class TableNavigation extends Plugin {
 		// of wrapped line (it's at the same time at the end of one line and at the start of the next line).
 		if ( this._isSingleLineRange( textRange, isForward ) ) {
 			model.change( writer => {
-				writer.setSelection( isForward ? cellRange.end : cellRange.start );
+				const newPosition = isForward ? cellRange.end : cellRange.start;
+
+				if ( expandSelection ) {
+					const newSelection = model.createSelection( selection.anchor );
+					newSelection.setFocus( newPosition );
+
+					writer.setSelection( newSelection );
+				} else {
+					writer.setSelection( newPosition );
+				}
 			} );
 
 			return true;

+ 303 - 0
packages/ckeditor5-table/tests/tablenavigation.js

@@ -1714,6 +1714,10 @@ describe( 'TableNavigation', () => {
 						`
 					) );
 					global.document.querySelector( 'head' ).appendChild( styleElement );
+
+					// The editing view must be focused because otherwise in Chrome the DOM selection will not contain
+					// any ranges and jumpOverUiElement will crash (for the right arrow when shift is pressed).
+					editor.editing.view.focus();
 				} );
 
 				afterEach( async () => {
@@ -1786,6 +1790,61 @@ describe( 'TableNavigation', () => {
 							[ '20', '21', '22' ]
 						] ) );
 					} );
+
+					describe( 'when shift key is pressed', () => {
+						beforeEach( () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '1[]1', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							leftArrowDomEvtDataStub.shiftKey = true;
+							rightArrowDomEvtDataStub.shiftKey = true;
+							upArrowDomEvtDataStub.shiftKey = true;
+							downArrowDomEvtDataStub.shiftKey = true;
+						} );
+
+						it( 'should not prevent default browser behavior for the left arrow pressed with shift', () => {
+							editor.editing.view.document.fire( 'keydown', leftArrowDomEvtDataStub );
+
+							sinon.assert.notCalled( leftArrowDomEvtDataStub.preventDefault );
+							sinon.assert.notCalled( leftArrowDomEvtDataStub.stopPropagation );
+						} );
+
+						it( 'should not prevent default browser behavior for the right arrow pressed with shift', () => {
+							editor.editing.view.document.fire( 'keydown', rightArrowDomEvtDataStub );
+
+							sinon.assert.notCalled( rightArrowDomEvtDataStub.preventDefault );
+							sinon.assert.notCalled( rightArrowDomEvtDataStub.stopPropagation );
+						} );
+
+						it( 'should expand selection to the beginning of the cell content', () => {
+							editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '[1]1', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+
+						it( 'should expand selection to the end of the cell content', () => {
+							editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '1[1]', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+					} );
 				} );
 
 				describe( 'selection inside paragraph', () => {
@@ -1841,6 +1900,115 @@ describe( 'TableNavigation', () => {
 							[ '20', '21', '22' ]
 						] ) );
 					} );
+
+					describe( 'when shift key is pressed', () => {
+						beforeEach( () => {
+							upArrowDomEvtDataStub.shiftKey = true;
+							downArrowDomEvtDataStub.shiftKey = true;
+						} );
+
+						it( 'should not prevent default browser behavior for the up arrow in the middle lines of the cell text', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', text + '[] ' + text, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
+							sinon.assert.notCalled( upArrowDomEvtDataStub.preventDefault );
+							sinon.assert.notCalled( upArrowDomEvtDataStub.stopPropagation );
+						} );
+
+						it( 'should not prevent default browser behavior for the down arrow in the middle lines of cell text', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', text + '[] ' + text, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+							sinon.assert.notCalled( downArrowDomEvtDataStub.preventDefault );
+							sinon.assert.notCalled( downArrowDomEvtDataStub.stopPropagation );
+						} );
+
+						it( 'should expand collapsed selection to the beginning of the cell content', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', 'word[] word' + text, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '[word] word' + text, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+
+						it( 'should expand not collapsed selection to the beginning of the cell content from the selection anchor', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', 'word [word]' + text, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '[word ]word' + text, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+
+						it( 'should expand collapsed selection to the end of the cell content', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', text + 'word[] word', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', text + 'word[ word]', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+
+						it( 'should expand not collapsed selection to the end of the cell content from the selection anchor', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', text + '[word] word', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', text + '[word word]', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+					} );
 				} );
 
 				if ( !env.isGecko ) {
@@ -1901,6 +2069,51 @@ describe( 'TableNavigation', () => {
 							sinon.assert.notCalled( downArrowDomEvtDataStub.preventDefault );
 							sinon.assert.notCalled( downArrowDomEvtDataStub.stopPropagation );
 						} );
+
+						describe( 'when shift key is pressed', () => {
+							beforeEach( () => {
+								upArrowDomEvtDataStub.shiftKey = true;
+								downArrowDomEvtDataStub.shiftKey = true;
+							} );
+
+							it( 'should expand collapsed selection to the end of the cell content', () => {
+								setModelData( model, modelTable( [
+									[ '00', '01', '02' ],
+									[ '10', text + '[] word word word', '12' ],
+									[ '20', '21', '22' ]
+								] ) );
+
+								editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+								sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+								sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+								assertEqualMarkup( getModelData( model ), modelTable( [
+									[ '00', '01', '02' ],
+									[ '10', text + '[ word word word]', '12' ],
+									[ '20', '21', '22' ]
+								] ) );
+							} );
+
+							it( 'should expand not collapsed selection to the end of the cell content from the selection anchor', () => {
+								setModelData( model, modelTable( [
+									[ '00', '01', '02' ],
+									[ '10', text + '[ word] word word', '12' ],
+									[ '20', '21', '22' ]
+								] ) );
+
+								editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+								sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+								sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+								assertEqualMarkup( getModelData( model ), modelTable( [
+									[ '00', '01', '02' ],
+									[ '10', text + '[ word word word]', '12' ],
+									[ '20', '21', '22' ]
+								] ) );
+							} );
+						} );
 					} );
 				}
 
@@ -1970,6 +2183,51 @@ describe( 'TableNavigation', () => {
 							[ '20', '21', '22' ]
 						] ) );
 					} );
+
+					describe( 'when shift key is pressed', () => {
+						beforeEach( () => {
+							upArrowDomEvtDataStub.shiftKey = true;
+							downArrowDomEvtDataStub.shiftKey = true;
+						} );
+
+						it( 'should expand selection to the beginning of the cell content', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', `<paragraph>word[] ${ text }</paragraph><paragraph>${ text }</paragraph>`, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', `<paragraph>[word] ${ text }</paragraph><paragraph>${ text }</paragraph>`, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+
+						it( 'should expand selection to the end of the cell content', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', `<paragraph>${ text }</paragraph><paragraph>${ text } []word</paragraph>`, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', `<paragraph>${ text }</paragraph><paragraph>${ text } [word]</paragraph>`, '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+					} );
 				} );
 
 				describe( 'with horizontal line widget', () => {
@@ -2063,6 +2321,51 @@ describe( 'TableNavigation', () => {
 							[ '20', '21', '22' ]
 						] ) );
 					} );
+
+					describe( 'when shift key is pressed', () => {
+						beforeEach( () => {
+							upArrowDomEvtDataStub.shiftKey = true;
+							downArrowDomEvtDataStub.shiftKey = true;
+						} );
+
+						it( 'should expand selection to the beginning of the cell content', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '<horizontalLine></horizontalLine><paragraph>foo[]bar</paragraph>', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( upArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '[<horizontalLine></horizontalLine><paragraph>foo]bar</paragraph>', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+
+						it( 'should expand selection to the end of the cell content', () => {
+							setModelData( model, modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '<paragraph>foo[]bar</paragraph><horizontalLine></horizontalLine>', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+
+							editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.preventDefault );
+							sinon.assert.calledOnce( downArrowDomEvtDataStub.stopPropagation );
+
+							assertEqualMarkup( getModelData( model ), modelTable( [
+								[ '00', '01', '02' ],
+								[ '10', '<paragraph>foo[bar</paragraph><horizontalLine></horizontalLine>]', '12' ],
+								[ '20', '21', '22' ]
+							] ) );
+						} );
+					} );
 				} );
 
 				describe( 'contains image widget with caption and selection inside the caption', () => {