Sfoglia il codice sorgente

Improving fake selection API.

Maciej Bukowski 7 anni fa
parent
commit
66254f1232

+ 1 - 3
packages/ckeditor5-engine/src/controller/editingcontroller.js

@@ -20,8 +20,7 @@ import { convertSelectionChange } from '../conversion/upcast-selection-converter
 import {
 	convertRangeSelection,
 	convertCollapsedSelection,
-	clearAttributes,
-	clearFakeSelection
+	clearAttributes
 } from '../conversion/downcast-selection-converters';
 
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
@@ -95,7 +94,6 @@ export default class EditingController {
 
 		// Attach default model selection converters.
 		this.downcastDispatcher.on( 'selection', clearAttributes(), { priority: 'low' } );
-		this.downcastDispatcher.on( 'selection', clearFakeSelection(), { priority: 'low' } );
 		this.downcastDispatcher.on( 'selection', convertRangeSelection(), { priority: 'low' } );
 		this.downcastDispatcher.on( 'selection', convertCollapsedSelection(), { priority: 'low' } );
 

+ 0 - 8
packages/ckeditor5-engine/src/conversion/downcast-selection-converters.js

@@ -127,11 +127,3 @@ export function clearAttributes() {
 		viewWriter.setSelection( null );
 	};
 }
-
-/**
- * Function factory, creates a converter that clears fake selection marking after the previous
- * {@link module:engine/model/selection~Selection model selection} conversion.
- */
-export function clearFakeSelection() {
-	return ( evt, data, conversionApi ) => conversionApi.writer.setFakeSelection( false );
-}

+ 1 - 2
packages/ckeditor5-engine/src/view/observer/fakeselectionobserver.js

