Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Added defaultValue to ManualDecorators. #258

Merged
merged 6 commits into from
Apr 7, 2020
Merged

Added defaultValue to ManualDecorators. #258

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3c88b6d
Added defaultValue to ManualDecorators.
niegowski Apr 2, 2020
8246201
Merge branch 'master' into i/6031
Reinmar Apr 3, 2020
2068a0f
Added upcast conversion for link manual decorators with default values.
niegowski Apr 6, 2020
e631e7e
Merge branch 'master' into i/6031
niegowski Apr 7, 2020
5ea7fac
Changed logic of link manual decorator's defaultValue.
niegowski Apr 7, 2020
b2dc424
Added more documentation and fixed the pointless interface doc.
Reinmar Apr 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,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=false] Controls whether the decorator is "on" by default.
*/
49 changes: 29 additions & 20 deletions src/linkcommand.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default class LinkCommand extends Command {
*/
restoreManualDecoratorStates() {
for ( const manualDecorator of this.manualDecorators ) {
manualDecorator.value = this._getDecoratorStateFromModel( manualDecorator.id );
manualDecorator.value = this._getDecoratorStateFromModel( manualDecorator );
}
}

Expand All @@ -60,7 +60,7 @@ export default class LinkCommand extends Command {
this.value = doc.selection.getAttribute( 'linkHref' );

for ( const manualDecorator of this.manualDecorators ) {
manualDecorator.value = this._getDecoratorStateFromModel( manualDecorator.id );
manualDecorator.value = this._getDecoratorStateFromModel( manualDecorator );
}

this.isEnabled = model.schema.checkAttributeInSelection( doc.selection, 'linkHref' );
Expand Down Expand Up @@ -132,14 +132,18 @@ export default class LinkCommand extends Command {
const model = this.editor.model;
const selection = model.document.selection;
// Stores information about manual decorators to turn them on/off when command is applied.
const truthyManualDecorators = [];
const falsyManualDecorators = [];
const setManualDecorators = [];
const removeManualDecorators = [];

for ( const name in manualDecoratorIds ) {
if ( manualDecoratorIds[ name ] ) {
truthyManualDecorators.push( name );
const manualDecorator = this.manualDecorators.get( name );
const value = manualDecoratorIds[ name ];

// We need to set an attribute for the disabled decorator if it's enabled by default (`defaultValue`).
if ( value || value != manualDecorator.defaultValue ) {
setManualDecorators.push( { name, value } );
} else {
falsyManualDecorators.push( name );
removeManualDecorators.push( name );
}
}

Expand All @@ -155,12 +159,12 @@ export default class LinkCommand extends Command {

writer.setAttribute( 'linkHref', href, linkRange );

truthyManualDecorators.forEach( item => {
writer.setAttribute( item, true, linkRange );
setManualDecorators.forEach( ( { name, value } ) => {
writer.setAttribute( name, value, linkRange );
} );

falsyManualDecorators.forEach( item => {
writer.removeAttribute( item, linkRange );
removeManualDecorators.forEach( name => {
writer.removeAttribute( name, linkRange );
} );

// Create new range wrapping changed link.
Expand All @@ -174,8 +178,8 @@ export default class LinkCommand extends Command {

attributes.set( 'linkHref', href );

truthyManualDecorators.forEach( item => {
attributes.set( item, true );
setManualDecorators.forEach( ( { name, value } ) => {
attributes.set( name, value );
} );

const node = writer.createText( href, attributes );
Expand All @@ -193,12 +197,12 @@ export default class LinkCommand extends Command {
for ( const range of ranges ) {
writer.setAttribute( 'linkHref', href, range );

truthyManualDecorators.forEach( item => {
writer.setAttribute( item, true, range );
setManualDecorators.forEach( ( { name, value } ) => {
writer.setAttribute( name, value, range );
} );

falsyManualDecorators.forEach( item => {
writer.removeAttribute( item, range );
removeManualDecorators.forEach( name => {
writer.removeAttribute( name, range );
} );
}
}
Expand All @@ -209,11 +213,16 @@ export default class LinkCommand extends Command {
* Provides information whether a decorator with a given name is present in the currently processed selection.
*
* @private
* @param {String} decoratorName The name of the manual decorator used in the model
* @param {module:link/utils~ManualDecorator} decorator The manual decorator used in the model.
* @returns {Boolean} The information whether a given decorator is currently present in the selection.
*/
_getDecoratorStateFromModel( decoratorName ) {
_getDecoratorStateFromModel( decorator ) {
const doc = this.editor.model.document;
return doc.selection.getAttribute( decoratorName ) || false;

if ( doc.selection.hasAttribute( decorator.id ) ) {
return doc.selection.getAttribute( decorator.id );
}

return decorator.defaultValue;
}
}
10 changes: 9 additions & 1 deletion src/utils/manualdecorator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=false] 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'.
*
Expand All @@ -44,6 +45,13 @@ export default class ManualDecorator {
*/
this.set( 'value' );

/**
* The default value of manual decorator.
*
* @type {Boolean}
*/
this.defaultValue = defaultValue || false;

/**
* The label used in the user interface to toggle the manual decorator.
*
Expand Down
91 changes: 76 additions & 15 deletions tests/linkcommand.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' } );
Expand All @@ -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', () => {
Expand All @@ -324,25 +332,33 @@ describe( 'LinkCommand', () => {

expect( getData( model ) ).to.equal( 'foo[<$text linkHref="url">url</$text>]bar' );
} );

it( 'should add additional attributes to link if those are falsy but decorator\'s defaultValue is set to true', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why storing the false value in the model? Also, how do you actually remove the value from the model?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I see this – the only thing that needs to change when defalutValue is true is the state of this button in the UI when you create a new link. Since it'll be on at that point, the command will be executed with isSth:true and then our job's done. The rest will work the way it works so far. Do I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed on a call:

  • LinkFormView is binding its isOn property to value of manual decorator,
  • value of a manual decorator is updated every time LinkCommand is refreshed,
  • every time value is extracted from the model's attribute,
  • we need to store false in the attribute to indicate that user already decided whether the manual decorator is supposed to be applied or not

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why storing the false value in the model? Also, how do you actually remove the value from the model?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that I correctly read the code – it indeed puts the false value in the model.

setData( model, 'foo<$text linkHref="url" linkIsBar="true" linkIsFoo="true">u[]rl</$text>bar' );

command.execute( 'url', { linkIsFoo: false, linkIsBar: false, linkIsSth: false } );

expect( getData( model ) ).to.equal( 'foo[<$text linkHref="url" linkIsSth="false">url</$text>]bar' );
} );
} );

describe( 'range selection', () => {
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', () => {
Expand All @@ -352,29 +368,66 @@ describe( 'LinkCommand', () => {

expect( getData( model ) ).to.equal( 'foo[<$text linkHref="url">url</$text>]bar' );
} );

it( 'should add additional attributes to link if those are falsy but decorator\'s defaultValue is set to true', () => {
setData( model, 'foo[<$text linkHref="url" linkIsBar="true" linkIsFoo="true">url</$text>]bar' );

command.execute( 'url', { linkIsFoo: false, linkIsBar: false, linkIsSth: false } );

expect( getData( model ) ).to.equal( 'foo[<$text linkHref="url" linkIsSth="false">url</$text>]bar' );
} );
} );

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
} );
} );
} );
Expand All @@ -383,8 +436,16 @@ 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( 'linkIsBar' ) ).to.be.true;
expect( command._getDecoratorStateFromModel( command.manualDecorators.get( 'linkIsFoo' ) ) ).to.be.false;
expect( command._getDecoratorStateFromModel( command.manualDecorators.get( 'linkIsBar' ) ) ).to.be.true;
} );

it( 'fallbacks to defaultValue if there is no attribute in model', () => {
setData( model, 'foo[<$text linkHref="url">url</$text>]bar' );

expect( command._getDecoratorStateFromModel( command.manualDecorators.get( 'linkIsFoo' ) ) ).to.be.false;
expect( command._getDecoratorStateFromModel( command.manualDecorators.get( 'linkIsBar' ) ) ).to.be.false;
expect( command._getDecoratorStateFromModel( command.manualDecorators.get( 'linkIsSth' ) ) ).to.be.true;
} );
} );
} );
Expand Down
17 changes: 17 additions & 0 deletions tests/utils/manualdecorator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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( false );
} );

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', () => {
Expand Down