Skip to content

Commit

Permalink
Merge pull request #12093 from ckeditor/cf/4593-integrate-styles-drop…
Browse files Browse the repository at this point in the history 
…down-with-tc

Other (style): `StyleCommand#execute()` now expects to be passed an object with properties `styleName` and optional `forceValue` instead of a single string.
  • Loading branch information
@niegowski
niegowski authored Jul 19, 2022
2 parents 376227b + dd5d0f5 commit cf708bd
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 31 deletions.
16 changes: 11 additions & 5 deletions packages/ckeditor5-style/src/stylecommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,14 @@ export default class StyleCommand extends Command {
* * If the selection is on a range, the command applies the style classes to the nearest block parent element.
*
* @fires execute
* @param {String} styleName Style name matching the one defined in the
* @param {Object} [options] Command options.
* @param {String} options.styleName Style name matching the one defined in the
* {@link module:style/style~StyleConfig#definitions configuration}.
* @param {Boolean} [options.forceValue] Whether the command should add given style (`true`) or remove it (`false`) from the selection.
* If not set (default), the command will toggle the style basing on the first selected node. Note, that this will not force
* setting a style on an element that cannot receive given style.
*/
execute( styleName ) {
execute( { styleName, forceValue } ) {
if ( !this.enabledStyles.includes( styleName ) ) {
/**
* Style command can be executed only with a correct style name.
Expand All @@ -177,6 +181,8 @@ export default class StyleCommand extends Command {
...this._styleDefinitions.block
].find( ( { name } ) => name == styleName );

const shouldAddStyle = forceValue === undefined ? !this.value.includes( definition.name ) : forceValue;

model.change( () => {
let selectables;

Expand All @@ -187,10 +193,10 @@ export default class StyleCommand extends Command {
}

for ( const selectable of selectables ) {
if ( this.value.includes( definition.name ) ) {
htmlSupport.removeModelHtmlClass( definition.element, definition.classes, selectable );
} else {
if ( shouldAddStyle ) {
htmlSupport.addModelHtmlClass( definition.element, definition.classes, selectable );
} else {
htmlSupport.removeModelHtmlClass( definition.element, definition.classes, selectable );
}
}
} );
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-style/src/styleui.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export default class StyleUI extends Plugin {

// Execute the command when a style is selected in the styles panel.
panelView.on( 'execute', evt => {
editor.execute( 'style', evt.source.styleDefinition.name );
editor.execute( 'style', { styleName: evt.source.styleDefinition.name } );
} );

// Bind the state of the styles panel to the command.
Expand Down
125 changes: 101 additions & 24 deletions packages/ckeditor5-style/tests/stylecommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ describe( 'StyleCommand', () => {
setData( model, '<paragraph>fo[ob]ar</paragraph>' );

command.isEnabled = false;
command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal( '<paragraph>fo[ob]ar</paragraph>' );
} );
Expand All @@ -455,7 +455,7 @@ describe( 'StyleCommand', () => {

setData( model, '<paragraph>fo[ob]ar</paragraph>' );

command.execute( 'Invalid style' );
command.execute( { styleName: 'Invalid style' } );

expect( getData( model ) ).to.equal( '<paragraph>fo[ob]ar</paragraph>' );
sinon.assert.calledWithMatch( stub, 'style-command-executed-with-incorrect-style-name' );
Expand All @@ -465,7 +465,7 @@ describe( 'StyleCommand', () => {
it( 'should add htmlSpan attribute with proper class to the collapsed selection', () => {
setData( model, '<paragraph>foobar[]</paragraph>' );

command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal(
'<paragraph>foobar<$text htmlSpan="{"classes":["marker"]}">[]</$text></paragraph>'
Expand All @@ -483,8 +483,8 @@ describe( 'StyleCommand', () => {
it( 'should add htmlSpan attribute with proper classes to the collapsed selection', () => {
setData( model, '<paragraph>foobar[]</paragraph>' );

command.execute( 'Marker' );
command.execute( 'Typewriter' );
command.execute( { styleName: 'Marker' } );
command.execute( { styleName: 'Typewriter' } );

expect( getData( model ) ).to.equal(
'<paragraph>foobar<$text htmlSpan="{"classes":["marker","typewriter"]}">[]</$text></paragraph>'
Expand All @@ -502,7 +502,7 @@ describe( 'StyleCommand', () => {
it( 'should add htmlSpan attribute with proper class to the selected text', () => {
setData( model, '<paragraph>fo[ob]ar</paragraph>' );

command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal(
'<paragraph>fo[<$text htmlSpan="{"classes":["marker"]}">ob</$text>]ar</paragraph>'
Expand All @@ -512,8 +512,8 @@ describe( 'StyleCommand', () => {
it( 'should add htmlSpan attribute with proper classes to the selected text', () => {
setData( model, '<paragraph>fo[ob]ar</paragraph>' );

command.execute( 'Marker' );
command.execute( 'Typewriter' );
command.execute( { styleName: 'Marker' } );
command.execute( { styleName: 'Typewriter' } );

expect( getData( model ) ).to.equal(
'<paragraph>fo[<$text htmlSpan="{"classes":["marker","typewriter"]}">ob</$text>]ar</paragraph>'
Expand All @@ -524,7 +524,7 @@ describe( 'StyleCommand', () => {
// initial selection [foo b]ar baz.
setData( model, '<paragraph>[foo b]ar baz</paragraph>' );

command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal(
'<paragraph>[<$text htmlSpan="{"classes":["marker"]}">foo b</$text>]ar baz</paragraph>'
Expand All @@ -538,7 +538,7 @@ describe( 'StyleCommand', () => {
) );
} );

command.execute( 'Typewriter' );
command.execute( { styleName: 'Typewriter' } );

expect( getData( model ) ).to.equal(
'<paragraph>[' +
Expand All @@ -552,7 +552,7 @@ describe( 'StyleCommand', () => {
it( 'should add htmlSpan attribute to the selected text if definition specify multiple classes', () => {
setData( model, '<paragraph>fo[ob]ar</paragraph>' );

command.execute( 'Multiple classes' );
command.execute( { styleName: 'Multiple classes' } );

expect( getData( model ) ).to.equal(
'<paragraph>fo[<$text htmlSpan="{"classes":["class-one","class-two"]}">ob</$text>]ar</paragraph>'
Expand All @@ -566,7 +566,7 @@ describe( 'StyleCommand', () => {
'<paragraph>ba]z</paragraph>'
);

command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal(
'<paragraph>f[<$text htmlSpan="{"classes":["marker"]}">oo</$text></paragraph>' +
Expand All @@ -578,9 +578,9 @@ describe( 'StyleCommand', () => {
it( 'should remove class from htmlSpan attribute element', () => {
setData( model, '<paragraph>foo[bar]</paragraph>' );

command.execute( 'Marker' );
command.execute( 'Typewriter' );
command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );
command.execute( { styleName: 'Typewriter' } );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal(
'<paragraph>foo[<$text htmlSpan="{"classes":["typewriter"]}">bar</$text>]</paragraph>'
Expand All @@ -590,20 +590,52 @@ describe( 'StyleCommand', () => {
it( 'should remove htmlSpan element when removing class attribute to the selection', () => {
setData( model, '<paragraph>foo[bar]</paragraph>' );

command.execute( 'Marker' );
command.execute( 'Marker' );
command.execute( { styleName: 'Marker' } );
command.execute( { styleName: 'Marker' } );

expect( getData( model ) ).to.equal(
'<paragraph>foo[bar]</paragraph>'
);
} );

it( 'should force adding style if the command was called with `forceValue=true`', () => {
setData( model,
'<paragraph>' +
'fo' +
'[<$text htmlSpan=\'{"classes":["marker"]}\'>ob</$text>' +
'ar]' +
'</paragraph>'
);

command.execute( { styleName: 'Marker', forceValue: true } );

expect( getData( model ) ).to.equal(
'<paragraph>fo[<$text htmlSpan="{"classes":["marker"]}">obar</$text>]</paragraph>'
);
} );

it( 'should force removing style if the command was called with `forceValue=false`', () => {
setData( model,
'<paragraph>' +
'[fo' +
'<$text htmlSpan=\'{"classes":["marker"]}\'>ob</$text>]' +
'ar' +
'</paragraph>'
);

command.execute( { styleName: 'Marker', forceValue: false } );

expect( getData( model ) ).to.equal(
'<paragraph>[foob]ar</paragraph>'
);
} );
} );

describe( 'block styles', () => {
it( 'should add htmlAttribute with proper class to the selected element', () => {
setData( model, '<heading1>foo[]bar</heading1>' );

command.execute( 'Big heading' );
command.execute( { styleName: 'Big heading' } );

expect( getData( model ) ).to.equal(
'<heading1 htmlAttributes="{"classes":["big-heading"]}">foo[]bar</heading1>'
Expand All @@ -613,8 +645,8 @@ describe( 'StyleCommand', () => {
it( 'should add htmlAttribute with multiple classes to the selected element', () => {
setData( model, '<heading1>foo[]bar</heading1>' );

command.execute( 'Big heading' );
command.execute( 'Red heading' );
command.execute( { styleName: 'Big heading' } );
command.execute( { styleName: 'Red heading' } );

expect( getData( model ) ).to.equal(
'<heading1 htmlAttributes="{"classes":["big-heading","red"]}">foo[]bar</heading1>'
Expand All @@ -628,7 +660,7 @@ describe( 'StyleCommand', () => {
'<heading1>ba]z</heading1>'
);

command.execute( 'Red heading' );
command.execute( { styleName: 'Red heading' } );

expect( getData( model ) ).to.equal(
'<heading1 htmlAttributes="{"classes":["red"]}">fo[o</heading1>' +
Expand All @@ -652,7 +684,7 @@ describe( 'StyleCommand', () => {
'</blockQuote>'
);

command.execute( 'Side quote' );
command.execute( { styleName: 'Side quote' } );

expect( getData( model ) ).to.equal(
'<blockQuote>' +
Expand All @@ -672,11 +704,56 @@ describe( 'StyleCommand', () => {
it( 'should remove htmlAttribute from the selected element', () => {
setData( model, '<heading1>foo[]bar</heading1>' );

command.execute( 'Big heading' );
command.execute( 'Big heading' );
command.execute( { styleName: 'Big heading' } );
command.execute( { styleName: 'Big heading' } );

expect( getData( model ) ).to.equal( '<heading1>foo[]bar</heading1>' );
} );

it( 'should force adding style if the command was called with `forceValue=true`', () => {
setData( model,
'<heading1>foo</heading1>' +
'<heading1 htmlAttributes=\'{"classes":["red"]}\'>b[ar</heading1>' +
'<heading1>ba]z</heading1>' );

command.execute( { styleName: 'Red heading', forceValue: true } );

expect( getData( model ) ).to.equal(
'<heading1>foo</heading1>' +
'<heading1 htmlAttributes="{"classes":["red"]}">b[ar</heading1>' +
'<heading1 htmlAttributes="{"classes":["red"]}">ba]z</heading1>'
);
} );

it( 'should not force adding a style on an element that cannot receive it', () => {
sinon.stub( console, 'warn' );

setData( model,
'<paragraph>f[oo</paragraph>' +
'<paragraph>ba]r</paragraph>' );

command.execute( { styleName: 'Red heading', forceValue: true } );

expect( getData( model ) ).to.equal(
'<paragraph>f[oo</paragraph>' +
'<paragraph>ba]r</paragraph>'
);
} );

it( 'should force removing style if the command was called with `forceValue=false`', () => {
setData( model,
'<heading1>f[oo</heading1>' +
'<heading1 htmlAttributes=\'{"classes":["red"]}\'>ba]r</heading1>' +
'<heading1>baz</heading1>' );

command.execute( { styleName: 'Red heading', forceValue: false } );

expect( getData( model ) ).to.equal(
'<heading1>f[oo</heading1>' +
'<heading1>ba]r</heading1>' +
'<heading1>baz</heading1>'
);
} );
} );
} );

Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-style/tests/styleui.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe( 'StyleUI', () => {

panel.fire( new EventInfo( buttonMock, 'execute' ) );

sinon.assert.calledOnceWithExactly( commandExecuteStub, 'style', 'foo' );
sinon.assert.calledOnceWithExactly( commandExecuteStub, 'style', { styleName: 'foo' } );
} );

it( 'should bind #activeStyles to the command', () => {
Expand Down

0 comments on commit cf708bd

Please sign in to comment.