@@ -82,8 +82,7 @@ export default class FakeSelectionObserver extends Observer {
 	 */
 	_handleSelectionMove( keyCode ) {
 		const selection = this.document.selection;
-		const newSelection = new ViewSelection( selection );
-		newSelection._setFake( false );
+		const newSelection = new ViewSelection( selection.getRanges(), { backward: selection.isBackward, fake: false } );
 
 		// Left or up arrow pressed - move selection to start.
 		if ( keyCode == keyCodes.arrowleft || keyCode == keyCodes.arrowup ) {

+ 24 - 17
packages/ckeditor5-engine/src/view/selection.js

@@ -69,10 +69,15 @@ export default class Selection {
 	 *
 	 * @param {module:engine/view/selection~Selection|module:engine/view/position~Position|
 	 * Iterable.<module:engine/view/range~Range>|module:engine/view/range~Range|module:engine/view/item~Item|null} [selectable=null]
-	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset]
-	 * @param {Boolean} [optionsOrPlaceOrOffset.backward]
-	 * @param {Object} [options]
-	 * @param {Boolean} [options.backward]
+	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset] Offset or place when selectable is an `Item`.
+	 * Options otherwise.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.backward] Sets this selection instance to be backward.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.fake] Sets this selection instance to be marked as `fake`.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.label] Label for the fake selection.
+	 * @param {Object} [options] Options when selectable is an `Item`.
+	 * @param {Boolean} [options.backward] Sets this selection instance to be backward.
+	 * @param {Boolean} [options.fake] Sets this selection instance to be marked as `fake`.
+	 * @param {String} [options.label] Label for the fake selection.
 	 */
 	constructor( selectable = null, optionsOrPlaceOrOffset, options ) {
 		/**
@@ -437,12 +442,13 @@ export default class Selection {
 	 * @protected
 	 * @param {module:engine/view/selection~Selection|module:engine/view/position~Position|
 	 * Iterable.<module:engine/view/range~Range>|module:engine/view/range~Range|module:engine/view/item~Item|null} selectable
-	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset]
-	 * @param {Boolean} [optionsOrPlaceOrOffset.backward] Sets this selection as backward.
+	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset] Offset or place when selectable is an `Item`.
+	 * Options otherwise.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.backward] Sets this selection instance to be backward.
 	 * @param {Boolean} [optionsOrPlaceOrOffset.fake] Sets this selection instance to be marked as `fake`.
-	 * @param {String} [optionsOrPlaceOrOffset.label] Label for the fake selection.
-	 * @param {Object} [options]
-	 * @param {Boolean} [options.backward] Sets this selection as backward.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.label] Label for the fake selection.
+	 * @param {Object} [options] Options when selectable is an `Item`.
+	 * @param {Boolean} [options.backward] Sets this selection instance to be backward.
 	 * @param {Boolean} [options.fake] Sets this selection instance to be marked as `fake`.
 	 * @param {String} [options.label] Label for the fake selection.
 	 */
@@ -463,20 +469,21 @@ export default class Selection {
 			const backward = !!options && !!options.backward;
 			let range;
 
-			if ( optionsOrPlaceOrOffset == 'in' ) {
-				range = Range.createIn( selectable );
-			} else if ( optionsOrPlaceOrOffset == 'on' ) {
-				range = Range.createOn( selectable );
-			} else if ( optionsOrPlaceOrOffset !== undefined ) {
-				range = Range.createCollapsedAt( selectable, optionsOrPlaceOrOffset );
-			} else {
+			if ( optionsOrPlaceOrOffset === undefined ) {
 				/**
 				 * Required second parameter when setting selection to node.
 				 *
 				 * @error view-selection-setTo-required-second-parameter
 				 */
 				throw new CKEditorError(
-					'view-selection-setTo-required-second-parameter: Required second parameter when setting selection to node.' );
+					'view-selection-setTo-required-second-parameter: Required second parameter when setting selection to node.'
+				);
+			} else if ( optionsOrPlaceOrOffset == 'in' ) {
+				range = Range.createIn( selectable );
+			} else if ( optionsOrPlaceOrOffset == 'on' ) {
+				range = Range.createOn( selectable );
+			} else {
+				range = Range.createCollapsedAt( selectable, optionsOrPlaceOrOffset );
 			}
 
 			this._setRanges( [ range ], backward );

+ 19 - 21
packages/ckeditor5-engine/src/view/writer.js

@@ -36,6 +36,16 @@ export default class Writer {
 	 * {@link module:engine/view/item~Item item}, {@link module:engine/view/range~Range range},
 	 * an iterable of {@link module:engine/view/range~Range ranges} or null.
 	 *
+	 * This method provides option to create a fake selection.
+	 * Fake selection does not render as browser native selection over selected elements and is hidden to the user.
+	 * This way, no native selection UI artifacts are displayed to the user and selection over elements can be
+	 * represented in other way, for example by applying proper CSS class.
+	 *
+	 * Additionally fake's selection label can be provided. It will be used to describe fake selection in DOM (and be
+	 * properly handled by screen readers).
+	 *
+	 * Usage:
+	 *
 	 *		// Sets ranges from the given range.
 	 *		const range = new Range( start, end );
 	 *		writer.setSelection( range, isBackwardSelection );
@@ -69,10 +79,15 @@ export default class Writer {
 	 *
 	 * @param {module:engine/view/selection~Selection|module:engine/view/position~Position|
 	 * Iterable.<module:engine/view/range~Range>|module:engine/view/range~Range|module:engine/view/item~Item|null} selectable
-	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset]
-	 * @param {Boolean} [optionsOrPlaceOrOffset.backward]
-	 * @param {Object} [options]
-	 * @param {Boolean} [options.backward]
+	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset] Offset or place when selectable is an `Item`.
+	 * Options otherwise.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.backward] Sets this selection instance to be backward.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.fake] Sets this selection instance to be marked as `fake`.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.label] Label for the fake selection.
+	 * @param {Object} [options] Options when selectable is an `Item`.
+	 * @param {Boolean} [options.backward] Sets this selection instance to be backward.
+	 * @param {Boolean} [options.fake] Sets this selection instance to be marked as `fake`.
+	 * @param {String} [options.label] Label for the fake selection.
 	 */
 	setSelection( selectable, optionsOrPlaceOrOffset, options ) {
 		this.document.selection._setTo( selectable, optionsOrPlaceOrOffset, options );
@@ -92,23 +107,6 @@ export default class Writer {
 	}
 
 	/**
-	 * Sets {@link module:engine/view/selection~Selection selection's} to be marked as `fake`. A fake selection does
-	 * not render as browser native selection over selected elements and is hidden to the user.
-	 * This way, no native selection UI artifacts are displayed to the user and selection over elements can be
-	 * represented in other way, for example by applying proper CSS class.
-	 *
-	 * Additionally fake's selection label can be provided. It will be used to describe fake selection in DOM (and be
-	 * properly handled by screen readers).
-	 *
-	 * @param {Boolean} [value=true] If set to true selection will be marked as `fake`.
-	 * @param {Object} [options] Additional options.
-	 * @param {String} [options.label=''] Fake selection label.
-	 */
-	setFakeSelection( value, options ) {
-		this.document.selection._setFake( value, options );
-	}
-
-	/**
 	 * Creates a new {@link module:engine/view/text~Text text node}.
 	 *
 	 *		writer.createText( 'foo' );

+ 3 - 5
packages/ckeditor5-engine/tests/conversion/downcast-selection-converters.js

@@ -17,7 +17,6 @@ import {
 	convertRangeSelection,
 	convertCollapsedSelection,
 	clearAttributes,
-	clearFakeSelection
 } from '../../src/conversion/downcast-selection-converters';
 
 import {
@@ -475,14 +474,13 @@ describe( 'downcast-selection-converters', () => {
 				const viewString = stringifyView( viewRoot, viewSelection, { showType: false } );
 				expect( viewString ).to.equal( '<div>f{}oobar</div>' );
 			} );
-		} );
 
-		describe( 'clearFakeSelection', () => {
 			it( 'should clear fake selection', () => {
-				dispatcher.on( 'selection', clearFakeSelection() );
+				const modelRange = ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 1 );
 
 				view.change( writer => {
-					writer.setFakeSelection( true );
+					writer.setSelection( modelRange, { fake: true } );
+
 					dispatcher.convertSelection( docSelection, model.markers, writer );
 				} );
 				expect( viewSelection.isFake ).to.be.false;

+ 1 - 2
packages/ckeditor5-engine/tests/view/manual/fakeselection.js

@@ -41,8 +41,7 @@ viewDocument.on( 'mouseup', ( evt, data ) => {
 		console.log( 'Making selection around the <strong>.' );
 
 		view.change( writer => {
-			writer.setSelection( ViewRange.createOn( viewStrong ) );
-			writer.setFakeSelection( true, { label: 'fake selection over bar' } );
+			writer.setSelection( ViewRange.createOn( viewStrong ), { fake: true, label: 'fake selection over bar' } );
 		} );
 
 		data.preventDefault();

+ 6 - 3
packages/ckeditor5-engine/tests/view/observer/fakeselectionobserver.js

@@ -33,7 +33,7 @@ describe( 'FakeSelectionObserver', () => {
 		root = createViewRoot( viewDocument );
 		view.attachDomRoot( domRoot );
 		observer = view.getObserver( FakeSelectionObserver );
-		viewDocument.selection._setFake();
+		viewDocument.selection._setTo( null, { fake: true } );
 	} );
 
 	afterEach( () => {
@@ -41,7 +41,7 @@ describe( 'FakeSelectionObserver', () => {
 	} );
 
 	it( 'should do nothing if selection is not fake', () => {
-		viewDocument.selection._setFake( false );
+		viewDocument.selection._setTo( null, { fake: true } );
 
 		return checkEventPrevention( keyCodes.arrowleft, false );
 	} );
@@ -200,7 +200,10 @@ describe( 'FakeSelectionObserver', () => {
 	//
 	// @param {Number} keyCode
 	function changeFakeSelectionPressing( keyCode ) {
-		viewDocument.selection._setFake();
+		viewDocument.selection._setTo( viewDocument.selection.getRanges(), {
+			backward: viewDocument.selection.isBackward,
+			fake: true
+		} );
 
 		const data = {
 			keyCode,

+ 18 - 19
packages/ckeditor5-engine/tests/view/writer/writer.js

@@ -10,23 +10,33 @@ import ViewPosition from '../../../src/view/position';
 import createViewRoot from '../_utils/createroot';
 
 describe( 'Writer', () => {
-	let writer, attributes, root;
+	let writer, attributes, root, doc;
 
 	before( () => {
 		attributes = { foo: 'bar', baz: 'quz' };
-		const document = new Document();
-		root = createViewRoot( document );
-		writer = new Writer( document );
+		doc = new Document();
+		root = createViewRoot( doc );
+		writer = new Writer( doc );
 	} );
 
 	describe( 'setSelection()', () => {
-		it( 'should use selection._setTo method internally', () => {
-			const spy = sinon.spy( writer.document.selection, '_setTo' );
+		it( 'should set document view selection', () => {
 			const position = ViewPosition.createAt( root );
 			writer.setSelection( position );
 
-			sinon.assert.calledWith( spy, position );
-			spy.restore();
+			const ranges = Array.from( doc.selection.getRanges() );
+
+			expect( ranges.length ).to.equal( 1 );
+			expect( ranges[ 0 ].start.compareWith( position ) ).to.equal( 'same' );
+			expect( ranges[ 0 ].end.compareWith( position ) ).to.equal( 'same' );
+		} );
+
+		it( 'should be able to set fake selection', () => {
+			const position = ViewPosition.createAt( root );
+			writer.setSelection( position, { fake: true, label: 'foo' } );
+
+			expect( doc.selection.isFake ).to.be.true;
+			expect( doc.selection.fakeSelectionLabel ).to.equal( 'foo' );
 		} );
 	} );
 
@@ -40,17 +50,6 @@ describe( 'Writer', () => {
 		} );
 	} );
 
-	describe( 'setFakeSelection()', () => {
-		it( 'should use selection._setFake method internally', () => {
-			const spy = sinon.spy( writer.document.selection, '_setFake' );
-			const options = {};
-			writer.setFakeSelection( true, options );
-
-			sinon.assert.calledWithExactly( spy, true, options );
-			spy.restore();
-		} );
-	} );
-
 	describe( 'createText()', () => {
 		it( 'should create Text instance', () => {
 			const text = writer.createText( 'foo bar' );