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

T/1555: Expose Position, Range and Selection factories. #1585

Merged
merged 32 commits into from
Nov 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2cf767c
WiP: Introduce Position, Range & Selection APIs on model & model/writer.
jodator Sep 26, 2018
17626ed
Changed: Make factory methods of model Position protected.
jodator Oct 10, 2018
a9c850d
Changed: Use Position._createAt( position ) instead of Position._crea…
jodator Oct 10, 2018
210bac5
Changed: Make Range factory methods protected. Remove Range.createFro…
jodator Oct 11, 2018
1dc6154
Changed: Make view Position & Range factory methods protected.
jodator Oct 15, 2018
dbe76e4
Other: Introduce ViewPosition.getWalker().
jodator Oct 15, 2018
a3d34e7
Docs: Update documentation on Position/Range/Selection factory methods.
jodator Oct 22, 2018
4ade501
Other: Remove unused Range.createCollapsedAt().
jodator Oct 23, 2018
5edb9e6
Docs: Update protected model factory methods in Position & Range.
jodator Oct 23, 2018
bf7c01f
Docs: Remove Position._createAt() references from docs.
jodator Oct 23, 2018
ed0605b
Update model/view position docs and tests.
jodator Oct 24, 2018
7c49b92
Update model/view range docs and tests.
jodator Oct 25, 2018
891bb37
Update model/view selection docs and tests.
jodator Oct 25, 2018
ceac18a
Create stub tests for model Position/Writer/Selection factory methods.
jodator Oct 25, 2018
237538a
Create stub tests for writer's Position/Writer/Selection factory meth…
jodator Oct 25, 2018
0956136
Create stub tests for downcast writer's Position/Writer/Selection fac…
jodator Oct 25, 2018
0e424d3
Create stub tests for views's Position/Writer/Selection factory methods.
jodator Oct 25, 2018
2b6e101
Add tests for editableElement.is( 'editableElement' ).
jodator Oct 25, 2018
8e0fc9e
Add tests for schema.createContext().
jodator Oct 26, 2018
6c1e160
Merge branch 'master' into t/1555
jodator Oct 26, 2018
dcd33cd
Fix code after merging master.
jodator Oct 26, 2018
5ee057b
Use writer to create Ranges/Positions in private API.
jodator Oct 26, 2018
d0e54f1
Remove ViewRange import from downcast-converts.
jodator Oct 26, 2018
6714db2
Merge branch 'master' into t/1555
jodator Oct 29, 2018
d73756f
Review writerSetSelection() API calls with single position.
jodator Oct 29, 2018
98422ea
Merge branch 'master' into t/1555
jodator Oct 29, 2018
d3644a9
Add model/view Position.clone() method.
jodator Oct 30, 2018
f7b4ac8
Add model/view Range.clone() method.
jodator Oct 30, 2018
e6b1f99
Remove new DocumentSelection from docs.
jodator Oct 30, 2018
a40fc17
Add Position/Range/Selection factory methods to UpcastWriter.
jodator Oct 30, 2018
6c14fba
Fix document selection examples.
jodator Oct 30, 2018
15dd8c5
Docs: A lot of corrections all across the docs.
Reinmar Nov 1, 2018
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
6 changes: 3 additions & 3 deletions src/controller/datacontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export default class DataController {
this.mapper.clearBindings();

// First, convert elements.
const modelRange = ModelRange.createIn( modelElementOrFragment );
const modelRange = ModelRange._createIn( modelElementOrFragment );

const viewDocumentFragment = new ViewDocumentFragment();

Expand Down Expand Up @@ -227,7 +227,7 @@ export default class DataController {
writer.setSelection( null );
writer.removeSelectionAttribute( this.model.document.selection.getAttributeKeys() );

writer.remove( ModelRange.createIn( modelRoot ) );
writer.remove( writer.createRangeIn( modelRoot ) );
writer.insert( this.parse( data, modelRoot ), modelRoot, 0 );
} );
}
Expand Down Expand Up @@ -297,7 +297,7 @@ function _getMarkersRelativeToElement( element ) {
return [];
}

const elementRange = ModelRange.createIn( element );
const elementRange = ModelRange._createIn( element );

