Browse Source

Added: Changing the style on collapsed selection changes the entire highlighter.

Maciej Gołaszewski 8 years ago
parent
commit
1bc3577bfa

+ 47 - 0
packages/ckeditor5-highlight/src/findattributerange.js

@@ -0,0 +1,47 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module highlight/findattributerange
+ */
+
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
+import Position from '@ckeditor/ckeditor5-engine/src/model/position';
+
+/**
+ * Walks backward and forward from the start position, node by node, as long as they have the same `attribute` value and return
+ * a {@link module:engine/model/range~Range Range} with the found attribute.
+ *
+ * @param {module:engine/model/position~Position} position The start position.
+ * @param {String} attribute The `attribute` key to search.
+ * @param {String} value The `attribute` value.
+ * @returns {module:engine/model/range~Range} The attribute range.
+ */
+export default function findAttributeRange( position, attribute, value ) {
+	return new Range( _findBound( position, attribute, value, true ), _findBound( position, attribute, value, false ) );
+}
+
+// Walks forward or backward (depends on the `lookBack` flag), node by node, as long as they have the same `linkHref` attribute value
+// and returns a position just before or after (depends on the `lookBack` flag) the last matched node.
+//
+// @param {module:engine/model/position~Position} position The start position.
+// @param {String} attribute The `attribute` key to search.
+// @param {String} value The `attribute` value.
+// @param {Boolean} lookBack Whether the walk direction is forward (`false`) or backward (`true`).
+// @returns {module:engine/model/position~Position} The position just before the last matched node.
+function _findBound( position, attribute, value, lookBack ) {
+	// Get node before or after position (depends on `lookBack` flag).
+	// When position is inside text node then start searching from text node.
+	let node = position.textNode || ( lookBack ? position.nodeBefore : position.nodeAfter );
+
+	let lastNode = null;
+
+	while ( node && node.getAttribute( attribute ) == value ) {
+		lastNode = node;
+		node = lookBack ? node.previousSibling : node.nextSibling;
+	}
+
+	return lastNode ? Position.createAt( lastNode, lookBack ? 'before' : 'after' ) : position;
+}

+ 24 - 6
packages/ckeditor5-highlight/src/highlightcommand.js

