8
0
Просмотр исходного кода

Vertical keyboard navigation next to isObject elements updated.

Kuba Niegowski 5 лет назад
Родитель
Сommit
d5b7f5d3d5

+ 37 - 24
packages/ckeditor5-engine/src/utils/verticalnavigation.js

@@ -25,7 +25,7 @@ export default function verticalNavigationHandler( editing ) {
 
 		const range = findTextRangeFromSelection( model, editing.mapper, selection, arrowDownPressed );
 
-		if ( range.start.isTouching( range.end ) ) {
+		if ( !range || range.start.isTouching( range.end ) ) {
 			return;
 		}
 
@@ -47,55 +47,68 @@ export default function verticalNavigationHandler( editing ) {
 			data.preventDefault();
 			data.stopPropagation();
 		}
-	}, { priority: 'highest' } );
+	} );
 }
 
 function findTextRangeFromSelection( model, mapper, selection, isForward ) {
 	const schema = model.schema;
 
-	const rootRange = model.createRangeIn( model.document.getRoot() );
-
 	if ( isForward ) {
-		const position = selection.getLastPosition();
-		const range = model.createRange( position, rootRange.end );
-		const lastRangePosition = getNearestVisibleTextPosition( schema, mapper, range, 'forward' );
+		const startPosition = selection.isCollapsed ? selection.focus : selection.getLastPosition();
+		const endPosition = getNearestNonInlineLimit( model, startPosition, 'forward' );
+
+		const range = model.createRange( startPosition, endPosition );
+		const lastRangePosition = getNearestVisibleTextPosition( schema, mapper, range, 'backward' );
 
-		if ( lastRangePosition && position.isBefore( lastRangePosition ) ) {
-			return model.createRange( position, lastRangePosition );
+		if ( lastRangePosition && startPosition.isBefore( lastRangePosition ) ) {
+			return model.createRange( startPosition, lastRangePosition );
 		}
 
 		return null;
 	} else {
-		const position = selection.getFirstPosition();
-		const range = model.createRange( rootRange.start, position );
-		const firstRangePosition = getNearestVisibleTextPosition( schema, mapper, range, 'backward' );
+		const endPosition = selection.isCollapsed ? selection.focus : selection.getFirstPosition();
+		const startPosition = getNearestNonInlineLimit( model, endPosition, 'backward' );
 
-		if ( firstRangePosition && position.isAfter( firstRangePosition ) ) {
-			return model.createRange( firstRangePosition, position );
+		const range = model.createRange( startPosition, endPosition );
+		const firstRangePosition = getNearestVisibleTextPosition( schema, mapper, range, 'forward' );
+
+		if ( firstRangePosition && endPosition.isAfter( firstRangePosition ) ) {
+			return model.createRange( firstRangePosition, endPosition );
 		}
 
 		return null;
 	}
 }
 
-function getNearestVisibleTextPosition( schema, mapper, range, direction ) {
-	let lastTextPosition = null;
+function getNearestNonInlineLimit( model, startPosition, direction ) {
+	const schema = model.schema;
+	const range = model.createRangeIn( startPosition.root );
 
-	for ( const { nextPosition, item, type } of range.getWalker( { direction } ) ) {
-		if ( schema.isLimit( item ) ) {
-			return lastTextPosition;
+	for ( const { previousPosition, item } of range.getWalker( { startPosition, direction } ) ) {
+		if ( schema.isLimit( item ) && !schema.isInline( item ) ) {
+			return previousPosition;
 		}
+	}
 
-		if ( schema.checkChild( item, '$text' ) && type == ( direction == 'forward' ? 'elementEnd' : 'elementStart' ) ) {
-			const viewElement = mapper.toViewElement( item );
+	return direction == 'forward' ? range.end : range.start;
+}
+
+function getNearestVisibleTextPosition( schema, mapper, range, direction ) {
+	const position = direction == 'backward' ? range.end : range.start;
+
+	if ( schema.checkChild( position, '$text' ) ) {
+		return position;
+	}
+
+	for ( const { nextPosition } of range.getWalker( { direction } ) ) {
+		if ( schema.checkChild( nextPosition, '$text' ) ) {
+			const viewElement = mapper.toViewElement( nextPosition.parent );
 
 			if ( viewElement && !viewElement.hasClass( 'ck-hidden' ) ) {
-				lastTextPosition = nextPosition;
+				return nextPosition;
 			}
 		}
 	}
-
-	return lastTextPosition;
 }
 
 function isSingleLineRange( model, editing, modelRange, isForward ) {

+ 6 - 153
packages/ckeditor5-table/src/tablekeyboard.js

@@ -11,7 +11,6 @@ import TableSelection from './tableselection';
 import TableWalker from './tablewalker';
 
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
-import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
 import priorities from '@ckeditor/ckeditor5-utils/src/priorities';
 import {
 	isArrowKeyCode,
@@ -223,46 +222,11 @@ export default class TableKeyboard extends Plugin {
 		const cellRange = model.createRangeIn( tableCell );
 
 		// Let's check if the selection is at the beginning/end of the cell.
-		if ( this._isSelectionAtCellEdge( selection, isForward ) ) {
+		if ( this._isSelectionAtCellEdge( selection, cellRange, isForward ) ) {
 			this._navigateFromCellInDirection( tableCell, direction, expandSelection );
 
 			return true;
 		}
-
-		// If there isn't any $text position between cell edge and selection then we shall move the selection to next cell.
-		const textRange = this._findTextRangeFromSelection( cellRange, selection, isForward );
-
-		if ( !textRange ) {
-			this._navigateFromCellInDirection( tableCell, direction, expandSelection );
-
-			return true;
-		}
-
-		// If the navigation is horizontal then we have no more custom cases.
-		if ( [ 'left', 'right' ].includes( direction ) ) {
-			return false;
-		}
-
-		// If the range is a single line then move the selection to the beginning/end of a cell content.
-		//
-		// We can't move the selection directly to the another cell because of dual position at the end/beginning
-		// 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 => {
-				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;
-		}
 	}
 
 	/**
@@ -270,10 +234,11 @@ export default class TableKeyboard extends Plugin {
 	 *
 	 * @private
 	 * @param {module:engine/model/selection~Selection} selection The current selection.
+	 * @param {module:engine/model/range~Range} cellRange The current table cell content range.
 	 * @param {Boolean} isForward The expected navigation direction.
 	 * @returns {Boolean}
 	 */
-	_isSelectionAtCellEdge( selection, isForward ) {
+	_isSelectionAtCellEdge( selection, cellRange, isForward ) {
 		const model = this.editor.model;
 		const schema = this.editor.model.schema;
 
@@ -282,7 +247,9 @@ export default class TableKeyboard extends Plugin {
 		// If the current limit element is not table cell we are for sure not at the cell edge.
 		// Also `modifySelection` will not let us out of it.
 		if ( !schema.getLimitElement( focus ).is( 'tableCell' ) ) {
-			return false;
+			const boundaryPosition = isForward ? cellRange.end : cellRange.start;
+
+			return boundaryPosition.isTouching( focus );
 		}
 
 		const probe = model.createSelection( focus );
@@ -293,120 +260,6 @@ export default class TableKeyboard extends Plugin {
 		return focus.isEqual( probe.focus );
 	}
 
-	/**
-	 * Truncates the range so that it spans from the last selection position
-	 * to the last allowed `$text` position (mirrored if `isForward` is false).
-	 *
-	 * Returns `null` if, according to the schema, the resulting range cannot contain a `$text` element.
-	 *
-	 * @private
-	 * @param {module:engine/model/range~Range} range The current table cell content range.
-	 * @param {module:engine/model/selection~Selection} selection The current selection.
-	 * @param {Boolean} isForward The expected navigation direction.
-	 * @returns {module:engine/model/range~Range|null}
-	 */
-	_findTextRangeFromSelection( range, selection, isForward ) {
-		const model = this.editor.model;
-
-		if ( isForward ) {
-			const position = selection.getLastPosition();
-			const lastRangePosition = this._getNearestVisibleTextPosition( range, 'backward' );
-
-			if ( lastRangePosition && position.isBefore( lastRangePosition ) ) {
-				return model.createRange( position, lastRangePosition );
-			}
-
-			return null;
-		} else {
-			const position = selection.getFirstPosition();
-			const firstRangePosition = this._getNearestVisibleTextPosition( range, 'forward' );
-
-			if ( firstRangePosition && position.isAfter( firstRangePosition ) ) {
-				return model.createRange( firstRangePosition, position );
-			}
-
-			return null;
-		}
-	}
-
-	/**
-	 * Basing on the provided range, finds the first or last (depending on `direction`) position inside the range
-	 * that can contain `$text` (according to schema) and is visible in the view.
-	 *
-	 * @private
-	 * @param {module:engine/model/range~Range} range The range to find the position in.
-	 * @param {'forward'|'backward'} direction Search direction.
-	 * @returns {module:engine/model/position~Position} The nearest selection range.
-	 */
-	_getNearestVisibleTextPosition( range, direction ) {
-		const schema = this.editor.model.schema;
-		const mapper = this.editor.editing.mapper;
-
-		for ( const { nextPosition, item } of range.getWalker( { direction } ) ) {
-			if ( schema.checkChild( nextPosition, '$text' ) ) {
-				const viewElement = mapper.toViewElement( item );
-
-				if ( viewElement && !viewElement.hasClass( 'ck-hidden' ) ) {
-					return nextPosition;
-				}
-			}
-		}
-	}
-
-	/**
-	 * Checks if the DOM range corresponding to the provided model range renders as a single line by analyzing DOMRects
-	 * (verifying if they visually wrap content to the next line).
-	 *
-	 * @private
-	 * @param {module:engine/model/range~Range} modelRange The current table cell content range.
-	 * @param {Boolean} isForward The expected navigation direction.
-	 * @returns {Boolean}
-	 */
-	_isSingleLineRange( modelRange, isForward ) {
-		const model = this.editor.model;
-		const editing = this.editor.editing;
-		const domConverter = editing.view.domConverter;
-
-		// Wrapped lines contain exactly the same position at the end of current line
-		// and at the beginning of next line. That position's client rect is at the end
-		// of current line. In case of caret at first position of the last line that 'dual'
-		// position would be detected as it's not the last line.
-		if ( isForward ) {
-			const probe = model.createSelection( modelRange.start );
-
-			model.modifySelection( probe );
-
-			// If the new position is at the end of the container then we can't use this position
-			// because it would provide incorrect result for eg caption of image and selection
-			// just before end of it. Also in this case there is no "dual" position.
-			if ( !probe.focus.isAtEnd && !modelRange.start.isEqual( probe.focus ) ) {
-				modelRange = model.createRange( probe.focus, modelRange.end );
-			}
-		}
-
-		const viewRange = editing.mapper.toViewRange( modelRange );
-		const domRange = domConverter.viewRangeToDom( viewRange );
-		const rects = Rect.getDomRangeRects( domRange );
-
-		let boundaryVerticalPosition;
-
-		for ( const rect of rects ) {
-			if ( boundaryVerticalPosition === undefined ) {
-				boundaryVerticalPosition = Math.round( rect.bottom );
-				continue;
-			}
-
-			// Let's check if this rect is in new line.
-			if ( Math.round( rect.top ) >= boundaryVerticalPosition ) {
-				return false;
-			}
-
-			boundaryVerticalPosition = Math.max( boundaryVerticalPosition, Math.round( rect.bottom ) );
-		}
-
-		return true;
-	}
-
 	/**
 	 * Moves the selection from the given table cell in the specified direction.
 	 *

+ 52 - 16
packages/ckeditor5-table/tests/tablekeyboard.js

@@ -2759,7 +2759,7 @@ describe( 'TableKeyboard', () => {
 
 						assertEqualMarkup( getModelData( model ), modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `[<horizontalLine></horizontalLine>]<paragraph>word ${ text }</paragraph>`, '12' ],
+							[ '10', `<horizontalLine></horizontalLine><paragraph>[]word ${ text }</paragraph>`, '12' ],
 							[ '20', '21', '22' ]
 						] ) );
 					} );
@@ -2778,7 +2778,7 @@ describe( 'TableKeyboard', () => {
 
 						assertEqualMarkup( getModelData( model ), modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `<paragraph>${ text } word word</paragraph>[<horizontalLine></horizontalLine>]`, '12' ],
+							[ '10', `<paragraph>${ text } word word[]</paragraph><horizontalLine></horizontalLine>`, '12' ],
 							[ '20', '21', '22' ]
 						] ) );
 					} );
@@ -2844,7 +2844,7 @@ describe( 'TableKeyboard', () => {
 
 							assertEqualMarkup( getModelData( model ), modelTable( [
 								[ '00', '01', '02' ],
-								[ '10', '[<horizontalLine></horizontalLine><paragraph>foo]bar</paragraph>', '12' ],
+								[ '10', '<horizontalLine></horizontalLine><paragraph>[foo]bar</paragraph>', '12' ],
 								[ '20', '21', '22' ]
 							] ) );
 						} );
@@ -2863,7 +2863,7 @@ describe( 'TableKeyboard', () => {
 
 							assertEqualMarkup( getModelData( model ), modelTable( [
 								[ '00', '01', '02' ],
-								[ '10', '<paragraph>foo[bar</paragraph><horizontalLine></horizontalLine>]', '12' ],
+								[ '10', '<paragraph>foo[bar]</paragraph><horizontalLine></horizontalLine>', '12' ],
 								[ '20', '21', '22' ]
 							] ) );
 						} );
@@ -2897,7 +2897,7 @@ describe( 'TableKeyboard', () => {
 						sinon.assert.notCalled( rightArrowDomEvtDataStub.stopPropagation );
 					} );
 
-					it( 'should not navigate to the cell above', () => {
+					it( 'should not navigate to the cell above (only to closest limit boundary)', () => {
 						setModelData( model, modelTable( [
 							[ '00', '01', '02' ],
 							[ '10', `<paragraph>foo</paragraph><image src="${ imageUrl }"><caption>1[]1</caption></image>`, '12' ],
@@ -2906,11 +2906,30 @@ describe( 'TableKeyboard', () => {
 
 						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>foo</paragraph><image src="${ imageUrl }"><caption>[]11</caption></image>`, '12' ],
+							[ '20', '21', '22' ]
+						] ) );
+					} );
+
+					it( 'should not navigate to the cell above (only to paragraph above)', () => {
+						setModelData( model, modelTable( [
+							[ '00', '01', '02' ],
+							[ '10', `<paragraph>foo</paragraph><image src="${ imageUrl }"><caption>[]11</caption></image>`, '12' ],
+							[ '20', '21', '22' ]
+						] ) );
+
+						editor.editing.view.document.fire( 'keydown', upArrowDomEvtDataStub );
+
 						sinon.assert.notCalled( upArrowDomEvtDataStub.preventDefault );
 						sinon.assert.notCalled( upArrowDomEvtDataStub.stopPropagation );
 					} );
 
-					it( 'should not navigate to the cell above but should select the image widget', () => {
+					it( 'should not navigate to the cell above but should put caret at first position of the image caption', () => {
 						setModelData( model, modelTable( [
 							[ '00', '01', '02' ],
 							[ '10', `<image src="${ imageUrl }"><caption>1[]1</caption></image><paragraph>foo</paragraph>`, '12' ],
@@ -2924,12 +2943,12 @@ describe( 'TableKeyboard', () => {
 
 						assertEqualMarkup( getModelData( model ), modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `[<image src="${ imageUrl }"><caption>11</caption></image>]<paragraph>foo</paragraph>`, '12' ],
+							[ '10', `<image src="${ imageUrl }"><caption>[]11</caption></image><paragraph>foo</paragraph>`, '12' ],
 							[ '20', '21', '22' ]
 						] ) );
 					} );
 
-					it( 'should not navigate to the cell below when followed by a paragraph', () => {
+					it( 'should not navigate to the cell below when inside the image caption', () => {
 						setModelData( model, modelTable( [
 							[ '00', '01', '02' ],
 							[ '10', `<image src="${ imageUrl }"><caption>1[]1</caption></image><paragraph>foo</paragraph>`, '12' ],
@@ -2938,14 +2957,33 @@ describe( 'TableKeyboard', () => {
 
 						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', `<image src="${ imageUrl }"><caption>11[]</caption></image><paragraph>foo</paragraph>`, '12' ],
+							[ '20', '21', '22' ]
+						] ) );
+					} );
+
+					it( 'should not navigate to the cell below when followed by a paragraph', () => {
+						setModelData( model, modelTable( [
+							[ '00', '01', '02' ],
+							[ '10', `<image src="${ imageUrl }"><caption>11[]</caption></image><paragraph>foo</paragraph>`, '12' ],
+							[ '20', '21', '22' ]
+						] ) );
+
+						editor.editing.view.document.fire( 'keydown', downArrowDomEvtDataStub );
+
 						sinon.assert.notCalled( downArrowDomEvtDataStub.preventDefault );
 						sinon.assert.notCalled( downArrowDomEvtDataStub.stopPropagation );
 					} );
 
-					it( 'should not navigate to the cell below but should select the image widget', () => {
+					it( 'should navigate to the cell below if the caret on last position in the image caption', () => {
 						setModelData( model, modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `<paragraph>foo</paragraph><image src="${ imageUrl }"><caption>1[]1</caption></image>`, '12' ],
+							[ '10', `<paragraph>foo</paragraph><image src="${ imageUrl }"><caption>11[]</caption></image>`, '12' ],
 							[ '20', '21', '22' ]
 						] ) );
 
@@ -2956,22 +2994,21 @@ describe( 'TableKeyboard', () => {
 
 						assertEqualMarkup( getModelData( model ), modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `<paragraph>foo</paragraph>[<image src="${ imageUrl }"><caption>11</caption></image>]`, '12' ],
-							[ '20', '21', '22' ]
+							[ '10', `<paragraph>foo</paragraph><image src="${ imageUrl }"><caption>11</caption></image>`, '12' ],
+							[ '20', '[]21', '22' ]
 						] ) );
 					} );
 
 					it( 'should not navigate to the cell above but should select the image widget without caption', () => {
 						setModelData( model, modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `<image src="${ imageUrl }"><caption></caption></image><paragraph>f[]oo</paragraph>`, '12' ],
+							[ '10', `<image src="${ imageUrl }"><caption></caption></image><paragraph>[]foo</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' ],
@@ -2983,14 +3020,13 @@ describe( 'TableKeyboard', () => {
 					it( 'should not navigate to the cell below but should select the image widget without caption', () => {
 						setModelData( model, modelTable( [
 							[ '00', '01', '02' ],
-							[ '10', `<paragraph>f[]oo</paragraph><image src="${ imageUrl }"><caption></caption></image>`, '12' ],
+							[ '10', `<paragraph>foo[]</paragraph><image src="${ imageUrl }"><caption></caption></image>`, '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' ],