for ( const marker of doc.model.markers ) {
const intersection = elementRange.getIntersection( marker.getRange() );
Expand Down
11 changes: 5 additions & 6 deletions src/conversion/downcast-converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import ModelSelection from '../model/selection';
import ModelElement from '../model/element';

import ViewAttributeElement from '../view/attributeelement';
import ViewRange from '../view/range';
import DocumentSelection from '../model/documentselection';

import { cloneDeep } from 'lodash-es';
Expand Down Expand Up @@ -533,14 +532,14 @@ export function remove() {
const modelEnd = data.position.getShiftedBy( data.length );
const viewEnd = conversionApi.mapper.toViewPosition( modelEnd, { isPhantom: true } );

const viewRange = new ViewRange( viewStart, viewEnd );
const viewRange = conversionApi.writer.createRange( viewStart, viewEnd );

// Trim the range to remove in case some UI elements are on the view range boundaries.
const removed = conversionApi.writer.remove( viewRange.getTrimmed() );

// After the range is removed, unbind all view elements from the model.
// Range inside view document fragment is used to unbind deeply.
for ( const child of ViewRange.createIn( removed ).getItems() ) {
for ( const child of conversionApi.writer.createRangeIn( removed ).getItems() ) {
conversionApi.mapper.unbindViewElement( child );
}
};
Expand Down Expand Up @@ -626,7 +625,7 @@ export function removeUIElement() {
conversionApi.mapper.unbindElementsFromMarkerName( data.markerName );

for ( const element of elements ) {
conversionApi.writer.clear( ViewRange.createOn( element ), element );
conversionApi.writer.clear( conversionApi.writer.createRangeOn( element ), element );
}

conversionApi.writer.clearClonedElementsGroup( data.markerName );
Expand Down Expand Up @@ -901,7 +900,7 @@ export function highlightElement( highlightDescriptor ) {
conversionApi.consumable.consume( data.item, evt.name );

// Consume all children nodes.
for ( const value of ModelRange.createIn( data.item ) ) {
for ( const value of ModelRange._createIn( data.item ) ) {
conversionApi.consumable.consume( value.item, evt.name );
}

Expand Down Expand Up @@ -963,7 +962,7 @@ export function removeHighlight( highlightDescriptor ) {

for ( const element of elements ) {
if ( element.is( 'attributeElement' ) ) {
conversionApi.writer.unwrap( ViewRange.createOn( element ), viewHighlightElement );
conversionApi.writer.unwrap( conversionApi.writer.createRangeOn( element ), viewHighlightElement );
} else {
// if element.is( 'containerElement' ).
element.getCustomProperty( 'removeHighlight' )( element, descriptor.id, conversionApi.writer );
Expand Down
8 changes: 4 additions & 4 deletions src/conversion/downcastdispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export default class DowncastDispatcher {
// Convert changes that happened on model tree.
for ( const entry of differ.getChanges() ) {
if ( entry.type == 'insert' ) {
this.convertInsert( Range.createFromPositionAndShift( entry.position, entry.length ), writer );
this.convertInsert( Range._createFromPositionAndShift( entry.position, entry.length ), writer );
} else if ( entry.type == 'remove' ) {
this.convertRemove( entry.position, entry.length, entry.name, writer );
} else {
Expand Down Expand Up @@ -166,7 +166,7 @@ export default class DowncastDispatcher {
// Fire a separate insert event for each node and text fragment contained in the range.
for ( const value of range ) {
const item = value.item;
const itemRange = Range.createFromPositionAndShift( value.previousPosition, value.length );
const itemRange = Range._createFromPositionAndShift( value.previousPosition, value.length );
const data = {
item,
range: itemRange
Expand Down Expand Up @@ -226,7 +226,7 @@ export default class DowncastDispatcher {
// Create a separate attribute event for each node in the range.
for ( const value of range ) {
const item = value.item;
const itemRange = Range.createFromPositionAndShift( value.previousPosition, value.length );
const itemRange = Range._createFromPositionAndShift( value.previousPosition, value.length );
const data = {
item,
range: itemRange,
Expand Down Expand Up @@ -343,7 +343,7 @@ export default class DowncastDispatcher {
continue;
}

const data = { item, range: Range.createOn( item ), markerName, markerRange };
const data = { item, range: Range._createOn( item ), markerName, markerRange };

this.fire( eventName, data, this.conversionApi );
}
Expand Down
2 changes: 1 addition & 1 deletion src/conversion/mapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export default class Mapper {

const modelOffset = this._toModelOffset( data.viewPosition.parent, data.viewPosition.offset, viewBlock );

data.modelPosition = ModelPosition.createFromParentAndOffset( modelParent, modelOffset );
data.modelPosition = ModelPosition._createAt( modelParent, modelOffset );
}, { priority: 'low' } );
}

Expand Down
11 changes: 5 additions & 6 deletions src/conversion/upcast-converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import Matcher from '../view/matcher';

import ModelRange from '../model/range';
import ModelPosition from '../model/position';

import { cloneDeep } from 'lodash-es';

Expand Down Expand Up @@ -358,20 +357,20 @@ function _prepareToElementConverter( config ) {
conversionApi.writer.insert( modelElement, splitResult.position );

// Convert children and insert to element.
const childrenResult = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelElement, 0 ) );
const childrenResult = conversionApi.convertChildren( data.viewItem, conversionApi.writer.createPositionAt( modelElement, 0 ) );

// Consume appropriate value from consumable values list.
conversionApi.consumable.consume( data.viewItem, match.match );

// Set conversion result range.
data.modelRange = new ModelRange(
// Range should start before inserted element
ModelPosition.createBefore( modelElement ),
conversionApi.writer.createPositionBefore( modelElement ),
// Should end after but we need to take into consideration that children could split our
// element, so we need to move range after parent of the last converted child.
// before: <allowed>[]</allowed>
// after: <allowed>[<converted><child></child></converted><child></child><converted>]</converted></allowed>
ModelPosition.createAfter( childrenResult.modelCursor.parent )
conversionApi.writer.createPositionAfter( childrenResult.modelCursor.parent )
);

// Now we need to check where the modelCursor should be.
Expand All @@ -380,7 +379,7 @@ function _prepareToElementConverter( config ) {
// before: <allowed><notAllowed>[]</notAllowed></allowed>
// after: <allowed><notAllowed></notAllowed><converted></converted><notAllowed>[]</notAllowed></allowed>
if ( splitResult.cursorParent ) {
data.modelCursor = ModelPosition.createAt( splitResult.cursorParent, 0 );
data.modelCursor = conversionApi.writer.createPositionAt( splitResult.cursorParent, 0 );

// Otherwise just continue after inserted element.
} else {
Expand Down Expand Up @@ -602,7 +601,7 @@ export function convertText() {

conversionApi.writer.insert( text, data.modelCursor );

data.modelRange = ModelRange.createFromPositionAndShift( data.modelCursor, text.offsetSize );
data.modelRange = ModelRange._createFromPositionAndShift( data.modelCursor, text.offsetSize );
data.modelCursor = data.modelRange.end;
}
}
Expand Down
20 changes: 13 additions & 7 deletions src/conversion/upcastdispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,16 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
* conversionApi.writer.insert( paragraph, splitResult.position );
*
* // Convert children to paragraph.
* const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph, 0 ) );
* const { modelRange } = conversionApi.convertChildren(
* data.viewItem,
* conversionApi.writer.createPositionAt( paragraph, 0 )
* );
*
* // Set as conversion result, attribute converters may use this property.
* data.modelRange = new Range( Position.createBefore( paragraph ), modelRange.end );
* data.modelRange = conversionApi.writer.createRange(
* conversionApi.writer.createPositionBefore( paragraph ),
* modelRange.end
* );
*
* // Continue conversion inside paragraph.
* data.modelCursor = data.modelRange.end;
Expand Down Expand Up @@ -372,7 +378,7 @@ function extractMarkersFromModelFragment( modelItem, writer ) {
const markers = new Map();

// Create ModelTreeWalker.
const range = ModelRange.createIn( modelItem ).getItems();
const range = ModelRange._createIn( modelItem ).getItems();

// Walk through DocumentFragment and collect marker elements.
for ( const item of range ) {
Expand All @@ -385,14 +391,14 @@ function extractMarkersFromModelFragment( modelItem, writer ) {
// Walk through collected marker elements store its path and remove its from the DocumentFragment.
for ( const markerElement of markerElements ) {
const markerName = markerElement.getAttribute( 'data-name' );
const currentPosition = ModelPosition.createBefore( markerElement );
const currentPosition = writer.createPositionBefore( markerElement );

// When marker of given name is not stored it means that we have found the beginning of the range.
if ( !markers.has( markerName ) ) {
markers.set( markerName, new ModelRange( ModelPosition.createFromPosition( currentPosition ) ) );
markers.set( markerName, new ModelRange( currentPosition.clone() ) );
// Otherwise is means that we have found end of the marker range.
} else {
markers.get( markerName ).end = ModelPosition.createFromPosition( currentPosition );
markers.get( markerName ).end = currentPosition.clone();
}

// Remove marker element from DocumentFragment.
Expand All @@ -419,7 +425,7 @@ function createContextTree( contextDefinition, writer ) {
writer.append( current, position );
}

position = ModelPosition.createAt( current, 0 );
position = ModelPosition._createAt( current, 0 );
}

return position;
Expand Down
4 changes: 2 additions & 2 deletions src/dev-utils/enableenginedebug.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ function enableLoggingTools() {
} );

sandbox.mock( DetachOperation.prototype, 'toString', function() {
const range = ModelRange.createFromPositionAndShift( this.sourcePosition, this.howMany );
const range = ModelRange._createFromPositionAndShift( this.sourcePosition, this.howMany );
const nodes = Array.from( range.getItems() );
const nodeString = nodes.length > 1 ? `[ ${ nodes.length } ]` : nodes[ 0 ];

Expand All @@ -330,7 +330,7 @@ function enableLoggingTools() {
} );

sandbox.mock( MoveOperation.prototype, 'toString', function() {
const range = ModelRange.createFromPositionAndShift( this.sourcePosition, this.howMany );
const range = ModelRange._createFromPositionAndShift( this.sourcePosition, this.howMany );

return getClassName( this ) + `( ${ this.baseVersion } ): ${ range } -> ${ this.targetPosition }`;
} );
Expand Down
16 changes: 8 additions & 8 deletions src/dev-utils/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function setData( model, data, options = {} ) {

model.change( writer => {
// Replace existing model in document by new one.
writer.remove( ModelRange.createIn( modelRoot ) );
writer.remove( writer.createRangeIn( modelRoot ) );
writer.insert( modelDocumentFragment, modelRoot );

// Clean up previous document selection.
Expand Down Expand Up @@ -169,16 +169,16 @@ export function stringify( node, selectionOrPositionOrRange = null, markers = nu

// Create a range witch wraps passed node.
if ( node instanceof RootElement || node instanceof ModelDocumentFragment ) {
range = ModelRange.createIn( node );
range = model.createRangeIn( node );
} else {
// Node is detached - create new document fragment.
if ( !node.parent ) {
const fragment = new ModelDocumentFragment( node );
range = ModelRange.createIn( fragment );
range = model.createRangeIn( fragment );
} else {
range = new ModelRange(
ModelPosition.createBefore( node ),
ModelPosition.createAfter( node )
model.createPositionBefore( node ),
model.createPositionAfter( node )
);
}
}
Expand Down Expand Up @@ -391,9 +391,9 @@ function convertToModelElement() {

conversionApi.mapper.bindElements( element, data.viewItem );

conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( element, 0 ) );
conversionApi.convertChildren( data.viewItem, ModelPosition._createAt( element, 0 ) );

data.modelRange = ModelRange.createOn( element );
data.modelRange = ModelRange._createOn( element );
data.modelCursor = data.modelRange.end;

evt.stop();
Expand All @@ -420,7 +420,7 @@ function convertToModelText( withAttributes = false ) {

conversionApi.writer.insert( node, data.modelCursor );

data.modelRange = ModelRange.createFromPositionAndShift( data.modelCursor, node.offsetSize );
data.modelRange = ModelRange._createFromPositionAndShift( data.modelCursor, node.offsetSize );
data.modelCursor = data.modelRange.end;

evt.stop();
Expand Down
10 changes: 5 additions & 5 deletions src/dev-utils/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ setData._parse = parse;
* const b = new Element( 'b', null, text );
* const p = new Element( 'p', null, b );
* const selection = new Selection(
* Range.createFromParentsAndOffsets( p, 0, p, 1 )
* Range._createFromParentsAndOffsets( p, 0, p, 1 )
* );
*
* stringify( p, selection ); // '<p>[<b>foobar</b>]</p>'
Expand All @@ -148,7 +148,7 @@ setData._parse = parse;
* const text = new Text( 'foobar' );
* const b = new Element( 'b', null, text );
* const p = new Element( 'p', null, b );
* const selection = new Selection( Range.createFromParentsAndOffsets( text, 1, text, 5 ) );
* const selection = new Selection( Range._createFromParentsAndOffsets( text, 1, text, 5 ) );
*
* stringify( p, selection ); // '<p><b>f{ooba}r</b></p>'
*
Expand All @@ -161,8 +161,8 @@ setData._parse = parse;
*
* const text = new Text( 'foobar' );
* const selection = new Selection( [
* Range.createFromParentsAndOffsets( text, 0, text, 1 ) ),
* Range.createFromParentsAndOffsets( text, 3, text, 5 ) )
* Range._createFromParentsAndOffsets( text, 0, text, 1 ) ),
* Range._createFromParentsAndOffsets( text, 3, text, 5 ) )
* ] );
*
* stringify( text, selection ); // '{f}oo{ba}r'
Expand All @@ -173,7 +173,7 @@ setData._parse = parse;
* be converted to a selection containing one range collapsed at this position.
*
* const text = new Text( 'foobar' );
* const range = Range.createFromParentsAndOffsets( text, 0, text, 1 );
* const range = Range._createFromParentsAndOffsets( text, 0, text, 1 );
* const position = new Position( text, 3 );
*
* stringify( text, range ); // '{f}oobar'
Expand Down
Loading