@@ -8,6 +8,7 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
+import findAttributeRange from './findattributerange';
 
 /**
  * The highlight command. It is used by the {@link module:highlight/highlightediting~HighlightEditing highlight feature}
@@ -48,8 +49,8 @@ export default class HighlightCommand extends Command {
 		const document = model.document;
 		const selection = document.selection;
 
-		// Do not apply highlight on collapsed selection.
-		if ( selection.isCollapsed ) {
+		// Do not apply highlight on collapsed selection when not inside existing highlight.
+		if ( selection.isCollapsed && !this.value ) {
 			return;
 		}
 
@@ -58,11 +59,28 @@ export default class HighlightCommand extends Command {
 		model.change( writer => {
 			const ranges = model.schema.getValidRanges( selection.getRanges(), 'highlight' );
 
-			for ( const range of ranges ) {
-				if ( value ) {
-					writer.setAttribute( 'highlight', value, range );
+			if ( selection.isCollapsed ) {
+				const position = selection.getFirstPosition();
+
+				// When selection is inside text with `linkHref` attribute.
+				if ( selection.hasAttribute( 'highlight' ) ) {
+					// Then update `highlight` value.
+					const linkRange = findAttributeRange( position, 'highlight', selection.getAttribute( 'highlight' ) );
+
+					writer.setAttribute( 'highlight', value, linkRange );
+
+					// Create new range wrapping changed link.
+					selection.setRanges( [ linkRange ] );
 				} else {
-					writer.removeAttribute( 'highlight', range );
+					// TODO
+				}
+			} else {
+				for ( const range of ranges ) {
+					if ( value ) {
+						writer.setAttribute( 'highlight', value, range );
+					} else {
+						writer.removeAttribute( 'highlight', range );
+					}
 				}
 			}
 		} );

+ 127 - 0
packages/ckeditor5-highlight/tests/findattributerange.js

@@ -0,0 +1,127 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import findAttributeRange from '../src/findattributerange';
+import Model from '@ckeditor/ckeditor5-engine/src/model/model';
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
+import Position from '@ckeditor/ckeditor5-engine/src/model/position';
+import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+
+describe( 'findAttributeRange', () => {
+	let model, document, root;
+
+	beforeEach( () => {
+		model = new Model();
+		document = model.document;
+		root = document.createRoot();
+		model.schema.extend( '$text', { allowIn: '$root' } );
+		model.schema.register( 'p', { inheritAllFrom: '$block' } );
+	} );
+
+	it( 'should find attribute range searching from the center of the attribute #1', () => {
+		setData( model, '<$text foo="bar">foobar</$text>' );
+
+		const startPosition = new Position( root, [ 3 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 0, root, 6 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range searching from the center of the attribute #2', () => {
+		setData( model, 'abc <$text foo="bar">foobar</$text> abc' );
+
+		const startPosition = new Position( root, [ 7 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 4, root, 10 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range searching from the beginning of the attribute #1', () => {
+		setData( model, '<$text foo="bar">foobar</$text>' );
+
+		const startPosition = new Position( root, [ 0 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 0, root, 6 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range searching from the beginning of the attribute #2', () => {
+		setData( model, 'abc <$text foo="bar">foobar</$text> abc' );
+
+		const startPosition = new Position( root, [ 4 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 4, root, 10 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range searching from the end of the attribute #1', () => {
+		setData( model, '<$text foo="bar">foobar</$text>' );
+
+		const startPosition = new Position( root, [ 6 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 0, root, 6 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range searching from the end of the attribute #2', () => {
+		setData( model, 'abc <$text foo="bar">foobar</$text> abc' );
+
+		const startPosition = new Position( root, [ 10 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 4, root, 10 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range when attribute stick to other attribute searching from the center of the attribute', () => {
+		setData( model, '<$text foo="other">abc</$text><$text foo="bar">foobar</$text><$text foo="other">abc</$text>' );
+
+		const startPosition = new Position( root, [ 6 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 3, root, 9 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range when attribute stick to other attribute searching from the beginning of the attribute', () => {
+		setData( model, '<$text foo="other">abc</$text><$text foo="bar">foobar</$text><$text foo="other">abc</$text>' );
+
+		const startPosition = new Position( root, [ 3 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 3, root, 9 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range when attribute stick to other attribute searching from the end of the attribute', () => {
+		setData( model, '<$text foo="other">abc</$text><$text foo="bar">foobar</$text><$text foo="other">abc</$text>' );
+
+		const startPosition = new Position( root, [ 9 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 3, root, 9 ) ) ).to.true;
+	} );
+
+	it( 'should find attribute range only inside current parent', () => {
+		setData(
+			model,
+			'<p><$text foo="bar">foobar</$text></p>' +
+			'<p><$text foo="bar">foobar</$text></p>' +
+			'<p><$text foo="bar">foobar</$text></p>'
+		);
+
+		const startPosition = new Position( root, [ 1, 3 ] );
+		const result = findAttributeRange( startPosition, 'foo', 'bar' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 6 ] ) ) ) ).to.true;
+	} );
+} );

+ 4 - 4
packages/ckeditor5-highlight/tests/highlightcommand.js

@@ -115,16 +115,16 @@ describe( 'HighlightCommand', () => {
 			expect( command.value ).to.be.undefined;
 		} );
 
-		it( 'should do nothing on collapsed range', () => {
+		it( 'should change entire highlight on collapsed range when inside highlighted text', () => {
 			setData( model, '<paragraph>abc<$text highlight="marker">foo[]bar</$text>xyz</paragraph>' );
 
 			expect( command.value ).to.equal( 'marker' );
 
-			command.execute();
+			command.execute( { value: 'greenMarker' } );
 
-			expect( getData( model ) ).to.equal( '<paragraph>abc<$text highlight="marker">foo[]bar</$text>xyz</paragraph>' );
+			expect( getData( model ) ).to.equal( '<paragraph>abc[<$text highlight="greenMarker">foobar</$text>]xyz</paragraph>' );
 
-			expect( command.value ).to.equal( 'marker' );
+			expect( command.value ).to.equal( 'greenMarker' );
 		} );
 	} );
 } );