jenkins-bot has submitted this change and it was merged. Change subject: Refactor the code that determine insertion annotations ......................................................................
Refactor the code that determine insertion annotations Also use the cursor position with respect to annotation tags, if available Change-Id: I5f7e97e279fa1a6f3c65b976a7e0da5fb9dd7c9c --- M src/dm/lineardata/ve.dm.ElementLinearData.js M tests/dm/lineardata/ve.dm.ElementLinearData.test.js 2 files changed, 35 insertions(+), 32 deletions(-) Approvals: Esanders: Looks good to me, approved jenkins-bot: Verified diff --git a/src/dm/lineardata/ve.dm.ElementLinearData.js b/src/dm/lineardata/ve.dm.ElementLinearData.js index f11f492..868de3f 100644 --- a/src/dm/lineardata/ve.dm.ElementLinearData.js +++ b/src/dm/lineardata/ve.dm.ElementLinearData.js @@ -539,44 +539,42 @@ * see https://phabricator.wikimedia.org/T113869 . * * @param {ve.Range} range The range into which text would be inserted + * @param {boolean} [startAfterAnnotations] Use annotations after cursor if collapsed * @return {ve.dm.AnnotationSet} The insertion annotations that should apply */ -ve.dm.ElementLinearData.prototype.getInsertionAnnotationsFromRange = function ( range ) { - var left, right, leftAnnotations, rightAnnotations; +ve.dm.ElementLinearData.prototype.getInsertionAnnotationsFromRange = function ( range, startAfterAnnotations ) { + var start, startAnnotations, afterAnnotations; - if ( range.isCollapsed() ) { - // Get annotations from either side of the cursor - left = Math.max( 0, range.start - 1 ); - if ( !this.isContentOffset( left ) ) { - left = -1; - } - right = Math.max( 0, range.start ); - if ( !this.isContentOffset( right ) ) { - right = -1; - } + // Get position for start annotations + if ( range.isCollapsed() && !startAfterAnnotations ) { + // Use the position just before the cursor + start = Math.max( 0, range.start - 1 ); } else { - // Get annotations from the first character of the range - // TODO: The use of getNearestContentOffset no longer makes much sense here - left = this.getNearestContentOffset( range.start ); - right = this.getNearestContentOffset( range.end ); - } - if ( left === -1 ) { - // No content offset to our left, use empty set - return new ve.dm.AnnotationSet( this.getStore() ); + // If uncollapsed, use the first character of the selection + // If collapsed, use the first position after the cursor + start = range.start; } - // Include each annotation on the left that either continues on the right - // or should get added to appended content - leftAnnotations = this.getAnnotationsFromOffset( left ); - if ( right === -1 ) { - // TODO: "return leftAnnotations" would match prior behaviour. Was that a mistake? - rightAnnotations = new ve.dm.AnnotationSet( this.getStore() ); + // Get startAnnotations: the annotations that apply at the selection start + if ( this.isContentOffset( start ) ) { + startAnnotations = this.getAnnotationsFromOffset( start ); } else { - rightAnnotations = this.getAnnotationsFromOffset( right ); + startAnnotations = new ve.dm.AnnotationSet( this.getStore() ); } - return leftAnnotations.filter( function ( annotation ) { + + // Get afterAnnotations: the annotations that apply straight after the selection + if ( this.isContentOffset( range.end ) ) { + afterAnnotations = this.getAnnotationsFromOffset( range.end ); + } else { + // Use the empty set + afterAnnotations = new ve.dm.AnnotationSet( this.getStore() ); + } + + // Return those startAnnotations that either continue in afterAnnotations or + // should get added to appended content + return startAnnotations.filter( function ( annotation ) { return annotation.constructor.static.applyToAppendedContent || - rightAnnotations.containsComparable( annotation ); + afterAnnotations.containsComparable( annotation ); } ); }; diff --git a/tests/dm/lineardata/ve.dm.ElementLinearData.test.js b/tests/dm/lineardata/ve.dm.ElementLinearData.test.js index e2d8d52..9ec66a7 100644 --- a/tests/dm/lineardata/ve.dm.ElementLinearData.test.js +++ b/tests/dm/lineardata/ve.dm.ElementLinearData.test.js @@ -391,12 +391,16 @@ { range: [ 1, 1 ], expected: [], msg: 'plain start at block start' }, { range: [ 2, 2 ], expected: [], msg: 'plain interior' }, { range: [ 3, 3 ], expected: [], msg: 'plain end before u' }, - { range: [ 4, 4 ], expected: [ u ], msg: 'u start' }, - { range: [ 5, 5 ], expected: [ u ], msg: 'u interior' }, + { range: [ 3, 3 ], startAfterAnnotations: true, expected: [ u ], msg: 'u start' }, + { range: [ 4, 4 ], expected: [ u ], msg: 'u interior' }, + { range: [ 5, 5 ], expected: [ u ], msg: 'u end' }, + { range: [ 5, 5 ], startAfterAnnotations: true, expected: [], msg: 'after u' }, { range: [ 6, 6 ], expected: [], msg: 'plain start after u' }, { range: [ 7, 7 ], expected: [], msg: 'plain end at block end' }, { range: [ 9, 9 ], expected: [], msg: 'block start before u' }, + { range: [ 9, 9 ], startAfterAnnotations: true, expected: [ u ], msg: 'u start at block start' }, { range: [ 10, 10 ], expected: [ u ], msg: 'u end before block end' }, + { range: [ 10, 10 ], startAfterAnnotations: true, expected: [], msg: 'after u before block end' }, { range: [ 12, 12 ], expected: [], msg: 'empty block' }, { range: [ 2, 3 ], expected: [], msg: 'forward to u start' }, { range: [ 3, 2 ], expected: [], msg: 'backward to u start' }, @@ -414,7 +418,8 @@ ).data; tests.forEach( function ( test ) { var observed = linearData.getInsertionAnnotationsFromRange( - new ve.Range( test.range[ 0 ], test.range[ 1 ] ) + new ve.Range( test.range[ 0 ], test.range[ 1 ] ), + test.startAfterAnnotations ).get().map( function ( annotation ) { return { type: annotation.element.type, -- To view, visit https://gerrit.wikimedia.org/r/242407 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5f7e97e279fa1a6f3c65b976a7e0da5fb9dd7c9c Gerrit-PatchSet: 1 Gerrit-Project: VisualEditor/VisualEditor Gerrit-Branch: master Gerrit-Owner: Divec <da...@troi.org> Gerrit-Reviewer: Esanders <esand...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
