Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The list style UI should call only the listStyle command #11171

Merged
merged 3 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions packages/ckeditor5-list/src/listpropertiesui.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,9 @@ function getStyleButtonCreator( { editor, listStyleCommand, parentCommandName }
editor.execute( 'listStyle', { type: listStyleCommand._defaultType } );
}
}
// If the content the selection is anchored to is not a list, let's create a list of a desired style.
// Otherwise, leave the creation of the styled list to the `ListStyleCommand`.
else {
editor.model.change( () => {
editor.execute( parentCommandName );
editor.execute( 'listStyle', { type } );
} );
}
Expand Down
36 changes: 34 additions & 2 deletions packages/ckeditor5-list/src/liststylecommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
*/

import { Command } from 'ckeditor5/src/core';
import { getSelectedListItems } from './utils';
import { getListTypeFromListStyleType, getSelectedListItems } from './utils';

/**
* The list style command. It changes the `listStyle` attribute of the selected list items.
*
* If the list type (numbered or bulleted) can be inferred from the passed style type,
* the command tries to convert selected items to a list of that type.
* It is used by the {@link module:list/listproperties~ListProperties list properties feature}.
*
* @extends module:core/command~Command
Expand Down Expand Up @@ -48,11 +51,13 @@ export default class ListStyleCommand extends Command {
* Executes the command.
*
* @param {Object} options
* @param {String|null} options.type The type of the list style, e.g. `'disc'` or `'square'`. If `null` is specified, the default
* @param {String|null} [options.type] The type of the list style, e.g. `'disc'` or `'square'`. If `null` is specified, the default
* style will be applied.
* @protected
*/
execute( options = {} ) {
this._tryToConvertItemsToList( options );

const model = this.editor.model;
const listItems = getSelectedListItems( model );

Expand Down Expand Up @@ -97,4 +102,31 @@ export default class ListStyleCommand extends Command {

return numberedList.isEnabled || bulletedList.isEnabled;
}

/**
* Check if the provided list style is valid. Also change the selection to a list if it's not set yet.
*
* @param {Object} options
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
* @param {String|null} [options.type] The type of the list style. If `null` is specified, the function does nothing.
* @private
*/
_tryToConvertItemsToList( options ) {
if ( !options.type ) {
return;
}

const listType = getListTypeFromListStyleType( options.type );

if ( !listType ) {
return;
}

const editor = this.editor;
const commandName = listType + 'List';
const command = editor.commands.get( commandName );

if ( !command.value ) {
editor.execute( commandName );
}
}
}
31 changes: 31 additions & 0 deletions packages/ckeditor5-list/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,37 @@ export function getSelectedListItems( model ) {
return listItems;
}

const BULLETED_LIST_STYLE_TYPES = [ 'disc', 'circle', 'square' ];

// There's a lot of them (https://www.w3.org/TR/css-counter-styles-3/#typedef-counter-style).
// Let's support only those that can be selected by ListPropertiesUI.
const NUMBERED_LIST_STYLE_TYPES = [
'decimal',
'decimal-leading-zero',
'lower-roman',
'upper-roman',
'lower-latin',
'upper-latin'
];

/**
* Checks whether the given list-style-type is supported by numbered or bulleted list.
*
* @param {String} listStyleType
* @returns {'bulleted'|'numbered'|null}
*/
export function getListTypeFromListStyleType( listStyleType ) {
if ( BULLETED_LIST_STYLE_TYPES.includes( listStyleType ) ) {
return 'bulleted';
}

if ( NUMBERED_LIST_STYLE_TYPES.includes( listStyleType ) ) {
return 'numbered';
}

return null;
}

// Implementation of getFillerOffset for view list item element.
//
// @returns {Number|null} Block filler offset or `null` if block filler is not needed.
Expand Down
98 changes: 91 additions & 7 deletions packages/ckeditor5-list/tests/liststylecommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ describe( 'ListStyleCommand', () => {
);
} );

