Browse Source

Merge pull request #258 from ckeditor/i/6031

Feature: Introduced an option to configure a manual decorator to be "on" by default. Closes ckeditor/ckeditor5#6031.
Piotrek Koszuliński 5 years ago
parent
commit
11d6ae3ef7

+ 10 - 1
packages/ckeditor5-link/docs/features/link.md

@@ -132,7 +132,7 @@ ClassicEditor
 		// ...
 		link: {
 			decorators: {
-				addTargetToExternalLinks: {
+				openInNewTab: {
 					mode: 'manual',
 					label: 'Open in a new tab',
 					attributes: {
@@ -195,6 +195,15 @@ ClassicEditor
 					attributes: {
 						download: 'file'
 					}
+				},
+				openInNewTab: {
+					mode: 'manual',
+					label: 'Open in a new tab',
+					defaultValue: true,			// This option will be selected by default.
+					attributes: {
+						target: '_blank',
+						rel: 'noopener noreferrer'
+					}
 				}
 			}
 		}

+ 9 - 2
packages/ckeditor5-link/src/link.js

@@ -147,8 +147,13 @@ export default class Link extends Plugin {
  */
 
 /**
- * Represents a link decorator definition ({@link module:link/link~LinkDecoratorManualDefinition `'manual'`}
- * or {@link module:link/link~LinkDecoratorAutomaticDefinition `'automatic'`}).
+ * A link decorator definition. Two types implement this defition:
+ *
+ * * {@link module:link/link~LinkDecoratorManualDefinition}
+ * * {@link module:link/link~LinkDecoratorAutomaticDefinition}
+ *
+ * Refer to their document for more information about available options or to the
+ * {@glink features/link#custom-link-attributes-decorators link feature guide} for general information.
  *
  * @interface LinkDecoratorDefinition
  */
@@ -203,6 +208,7 @@ export default class Link extends Plugin {
  *		{
  *			mode: 'manual',
  *			label: 'Open in a new tab',
+ *			defaultValue: true,
  *			attributes: {
  *				target: '_blank',
  *				rel: 'noopener noreferrer'
@@ -215,4 +221,5 @@ export default class Link extends Plugin {
  * @property {Object} attributes Key-value pairs used as link attributes added to the output during the
  * {@glink framework/guides/architecture/editing-engine#conversion downcasting}.
  * Attributes should follow the {@link module:engine/view/elementdefinition~ElementDefinition} syntax.
+ * @property {Boolean} [defaultValue] Controls whether the decorator is "on" by default.
  */

+ 1 - 1
packages/ckeditor5-link/src/linkcommand.js

@@ -214,6 +214,6 @@ export default class LinkCommand extends Command {
 	 */
 	_getDecoratorStateFromModel( decoratorName ) {
 		const doc = this.editor.model.document;
-		return doc.selection.getAttribute( decoratorName ) || false;
+		return doc.selection.getAttribute( decoratorName );
 	}
 }

+ 1 - 1
packages/ckeditor5-link/src/linkui.js

@@ -144,7 +144,7 @@ export default class LinkUI extends Plugin {
 		const editor = this.editor;
 		const linkCommand = editor.commands.get( 'link' );
 
-		const formView = new LinkFormView( editor.locale, linkCommand.manualDecorators );
+		const formView = new LinkFormView( editor.locale, linkCommand );
 
 		formView.urlInputView.fieldView.bind( 'value' ).to( linkCommand, 'value' );
 

+ 11 - 11
packages/ckeditor5-link/src/ui/linkformview.js

@@ -39,10 +39,9 @@ export default class LinkFormView extends View {
 	 * Also see {@link #render}.
 	 *
 	 * @param {module:utils/locale~Locale} [locale] The localization services instance.
-	 * @param {module:utils/collection~Collection} [manualDecorators] Reference to manual decorators in
-	 * {@link module:link/linkcommand~LinkCommand#manualDecorators}.
+	 * @param {module:link/linkcommand~LinkCommand} linkCommand Reference to {@link module:link/linkcommand~LinkCommand}.
 	 */
-	constructor( locale, manualDecorators = [] ) {
+	constructor( locale, linkCommand ) {
 		super( locale );
 
 		const t = locale.t;
@@ -94,7 +93,7 @@ export default class LinkFormView extends View {
 		 * @readonly
 		 * @type {module:ui/viewcollection~ViewCollection}
 		 */
-		this._manualDecoratorSwitches = this._createManualDecoratorSwitches( manualDecorators );
+		this._manualDecoratorSwitches = this._createManualDecoratorSwitches( linkCommand );
 
 		/**
 		 * A collection of child views in the form.
@@ -102,7 +101,7 @@ export default class LinkFormView extends View {
 		 * @readonly
 		 * @type {module:ui/viewcollection~ViewCollection}
 		 */
-		this.children = this._createFormChildren( manualDecorators );
+		this.children = this._createFormChildren( linkCommand.manualDecorators );
 
 		/**
 		 * A collection of views that can be focused in the form.
@@ -135,7 +134,7 @@ export default class LinkFormView extends View {
 
 		const classList = [ 'ck', 'ck-link-form' ];
 
-		if ( manualDecorators.length ) {
+		if ( linkCommand.manualDecorators.length ) {
 			classList.push( 'ck-link-form_layout-vertical' );
 		}
 
@@ -258,14 +257,13 @@ export default class LinkFormView extends View {
 	 * made based on {@link module:link/linkcommand~LinkCommand#manualDecorators}.
 	 *
 	 * @private
-	 * @param {module:utils/collection~Collection} manualDecorators A reference to the
-	 * collection of manual decorators stored in the link command.
+	 * @param {module:link/linkcommand~LinkCommand} linkCommand A reference to the link command.
 	 * @returns {module:ui/viewcollection~ViewCollection} of switch buttons.
 	 */
-	_createManualDecoratorSwitches( manualDecorators ) {
+	_createManualDecoratorSwitches( linkCommand ) {
 		const switches = this.createCollection();
 
-		for ( const manualDecorator of manualDecorators ) {
+		for ( const manualDecorator of linkCommand.manualDecorators ) {
 			const switchButton = new SwitchButtonView( this.locale );
 
 			switchButton.set( {
@@ -274,7 +272,9 @@ export default class LinkFormView extends View {
 				withText: true
 			} );
 
-			switchButton.bind( 'isOn' ).to( manualDecorator, 'value' );
+			switchButton.bind( 'isOn' ).toMany( [ manualDecorator, linkCommand ], 'value', ( decoratorValue, commandValue ) => {
+				return commandValue === undefined && decoratorValue === undefined ? manualDecorator.defaultValue : decoratorValue;
+			} );
 
 			switchButton.on( 'execute', () => {
 				manualDecorator.set( 'value', !switchButton.isOn );

+ 9 - 1
packages/ckeditor5-link/src/utils/manualdecorator.js

@@ -27,8 +27,9 @@ export default class ManualDecorator {
 	 * @param {String} config.label The label used in the user interface to toggle the manual decorator.
 	 * @param {Object} config.attributes A set of attributes added to output data when the decorator is active for a specific link.
 	 * Attributes should keep the format of attributes defined in {@link module:engine/view/elementdefinition~ElementDefinition}.
+	 * @param {Boolean} [config.defaultValue] Controls whether the decorator is "on" by default.
 	 */
-	constructor( { id, label, attributes } ) {
+	constructor( { id, label, attributes, defaultValue } ) {
 		/**
 		 * An ID of a manual decorator which is the name of the attribute in the model, for example: 'linkManualDecorator0'.
 		 *
@@ -45,6 +46,13 @@ export default class ManualDecorator {
 		this.set( 'value' );
 
 		/**
+		 * The default value of manual decorator.
+		 *
+		 * @type {Boolean}
+		 */
+		this.defaultValue = defaultValue;
+
+		/**
 		 * The label used in the user interface to toggle the manual decorator.
 		 *
 		 * @type {String}

+ 51 - 14
packages/ckeditor5-link/tests/linkcommand.js

@@ -284,10 +284,18 @@ describe( 'LinkCommand', () => {
 							target: '_blank'
 						}
 					} ) );
+					command.manualDecorators.add( new ManualDecorator( {
+						id: 'linkIsSth',
+						label: 'Sth',
+						attributes: {
+							class: 'sth'
+						},
+						defaultValue: true
+					} ) );
 
 					model.schema.extend( '$text', {
 						allowIn: '$root',
-						allowAttributes: [ 'linkHref', 'linkIsFoo', 'linkIsBar' ]
+						allowAttributes: [ 'linkHref', 'linkIsFoo', 'linkIsBar', 'linkIsSth' ]
 					} );
 
 					model.schema.register( 'p', { inheritAllFrom: '$block' } );
@@ -302,19 +310,19 @@ describe( 'LinkCommand', () => {
 			it( 'should insert additional attributes to link when it is created', () => {
 				setData( model, 'foo[]bar' );
 
-				command.execute( 'url', { linkIsFoo: true, linkIsBar: true } );
+				command.execute( 'url', { linkIsFoo: true, linkIsBar: true, linkIsSth: true } );
 
 				expect( getData( model ) ).to
-					.equal( 'foo[<$text linkHref="url" linkIsBar="true" linkIsFoo="true">url</$text>]bar' );
+					.equal( 'foo[<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">url</$text>]bar' );
 			} );
 
 			it( 'should add additional attributes to link when link is modified', () => {
 				setData( model, 'f<$text linkHref="url">o[]oba</$text>r' );
 
-				command.execute( 'url', { linkIsFoo: true, linkIsBar: true } );
+				command.execute( 'url', { linkIsFoo: true, linkIsBar: true, linkIsSth: true } );
 
 				expect( getData( model ) ).to
-					.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true">ooba</$text>]r' );
+					.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">ooba</$text>]r' );
 			} );
 
 			it( 'should remove additional attributes to link if those are falsy', () => {
@@ -330,19 +338,19 @@ describe( 'LinkCommand', () => {
 			it( 'should insert additional attributes to link when it is created', () => {
 				setData( model, 'f[ooba]r' );
 
-				command.execute( 'url', { linkIsFoo: true, linkIsBar: true } );
+				command.execute( 'url', { linkIsFoo: true, linkIsBar: true, linkIsSth: true } );
 
 				expect( getData( model ) ).to
-					.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true">ooba</$text>]r' );
+					.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">ooba</$text>]r' );
 			} );
 
 			it( 'should add additional attributes to link when link is modified', () => {
 				setData( model, 'f[<$text linkHref="foo">ooba</$text>]r' );
 
-				command.execute( 'url', { linkIsFoo: true, linkIsBar: true } );
+				command.execute( 'url', { linkIsFoo: true, linkIsBar: true, linkIsSth: true } );
 
 				expect( getData( model ) ).to
-					.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true">ooba</$text>]r' );
+					.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">ooba</$text>]r' );
 			} );
 
 			it( 'should remove additional attributes to link if those are falsy', () => {
@@ -356,25 +364,54 @@ describe( 'LinkCommand', () => {
 
 		describe( 'restoreManualDecoratorStates()', () => {
 			it( 'synchronize values with current model state', () => {
-				setData( model, 'foo<$text linkHref="url" linkIsBar="true" linkIsFoo="true">u[]rl</$text>bar' );
+				setData( model, 'foo<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">u[]rl</$text>bar' );
 
 				expect( decoratorStates( command.manualDecorators ) ).to.deep.equal( {
 					linkIsFoo: true,
-					linkIsBar: true
+					linkIsBar: true,
+					linkIsSth: true
 				} );
 
 				command.manualDecorators.first.value = false;
 
 				expect( decoratorStates( command.manualDecorators ) ).to.deep.equal( {
 					linkIsFoo: false,
-					linkIsBar: true
+					linkIsBar: true,
+					linkIsSth: true
+				} );
+
+				command.restoreManualDecoratorStates();
+
+				expect( decoratorStates( command.manualDecorators ) ).to.deep.equal( {
+					linkIsFoo: true,
+					linkIsBar: true,
+					linkIsSth: true
+				} );
+			} );
+
+			it( 'synchronize values with current model state when the decorator that is "on" default is "off"', () => {
+				setData( model, 'foo<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="false">u[]rl</$text>bar' );
+
+				expect( decoratorStates( command.manualDecorators ) ).to.deep.equal( {
+					linkIsFoo: true,
+					linkIsBar: true,
+					linkIsSth: false
+				} );
+
+				command.manualDecorators.last.value = true;
+
+				expect( decoratorStates( command.manualDecorators ) ).to.deep.equal( {
+					linkIsFoo: true,
+					linkIsBar: true,
+					linkIsSth: true
 				} );
 
 				command.restoreManualDecoratorStates();
 
 				expect( decoratorStates( command.manualDecorators ) ).to.deep.equal( {
 					linkIsFoo: true,
-					linkIsBar: true
+					linkIsBar: true,
+					linkIsSth: false
 				} );
 			} );
 		} );
@@ -383,7 +420,7 @@ describe( 'LinkCommand', () => {
 			it( 'obtain current values from the model', () => {
 				setData( model, 'foo[<$text linkHref="url" linkIsBar="true">url</$text>]bar' );
 
-				expect( command._getDecoratorStateFromModel( 'linkIsFoo' ) ).to.be.false;
+				expect( command._getDecoratorStateFromModel( 'linkIsFoo' ) ).to.be.undefined;
 				expect( command._getDecoratorStateFromModel( 'linkIsBar' ) ).to.be.true;
 			} );
 		} );

+ 105 - 11
packages/ckeditor5-link/tests/ui/linkformview.js

@@ -18,6 +18,8 @@ import Collection from '@ckeditor/ckeditor5-utils/src/collection';
 import { add as addTranslations, _clear as clearTranslations } from '@ckeditor/ckeditor5-utils/src/translation-service';
 import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 import Link from '../../src/link';
+import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
+import mix from '@ckeditor/ckeditor5-utils/src/mix';
 
 describe( 'LinkFormView', () => {
 	let view;
@@ -25,7 +27,7 @@ describe( 'LinkFormView', () => {
 	testUtils.createSinonSandbox();
 
 	beforeEach( () => {
-		view = new LinkFormView( { t: val => val } );
+		view = new LinkFormView( { t: val => val }, { manualDecorators: [] } );
 		view.render();
 	} );
 
@@ -109,7 +111,7 @@ describe( 'LinkFormView', () => {
 		it( 'should register child views\' #element in #focusTracker', () => {
 			const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' );
 
-			view = new LinkFormView( { t: () => {} } );
+			view = new LinkFormView( { t: () => {} }, { manualDecorators: [] } );
 			view.render();
 
 			sinon.assert.calledWithExactly( spy.getCall( 0 ), view.urlInputView.element );
@@ -118,7 +120,7 @@ describe( 'LinkFormView', () => {
 		} );
 
 		it( 'starts listening for #keystrokes coming from #element', () => {
-			view = new LinkFormView( { t: () => {} } );
+			view = new LinkFormView( { t: () => {} }, { manualDecorators: [] } );
 
 			const spy = sinon.spy( view.keystrokes, 'listenTo' );
 
@@ -193,7 +195,7 @@ describe( 'LinkFormView', () => {
 	} );
 
 	describe( 'manual decorators', () => {
-		let view, collection;
+		let view, collection, linkCommand;
 		beforeEach( () => {
 			collection = new Collection();
 			collection.add( new ManualDecorator( {
@@ -208,7 +210,8 @@ describe( 'LinkFormView', () => {
 				label: 'Download',
 				attributes: {
 					download: 'download'
-				}
+				},
+				defaultValue: true
 			} ) );
 			collection.add( new ManualDecorator( {
 				id: 'decorator3',
@@ -220,7 +223,17 @@ describe( 'LinkFormView', () => {
 				}
 			} ) );
 
-			view = new LinkFormView( { t: val => val }, collection );
+			class LinkCommandMock {
+				constructor( manualDecorators ) {
+					this.manualDecorators = manualDecorators;
+					this.set( 'value' );
+				}
+			}
+			mix( LinkCommandMock, ObservableMixin );
+
+			linkCommand = new LinkCommandMock( collection );
+
+			view = new LinkFormView( { t: val => val }, linkCommand );
 			view.render();
 		} );
 
@@ -234,15 +247,18 @@ describe( 'LinkFormView', () => {
 
 			expect( view._manualDecoratorSwitches.get( 0 ) ).to.deep.include( {
 				name: 'decorator1',
-				label: 'Foo'
+				label: 'Foo',
+				isOn: undefined
 			} );
 			expect( view._manualDecoratorSwitches.get( 1 ) ).to.deep.include( {
 				name: 'decorator2',
-				label: 'Download'
+				label: 'Download',
+				isOn: true
 			} );
 			expect( view._manualDecoratorSwitches.get( 2 ) ).to.deep.include( {
 				name: 'decorator3',
-				label: 'Multi'
+				label: 'Multi',
+				isOn: undefined
 			} );
 		} );
 
@@ -264,11 +280,29 @@ describe( 'LinkFormView', () => {
 			expect( viewItem.isOn ).to.be.false;
 		} );
 
+		it( 'reacts on switch button changes for the decorator with defaultValue', () => {
+			const modelItem = collection.get( 1 );
+			const viewItem = view._manualDecoratorSwitches.get( 1 );
+
+			expect( modelItem.value ).to.be.undefined;
+			expect( viewItem.isOn ).to.be.true;
+
+			viewItem.element.dispatchEvent( new Event( 'click' ) );
+
+			expect( modelItem.value ).to.be.false;
+			expect( viewItem.isOn ).to.be.false;
+
+			viewItem.element.dispatchEvent( new Event( 'click' ) );
+
+			expect( modelItem.value ).to.be.true;
+			expect( viewItem.isOn ).to.be.true;
+		} );
+
 		describe( 'getDecoratorSwitchesState()', () => {
 			it( 'should provide object with decorators states', () => {
 				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
 					decorator1: undefined,
-					decorator2: undefined,
+					decorator2: true,
 					decorator3: undefined
 				} );
 
@@ -280,9 +314,69 @@ describe( 'LinkFormView', () => {
 
 				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
 					decorator1: true,
+					decorator2: false,
+					decorator3: false
+				} );
+			} );
+
+			it( 'should use decorator default value if command and decorator values are not set', () => {
+				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
+					decorator1: undefined,
 					decorator2: true,
+					decorator3: undefined
+				} );
+			} );
+
+			it( 'should use a decorator value if decorator value is set', () => {
+				for ( const decorator of collection ) {
+					decorator.value = true;
+				}
+
+				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
+					decorator1: true,
+					decorator2: true,
+					decorator3: true
+				} );
+
+				for ( const decorator of collection ) {
+					decorator.value = false;
+				}
+
+				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
+					decorator1: false,
+					decorator2: false,
+					decorator3: false
+				} );
+			} );
+
+			it( 'should use a decorator value if link command value is set', () => {
+				linkCommand.value = '';
+
+				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
+					decorator1: undefined,
+					decorator2: undefined,
+					decorator3: undefined
+				} );
+
+				for ( const decorator of collection ) {
+					decorator.value = false;
+				}
+
+				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
+					decorator1: false,
+					decorator2: false,
 					decorator3: false
 				} );
+
+				for ( const decorator of collection ) {
+					decorator.value = true;
+				}
+
+				expect( view.getDecoratorSwitchesState() ).to.deep.equal( {
+					decorator1: true,
+					decorator2: true,
+					decorator3: true
+				} );
 			} );
 		} );
 	} );
@@ -322,7 +416,7 @@ describe( 'LinkFormView', () => {
 				} )
 				.then( newEditor => {
 					editor = newEditor;
-					linkFormView = new LinkFormView( editor.locale, editor.commands.get( 'link' ).manualDecorators );
+					linkFormView = new LinkFormView( editor.locale, editor.commands.get( 'link' ) );
 				} );
 		} );
 

+ 17 - 0
packages/ckeditor5-link/tests/utils/manualdecorator.js

@@ -24,6 +24,23 @@ describe( 'Manual Decorator', () => {
 		expect( manualDecorator.id ).to.equal( 'foo' );
 		expect( manualDecorator.label ).to.equal( 'bar' );
 		expect( manualDecorator.attributes ).to.deep.equal( { one: 'two' } );
+		expect( manualDecorator.defaultValue ).to.deep.equal( undefined );
+	} );
+
+	it( 'constructor with defaultValue', () => {
+		manualDecorator = new ManualDecorator( {
+			id: 'foo',
+			label: 'bar',
+			attributes: {
+				one: 'two'
+			},
+			defaultValue: true
+		} );
+
+		expect( manualDecorator.id ).to.equal( 'foo' );
+		expect( manualDecorator.label ).to.equal( 'bar' );
+		expect( manualDecorator.attributes ).to.deep.equal( { one: 'two' } );
+		expect( manualDecorator.defaultValue ).to.deep.equal( true );
 	} );
 
 	it( '#value is observable', () => {