Skip to content

Commit

Permalink
Merge pull request #6727 from ckeditor/i/6641
Browse files Browse the repository at this point in the history 
Fix (table): Table keyboard navigation should not alter the native Shift+Arrow behavior inside table cells. Closes #6641.
  • Loading branch information
@jodator
jodator authored May 5, 2020
2 parents 887c6a5 + 894f26f commit 8854337
Show file tree
Hide file tree
Showing 2 changed files with 317 additions and 3 deletions.
17 changes: 14 additions & 3 deletions packages/ckeditor5-table/src/tablenavigation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 );
Expand Down Expand Up @@ -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;
Expand Down
303 changes: 303 additions & 0 deletions packages/ckeditor5-table/tests/tablenavigation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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' ]
] ) );
} );
} );
} );
}

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 8854337

Please sign in to comment.