it( 'should start searching for the list items from starting position (collapsed selection)', () => {
it( 'should start searching for the list items from starting position (non-collapsed selection)', () => {
setData( model,
'<listItem listIndent="0" listStyle="default" listType="bulleted">1.</listItem>' +
'<listItem listIndent="0" listStyle="default" listType="bulleted">2.</listItem>' +
Expand All @@ -271,7 +271,7 @@ describe( 'ListStyleCommand', () => {
);
} );

it( 'should start searching for the list items from ending position (collapsed selection)', () => {
it( 'should start searching for the list items from ending position (non-collapsed selection)', () => {
setData( model,
'<paragraph>[Foo.</paragraph>' +
'<listItem listIndent="0" listStyle="default" listType="bulleted">1.]</listItem>' +
Expand All @@ -282,7 +282,7 @@ describe( 'ListStyleCommand', () => {
listStyleCommand.execute( { type: 'circle' } );

expect( getData( model ) ).to.equal(
'<paragraph>[Foo.</paragraph>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">[Foo.</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">1.]</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">2.</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">3.</listItem>'
Expand Down Expand Up @@ -325,24 +325,108 @@ describe( 'ListStyleCommand', () => {
);
} );

it( 'should not update anything if no listItem found in the selection', () => {
it( 'should not update anything if no listItem found in the selection (default style)', () => {
setData( model,
'<paragraph>[Foo.]</paragraph>' +
'<listItem listIndent="0" listStyle="default" listType="bulleted">1.</listItem>'
'<listItem listIndent="0" listStyle="circle" listType="bulleted">1.</listItem>'
);

const modelChangeStub = sinon.stub( model, 'change' ).named( 'model#change' );

listStyleCommand.execute( { type: 'circle' } );
listStyleCommand.execute();

expect( getData( model ) ).to.equal(
'<paragraph>[Foo.]</paragraph>' +
'<listItem listIndent="0" listStyle="default" listType="bulleted">1.</listItem>'
'<listItem listIndent="0" listStyle="circle" listType="bulleted">1.</listItem>'
);

expect( modelChangeStub.called ).to.equal( false );
} );

it( 'should create a list list if no listItem found in the selection (circle, non-collapsed selection)', () => {
setData( model,
'<paragraph>[Foo.</paragraph>' +
'<paragraph>Bar.]</paragraph>'
);

const listCommand = editor.commands.get( 'bulletedList' );
const spy = sinon.spy( listCommand, 'execute' );

listStyleCommand.execute( { type: 'circle' } );

expect( getData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="circle" listType="bulleted">[Foo.</listItem>' +
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Bar.]</listItem>'
);

expect( spy.called ).to.be.true;

spy.restore();
} );

it( 'should create a list list if no listItem found in the selection (square, collapsed selection)', () => {
setData( model,
'<paragraph>Fo[]o.</paragraph>' +
'<paragraph>Bar.</paragraph>'
);

const listCommand = editor.commands.get( 'bulletedList' );
const spy = sinon.spy( listCommand, 'execute' );

listStyleCommand.execute( { type: 'circle' } );

expect( getData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="circle" listType="bulleted">Fo[]o.</listItem>' +
'<paragraph>Bar.</paragraph>'
);

expect( spy.called ).to.be.true;

spy.restore();
} );

it( 'should create a list list if no listItem found in the selection (decimal, non-collapsed selection)', () => {
setData( model,
'<paragraph>[Foo.</paragraph>' +
'<paragraph>Bar.]</paragraph>'
);

const listCommand = editor.commands.get( 'numberedList' );
const spy = sinon.spy( listCommand, 'execute' );

listStyleCommand.execute( { type: 'decimal' } );

expect( getData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="decimal" listType="numbered">[Foo.</listItem>' +
'<listItem listIndent="0" listStyle="decimal" listType="numbered">Bar.]</listItem>'
);

expect( spy.called ).to.be.true;

spy.restore();
} );

it( 'should create a list list if no listItem found in the selection (upper-roman, collapsed selection)', () => {
setData( model,
'<paragraph>Fo[]o.</paragraph>' +
'<paragraph>Bar.</paragraph>'
);

const listCommand = editor.commands.get( 'numberedList' );
const spy = sinon.spy( listCommand, 'execute' );

listStyleCommand.execute( { type: 'upper-roman' } );

expect( getData( model ) ).to.equal(
'<listItem listIndent="0" listStyle="upper-roman" listType="numbered">Fo[]o.</listItem>' +
'<paragraph>Bar.</paragraph>'
);

expect( spy.called ).to.be.true;

spy.restore();
} );

it( 'should update all items that belong to selected elements', () => {
// [x] = items that should be updated.
// All list items that belong to the same lists that selected items should be updated.
Expand Down
24 changes: 23 additions & 1 deletion packages/ckeditor5-list/tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest
import ListEditing from '../src/listediting';
import ListPropertiesEditing from '../src/listpropertiesediting';

import { createViewListItemElement, getSiblingListItem, getSiblingNodes } from '../src/utils';
import { createViewListItemElement, getListTypeFromListStyleType, getSiblingListItem, getSiblingNodes } from '../src/utils';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';

describe( 'utils', () => {
Expand Down Expand Up @@ -449,4 +449,26 @@ describe( 'utils', () => {
] );
} );
} );

describe( 'getListTypeFromListStyleType()', () => {
const testData = [
[ 'decimal', 'numbered' ],
[ 'decimal-leading-zero', 'numbered' ],
[ 'lower-roman', 'numbered' ],
[ 'upper-roman', 'numbered' ],
[ 'lower-latin', 'numbered' ],
[ 'upper-latin', 'numbered' ],
[ 'disc', 'bulleted' ],
[ 'circle', 'bulleted' ],
[ 'square', 'bulleted' ],
[ 'default', null ],
[ 'style-type-that-is-not-possibly-supported-by-css', null ]
];

for ( const [ style, type ] of testData ) {
it( `shoud return "${ type }" for "${ style }" style`, () => {
expect( getListTypeFromListStyleType( style ) ).to.equal( type );
} );
}
} );
} );