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

Added DocumentListStyle command #11232

Merged
Show file tree
Hide file tree
Changes from 3 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
29 changes: 28 additions & 1 deletion packages/ckeditor5-list/src/documentlist/utils/listwalker.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export default class ListWalker {
* @param {Boolean} [options.sameItemId=false] Whether should return only blocks with the same `listItemId` attribute
* as the start element.
* @param {Boolean} [options.sameItemType=false] Whether should return only blocks wit the same `listType` attribute.
* @param {Array.<String>} [options.sameAttributes=[]] Additional attributes that must be the same for each block.
niegowski marked this conversation as resolved.
Show resolved Hide resolved
* @param {Boolean} [options.sameIndent=false] Whether blocks with the same indent level as the start block should be included
* in the result.
* @param {Boolean} [options.lowerIndent=false] Whether blocks with a lower indent level than the start block should be included
Expand Down Expand Up @@ -63,6 +64,18 @@ export default class ListWalker {
*/
this._startItemType = startElement.getAttribute( 'listType' );

/**
* Additional attributes of the start block.
*
* @private
* @type {Map}
*/
this._startAttributes = new Map();

for ( const attr of ( options.sameAttributes || [] ) ) {
this._startAttributes.set( attr, startElement.getAttribute( attr ) );
}

/**
* The iterating direction.
*
Expand Down Expand Up @@ -95,6 +108,14 @@ export default class ListWalker {
*/
this._sameItemType = !!options.sameItemType;

/**
* Additional attributes that must be the same for each block.
*
* @private
* @type {Array.<String>}
*/
this._sameAttributes = options.sameAttributes || [];

/**
* Whether blocks with the same indent level as the start block should be included in the result.
*
Expand Down Expand Up @@ -129,7 +150,8 @@ export default class ListWalker {
* @param {Boolean} [options.includeSelf=false] Whether start block should be included in the result (if it's matching other criteria).
* @param {Boolean} [options.sameItemId=false] Whether should return only blocks with the same `listItemId` attribute
* as the start element.
* @param {Boolean} [options.sameItemType=false] Whether should return only blocks wit the same `listType` attribute.
* @param {Boolean} [options.sameItemType=false] Whether should return only blocks with the same `listType` attribute.
* @param {Array.<String>} [options.sameAttributes=[]] Additional attributes that must be the same for each block.
* @param {Boolean} [options.sameIndent=false] Whether blocks with the same indent level as the start block should be included
* in the result.
* @param {Boolean} [options.lowerIndent=false] Whether blocks with a lower indent level than the start block should be included
Expand Down Expand Up @@ -207,6 +229,11 @@ export default class ListWalker {
if ( this._sameItemType && node.getAttribute( 'listType' ) != this._startItemType ) {
break;
}

// Abort if item has any additionaly specified attribute different.
if ( this._sameAttributes.some( attr => node.getAttribute( attr ) != this._startAttributes.get( attr ) ) ) {
break;
}
}

// There is another block for the same list item so the nested items were in the same list item.
Expand Down
8 changes: 6 additions & 2 deletions packages/ckeditor5-list/src/documentlist/utils/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,25 @@ export function getNestedListBlocks( listItem ) {
}

/**
* Returns array of all blocks/items of the same list as given block (same indent, same type).
* Returns array of all blocks/items of the same list as given block (same indent, same type and properties).
*
* @protected
* @param {module:engine/model/element~Element} listItem Starting list item element.
* @returns {Array.<module:engine/model/element~Element>}
*/
export function getListItems( listItem ) {
const listAddidionalProperties = [ 'listReversed', 'listStart', 'listStyle' ];
niegowski marked this conversation as resolved.
Show resolved Hide resolved

const backwardBlocks = new ListWalker( listItem, {
sameIndent: true,
sameItemType: true
sameItemType: true,
sameAttributes: listAddidionalProperties
} );

const forwardBlocks = new ListWalker( listItem, {
sameIndent: true,
sameItemType: true,
sameAttributes: listAddidionalProperties,
includeSelf: true,
direction: 'forward'
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import { Plugin } from 'ckeditor5/src/core';
import DocumentListEditing from '../documentlist/documentlistediting';
import DocumentListStyleCommand from './documentliststylecommand';
import { listPropertiesDowncastConverter, listPropertiesUpcastConverter } from './converters';
import { iterateSiblingListBlocks } from '../documentlist/utils/listwalker';

Expand Down Expand Up @@ -145,8 +146,8 @@ function createAttributeStrategies( enabledProperties ) {
defaultValue: DEFAULT_LIST_TYPE,
viewConsumables: { styles: 'list-style-type' },

addCommand( /* editor */ ) {
// editor.commands.add( 'listStyle', new ListStyleCommand( editor, DEFAULT_LIST_TYPE ) );
addCommand( editor ) {
editor.commands.add( 'listStyle', new DocumentListStyleCommand( editor, DEFAULT_LIST_TYPE ) );
},

appliesToListItem() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/**
* @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/**
* @module list/documentlistproperties/documentliststylecommand
*/

import { Command } from 'ckeditor5/src/core';
import { expandListBlocksToCompleteItems, getListItems } from '../documentlist/utils/model';
import { getListTypeFromListStyleType } from './utils/style';

/**
* The list style command. It changes `listStyle` attribute of the selected list items.
* It is used by the {@link module:list/documentlistproperties~DocumentListProperties list properties feature}.
*
* @extends module:core/command~Command
*/
export default class DocumentListStyleCommand extends Command {
/**
* Creates an instance of the command.
*
* @param {module:core/editor/editor~Editor} editor The editor instance.
* @param {String} defaultType The list type that will be used by default if the value was not specified during
* the command execution.
*/
constructor( editor, defaultType ) {
super( editor );

/**
* The default type of the list style.
*
* @protected
* @member {String}
*/
this._defaultType = defaultType;
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @inheritDoc
*/
refresh() {
this.value = this._getValue();
this.isEnabled = this._checkEnabled();
}

/**
* 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
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
* style will be applied.
* @protected
*/
execute( options = {} ) {
const model = this.editor.model;
const document = model.document;

model.change( writer => {
this._tryToConvertItemsToList( options );

model.enqueueChange( writer.batch, writer => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be required, commands executed inside a change block will automatically be a part of the outermost change block.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you needed this to make the following blocks pass the block.hasAttribute( 'listStyle' ) test that was added by the post-fixer after the first change block and before the enqueueChange block but you could check whether the selected blocks are list items at all.

let blocks = Array.from( document.selection.getSelectedBlocks() )
.filter( block => block.hasAttribute( 'listStyle' ) );

if ( !blocks.length ) {
return;
}

if ( document.selection.isCollapsed ) {
blocks = getListItems( blocks[ 0 ] );
} else {
blocks = expandListBlocksToCompleteItems( blocks, { withNested: false } );
}

for ( const block of blocks ) {
writer.setAttribute( 'listStyle', options.type || this._defaultType, block );
}
} );
} );
}

/**
* Checks the command's {@link #value}.
*
* @private
* @returns {String|null} The current value.
*/
_getValue() {
const listItem = this.editor.model.document.selection.getFirstPosition().parent;

if ( listItem && listItem.hasAttribute( 'listItemId' ) ) {
return listItem.getAttribute( 'listStyle' );
}

return null;
}

/**
* Checks whether the command can be enabled in the current context.
*
* @private
* @returns {Boolean} Whether the command should be enabled.
*/
_checkEnabled() {
const editor = this.editor;

const numberedList = editor.commands.get( 'numberedList' );
const bulletedList = editor.commands.get( 'bulletedList' );

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
* @param {String|null} [options.type] The type of the list style. If `null` is specified, the function does nothing.
* @private
niegowski marked this conversation as resolved.
Show resolved Hide resolved
*/
_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 );
}
}
}
39 changes: 39 additions & 0 deletions packages/ckeditor5-list/src/documentlistproperties/utils/style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/**
* @module list/documentlistproperties/utils/style
*/

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;
}
12 changes: 0 additions & 12 deletions packages/ckeditor5-list/tests/documentlist/_utils-tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,18 +426,6 @@ describe( 'mockList()', () => {
);
} );

it( 'should not parse the custom list style if provided in the following list item', () => {
expect( modelList( [
'* foo {style:123}',
' bar',
'* baz {style:abc}'
] ) ).to.equalMarkup(
'<paragraph listIndent="0" listItemId="000" listStyle="123" listType="bulleted">foo</paragraph>' +
'<paragraph listIndent="0" listItemId="000" listStyle="123" listType="bulleted">bar</paragraph>' +
'<paragraph listIndent="0" listItemId="002" listStyle="123" listType="bulleted">baz {style:abc}</paragraph>'
);
} );

it( 'should parse the custom list style of the different adjacent list type', () => {
expect( modelList( [
'* foo {style:123}',
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-list/tests/documentlist/_utils/uid.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { ListItemUid } from '../../../src/documentlist/utils/model';
export default function stubUid( start = 0xa00 ) {
const seq = sequence( start );

sinon.stub( ListItemUid, 'next' ).callsFake( () => seq.next().value );
return sinon.stub( ListItemUid, 'next' ).callsFake( () => seq.next().value );
}

function* sequence( num ) {
Expand Down
32 changes: 15 additions & 17 deletions packages/ckeditor5-list/tests/documentlist/_utils/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,23 +274,21 @@ export function modelList( lines, { ignoreIdConflicts = false } = {} ) {
return '';
} );

if ( !stack[ listIndent ] || stack[ listIndent ].listType != props.listType ) {
niegowski marked this conversation as resolved.
Show resolved Hide resolved
content = content.replace( /\s*{(?:(style|start|reversed):)([^}]+)}\s*/g, ( match, key, value ) => {
switch ( key ) {
case 'style':
props.listStyle = value;
break;
case 'start':
props.listStart = parseInt( value );
break;
case 'reversed':
props.listReversed = value;
break;
}

return '';
} );
}
content = content.replace( /\s*{(?:(style|start|reversed):)([^}]+)}\s*/g, ( match, key, value ) => {
switch ( key ) {
case 'style':
props.listStyle = value;
break;
case 'start':
props.listStart = parseInt( value );
break;
case 'reversed':
props.listReversed = value;
break;
}

return '';
} );

if ( !ignoreIdConflicts && seenIds.has( props.listItemId ) ) {
throw new Error( 'ID conflict: ' + props.listItemId );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';

describe( 'DocumentListCommand', () => {
let editor, command, model, doc, root, changedBlocks;
let editor, command, model, doc, root, changedBlocks, stub;

testUtils.createSinonSandbox();

Expand All @@ -30,7 +30,11 @@ describe( 'DocumentListCommand', () => {
model.schema.register( 'blockQuote', { inheritAllFrom: '$container' } );
model.schema.extend( '$container', { allowAttributes: [ 'listType', 'listIndent', 'listItemId' ] } );

stubUid();
stub = stubUid();
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
} );

afterEach( () => {
stub.restore();
} );

describe( 'bulleted', () => {
Expand Down
Loading