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

getNearestSelectionPosition changes #717

Merged
merged 6 commits into from
Dec 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions src/controller/insertcontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,22 @@ export default function insertContent( dataController, content, selection, batch
isLast: true
} );

selection.setRanges( insertion.getSelectionRanges() );
const newRange = insertion.getSelectionRange();

/* istanbul ignore else */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have every case like this commented. We're not testing else because it's a safe check for some unpredictable edge cases. Also, having a log.warning() would be good there, like in the other cases in this method.

if ( newRange ) {
selection.setRanges( [ newRange ] );
} else {
// We are not testing else because it's a safe check for unpredictable edge cases:
// an insertion without proper range to select.

/**
* Cannot determine a proper selection range after insertion.
*
* @warning insertcontent-no-range
*/
log.warn( 'insertcontent-no-range: Cannot determine a proper selection range after insertion.' );
}
}

/**
Expand Down Expand Up @@ -123,19 +138,17 @@ class Insertion {
}

/**
* Returns a range to be selected after insertion.
* Returns range to be selected after insertion.
* Returns null if there is no valid range to select after insertion.
*
* @returns {module:engine/model/range~Range}
* @returns {module:engine/model/range~Range|null}
*/
getSelectionRanges() {
getSelectionRange() {
if ( this.nodeToSelect ) {
return [ Range.createOn( this.nodeToSelect ) ];
} else {
const document = this.dataController.model;
const selectionPosition = document.getNearestSelectionPosition( this.position );

return [ new Range( selectionPosition ) ];
return Range.createOn( this.nodeToSelect );
}

return this.dataController.model.getNearestSelectionRange( this.position );
}

/**
Expand Down
108 changes: 76 additions & 32 deletions src/model/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,51 +280,55 @@ export default class Document {
}

/**
* Basing on given `position`, finds and returns a {@link module:engine/model/position~Position Position} instance that is nearest
* to that `position` and is a correct position for selection. A position is correct for selection if
* text node can be placed at that position.
* Basing on given `position`, finds and returns a {@link module:engine/model/range~Range Range} instance that is
* nearest to that `position` and is a correct range for selection.
*
* If no correct position is found, the first position in given `position` root is returned. This can happen if no node
* has been added to the root or it may mean incorrect model document state.
* Correct selection range might be collapsed - when it's located in position where text node can be placed.
* Non-collapsed range is returned when selection can be placed around element marked as "object" in
* {@link module:engine/model/schema~Schema schema}.
*
* @param {module:engine/model/position~Position} position Reference position where selection position should be looked for.
* @returns {module:engine/model/position~Position|null} Nearest selection position.
* Direction of searching for nearest correct selection range can be specified as:
* * `both` - searching will be performed in both ways,
* * `forward` - searching will be performed only forward,
* * `backward` - searching will be performed only backward.
*
* When valid selection range cannot be found, `null` is returned.
*
* @param {module:engine/model/position~Position} position Reference position where new selection range should be looked for.
* @param {'both'|'forward'|'backward'} [direction='both'] Search direction.
* @returns {module:engine/model/range~Range|null} Nearest selection range or `null` if one cannot be found.
*/
getNearestSelectionPosition( position ) {
getNearestSelectionRange( position, direction = 'both' ) {
// Return collapsed range if provided position is valid.
if ( this.schema.check( { name: '$text', inside: position } ) ) {
return Position.createFromPosition( position );
return new Range( position );
}

const backwardWalker = new TreeWalker( { startPosition: position, direction: 'backward' } );
const forwardWalker = new TreeWalker( { startPosition: position } );
let backwardWalker;
let forwardWalker;

let done = false;

while ( !done ) {
done = true;
if ( direction == 'both' || direction == 'backward' ) {
backwardWalker = new TreeWalker( { startPosition: position, direction: 'backward' } );
}

let step = backwardWalker.next();
if ( direction == 'both' || direction == 'forward' ) {
forwardWalker = new TreeWalker( { startPosition: position } );
}

if ( !step.done ) {
if ( this.schema.check( { name: '$text', inside: step.value.nextPosition } ) ) {
return step.value.nextPosition;
}
for ( let data of combineWalkers( backwardWalker, forwardWalker ) ) {
const type = ( data.walker == backwardWalker ? 'elementEnd' : 'elementStart' );
const value = data.value;

done = false;
if ( value.type == type && this.schema.objects.has( value.item.name ) ) {
return Range.createOn( value.item );
}

step = forwardWalker.next();

if ( !step.done ) {
if ( this.schema.check( { name: '$text', inside: step.value.nextPosition } ) ) {
return step.value.nextPosition;
}

done = false;
if ( this.schema.check( { name: '$text', inside: value.nextPosition } ) ) {
return new Range( value.nextPosition );
}
}

return new Position( position.root, [ 0 ] );
return null;
}

/**
Expand Down Expand Up @@ -370,9 +374,10 @@ export default class Document {

// Find the first position where the selection can be put.
const position = new Position( defaultRoot, [ 0 ] );
const selectionPosition = this.getNearestSelectionPosition( position );
const nearestRange = this.getNearestSelectionRange( position );

return new Range( selectionPosition );
// If valid selection range is not found - return range collapsed at the beginning of the root.
return nearestRange || new Range( position );
}

/**
Expand Down Expand Up @@ -447,3 +452,42 @@ function validateTextNodePosition( rangeBoundary ) {

return true;
}

// Generator function returning values from provided walkers, switching between them at each iteration. If only one walker
// is provided it will return data only from that walker.
//
// @param {module:engine/module/treewalker~TreeWalker} [backward] Walker iterating in backward direction.
// @param {module:engine/module/treewalker~TreeWalker} [forward] Walker iterating in forward direction.
// @returns {Iterable.<Object>} Object returned at each iteration contains `value` and `walker` (informing which walker returned
// given value) fields.
function *combineWalkers( backward, forward ) {
let done = false;

while ( !done ) {
done = true;

if ( backward ) {
const step = backward.next();

if ( !step.done ) {
done = false;
yield {
walker: backward,
value: step.value
};
}
}

if ( forward ) {
const step = forward.next();

if ( !step.done ) {
done = false;
yield {
walker: forward,
value: step.value
};
}
}
}
}
9 changes: 7 additions & 2 deletions src/model/liveselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,9 +629,14 @@ export default class LiveSelection extends Selection {
newPath[ newPath.length - 1 ] -= gyPath[ 1 ];

const newPosition = new Position( oldRange.root, newPath );
const selectionPosition = this._document.getNearestSelectionPosition( newPosition );
const newRange = this._prepareRange( new Range( selectionPosition ) );
let selectionRange = this._document.getNearestSelectionRange( newPosition );

// If nearest valid selection range cannot be found - use one created at root beginning.
if ( !selectionRange ) {
selectionRange = new Range( new Position( newPosition.root, [ 0 ] ) );
}

const newRange = this._prepareRange( selectionRange );
const index = this._ranges.indexOf( gyRange );
this._ranges.splice( index, 1, newRange );

Expand Down
1 change: 1 addition & 0 deletions tests/controller/datacontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ describe( 'DataController', () => {
const spy = sinon.spy();
const content = new ModelDocumentFragment( [ new ModelText( 'x' ) ] );
const batch = modelDocument.batch();
schema.allow( { name: '$text', inside: '$root' } );

data.on( 'insertContent', spy );

Expand Down
Loading