8
0
Просмотр исходного кода

Merge branch 'master' into t/ckeditor5/930

Piotrek Koszuliński 7 лет назад
Родитель
Сommit
730f806ed4

+ 1 - 1
packages/ckeditor5-utils/README.md

@@ -4,7 +4,7 @@ CKEditor 5 utilities
 [![Join the chat at https://gitter.im/ckeditor/ckeditor5](https://badges.gitter.im/ckeditor/ckeditor5.svg)](https://gitter.im/ckeditor/ckeditor5?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
 [![Join the chat at https://gitter.im/ckeditor/ckeditor5](https://badges.gitter.im/ckeditor/ckeditor5.svg)](https://gitter.im/ckeditor/ckeditor5?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
 [![npm version](https://badge.fury.io/js/%40ckeditor%2Fckeditor5-utils.svg)](https://www.npmjs.com/package/@ckeditor/ckeditor5-utils)
 [![npm version](https://badge.fury.io/js/%40ckeditor%2Fckeditor5-utils.svg)](https://www.npmjs.com/package/@ckeditor/ckeditor5-utils)
 [![Build Status](https://travis-ci.org/ckeditor/ckeditor5-utils.svg?branch=master)](https://travis-ci.org/ckeditor/ckeditor5-utils)
 [![Build Status](https://travis-ci.org/ckeditor/ckeditor5-utils.svg?branch=master)](https://travis-ci.org/ckeditor/ckeditor5-utils)
-[![BrowserStack Status](https://www.browserstack.com/automate/badge.svg?badge_key=d3hvenZqQVZERFQ5d09FWXdyT0ozVXhLaVltRFRjTTUyZGpvQWNmWVhUUT0tLUZqNlJ1YWRUd0RvdEVOaEptM1B2Q0E9PQ==--c9d3dee40b9b4471ff3fb516d9ecf8d09292c7e0)](https://www.browserstack.com/automate/public-build/d3hvenZqQVZERFQ5d09FWXdyT0ozVXhLaVltRFRjTTUyZGpvQWNmWVhUUT0tLUZqNlJ1YWRUd0RvdEVOaEptM1B2Q0E9PQ==--c9d3dee40b9b4471ff3fb516d9ecf8d09292c7e0)
+[![BrowserStack Status](https://automate.browserstack.com/automate/badge.svg?badge_key=d3hvenZqQVZERFQ5d09FWXdyT0ozVXhLaVltRFRjTTUyZGpvQWNmWVhUUT0tLUZqNlJ1YWRUd0RvdEVOaEptM1B2Q0E9PQ==--c9d3dee40b9b4471ff3fb516d9ecf8d09292c7e0)](https://automate.browserstack.com/public-build/d3hvenZqQVZERFQ5d09FWXdyT0ozVXhLaVltRFRjTTUyZGpvQWNmWVhUUT0tLUZqNlJ1YWRUd0RvdEVOaEptM1B2Q0E9PQ==--c9d3dee40b9b4471ff3fb516d9ecf8d09292c7e0)
 [![Coverage Status](https://coveralls.io/repos/github/ckeditor/ckeditor5-utils/badge.svg?branch=master)](https://coveralls.io/github/ckeditor/ckeditor5-utils?branch=master)
 [![Coverage Status](https://coveralls.io/repos/github/ckeditor/ckeditor5-utils/badge.svg?branch=master)](https://coveralls.io/github/ckeditor/ckeditor5-utils?branch=master)
 <br>
 <br>
 [![Dependency Status](https://david-dm.org/ckeditor/ckeditor5-utils/status.svg)](https://david-dm.org/ckeditor/ckeditor5-utils)
 [![Dependency Status](https://david-dm.org/ckeditor/ckeditor5-utils/status.svg)](https://david-dm.org/ckeditor/ckeditor5-utils)

+ 42 - 3
packages/ckeditor5-utils/src/observablemixin.js

@@ -75,8 +75,17 @@ const ObservableMixin = {
 				// Allow undefined as an initial value like A.define( 'x', undefined ) (#132).
 				// Allow undefined as an initial value like A.define( 'x', undefined ) (#132).
 				// Note: When properties map has no such own property, then its value is undefined.
 				// Note: When properties map has no such own property, then its value is undefined.
 				if ( oldValue !== value || !properties.has( name ) ) {
 				if ( oldValue !== value || !properties.has( name ) ) {
-					properties.set( name, value );
-					this.fire( 'change:' + name, name, value, oldValue );
+					// Fire `beforeChange` event before the new value will be changed to make it possible
+					// to override observable property without affecting `change` event.
+					// See https://github.com/ckeditor/ckeditor5-utils/issues/171.
+					let newValue = this.fire( 'beforeChange:' + name, name, value, oldValue );
+
+					if ( newValue === undefined ) {
+						newValue = value;
+					}
+
+					properties.set( name, newValue );
+					this.fire( 'change:' + name, name, newValue, oldValue );
 				}
 				}
 			}
 			}
 		} );
 		} );
@@ -664,7 +673,7 @@ function attachBindToListeners( observable, toBindings ) {
  *
  *
  *		observable.on( 'change:prop', ( evt, propertyName, newValue, oldValue ) => {
  *		observable.on( 'change:prop', ( evt, propertyName, newValue, oldValue ) => {
  *			console.log( `${ propertyName } has changed from ${ oldValue } to ${ newValue }` );
  *			console.log( `${ propertyName } has changed from ${ oldValue } to ${ newValue }` );
- *		} )
+ *		} );
  *
  *
  *		observable.prop = 2; // -> 'prop has changed from 1 to 2'
  *		observable.prop = 2; // -> 'prop has changed from 1 to 2'
  *
  *
@@ -674,6 +683,36 @@ function attachBindToListeners( observable, toBindings ) {
  * @param {*} oldValue The previous property value.
  * @param {*} oldValue The previous property value.
  */
  */
 
 
+/**
+ * Fired when a property value is going to be changed but is not changed yet (before the `change` event is fired).
+ *
+ * You can control the final value of the property by using
+ * the {@link module:utils/eventinfo~EventInfo#return event's `return` property}.
+ *
+ *		observable.set( 'prop', 1 );
+ *
+ *		observable.on( 'beforeChange:prop', ( evt, propertyName, newValue, oldValue ) => {
+ *			console.log( `Value is going to be changed from ${ oldValue } to ${ newValue }` );
+ *			console.log( `Current property value is ${ observable[ propertyName ] }` );
+ *
+ *			// Let's override the value.
+ *			evt.return = 3;
+ *		} );
+ *
+ *		observable.on( 'change:prop', ( evt, propertyName, newValue, oldValue ) => {
+ *			console.log( `Value has changed from ${ oldValue } to ${ newValue }` );
+ *		} );
+ *
+ *		observable.prop = 2; // -> 'Value is going to be changed from 1 to 2'
+ *		                     // -> 'Current property value is 1'
+ *		                     // -> 'Value has changed from 1 to 3'
+ *
+ * @event beforeChange:{property}
+ * @param {String} name The property name.
+ * @param {*} value The new property value.
+ * @param {*} oldValue The previous property value.
+ */
+
 /**
 /**
  * Creates and sets the value of an observable property of this object. Such an property becomes a part
  * Creates and sets the value of an observable property of this object. Such an property becomes a part
  * of the state and is be observable.
  * of the state and is be observable.

+ 73 - 0
packages/ckeditor5-utils/tests/observablemixin.js

@@ -144,6 +144,79 @@ describe( 'Observable', () => {
 			sinon.assert.notCalled( spyColor );
 			sinon.assert.notCalled( spyColor );
 		} );
 		} );
 
 
+		it( 'should fire the "beforeChange" event', () => {
+			const spy = sinon.spy();
+			const spyColor = sinon.spy();
+			const spyYear = sinon.spy();
+			const spyWheels = sinon.spy();
+
+			car.on( 'beforeChange', spy );
+			car.on( 'beforeChange:color', spyColor );
+			car.on( 'beforeChange:year', spyYear );
+			car.on( 'beforeChange:wheels', spyWheels );
+
+			// Set property in all possible ways.
+			car.color = 'blue';
+			car.set( { year: 2003 } );
+			car.set( 'wheels', 4 );
+
+			// Check number of calls.
+			sinon.assert.calledThrice( spy );
+			sinon.assert.calledOnce( spyColor );
+			sinon.assert.calledOnce( spyYear );
+			sinon.assert.calledOnce( spyWheels );
+
+			// Check context.
+			sinon.assert.alwaysCalledOn( spy, car );
+			sinon.assert.calledOn( spyColor, car );
+			sinon.assert.calledOn( spyYear, car );
+			sinon.assert.calledOn( spyWheels, car );
+
+			// Check params.
+			sinon.assert.calledWithExactly( spy, sinon.match.instanceOf( EventInfo ), 'color', 'blue', 'red' );
+			sinon.assert.calledWithExactly( spy, sinon.match.instanceOf( EventInfo ), 'year', 2003, 2015 );
+			sinon.assert.calledWithExactly( spy, sinon.match.instanceOf( EventInfo ), 'wheels', 4, sinon.match.typeOf( 'undefined' ) );
+			sinon.assert.calledWithExactly( spyColor, sinon.match.instanceOf( EventInfo ), 'color', 'blue', 'red' );
+			sinon.assert.calledWithExactly( spyYear, sinon.match.instanceOf( EventInfo ), 'year', 2003, 2015 );
+			sinon.assert.calledWithExactly(
+				spyWheels, sinon.match.instanceOf( EventInfo ),
+				'wheels', 4, sinon.match.typeOf( 'undefined' )
+			);
+		} );
+
+		it( 'should use "beforeChange" return value as an observable new value', () => {
+			car.color = 'blue';
+
+			const spy = sinon.spy();
+
+			car.on( 'beforeChange:color', evt => {
+				evt.stop();
+				evt.return = 'red';
+			}, { priority: 'high' } );
+
+			car.on( 'change:color', spy );
+
+			car.color = 'pink';
+
+			sinon.assert.calledWithExactly( spy, sinon.match.instanceOf( EventInfo ), 'color', 'red', 'blue' );
+		} );
+
+		it( 'should not fire the "beforeChange" event for the same property value', () => {
+			const spy = sinon.spy();
+			const spyColor = sinon.spy();
+
+			car.on( 'beforeChange', spy );
+			car.on( 'beforeChange:color', spyColor );
+
+			// Set the "color" property in all possible ways.
+			car.color = 'red';
+			car.set( 'color', 'red' );
+			car.set( { color: 'red' } );
+
+			sinon.assert.notCalled( spy );
+			sinon.assert.notCalled( spyColor );
+		} );
+
 		it( 'should throw when overriding already existing property', () => {
 		it( 'should throw when overriding already existing property', () => {
 			car.normalProperty = 1;
 			car.normalProperty = 1;