From b1513a47b16c6a5b1f4e90acd7e99cb0dee5f619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 5 Aug 2019 15:57:31 +0200 Subject: [PATCH 01/42] Introduced the editing part of TODO list feature. --- src/converters.js | 81 +++++++++++------- src/list.js | 3 +- src/listcommand.js | 4 +- src/listediting.js | 8 +- src/listui.js | 40 ++------- src/todolist.js | 34 ++++++++ src/todolistconverters.js | 55 ++++++++++++ src/todolistediting.js | 171 ++++++++++++++++++++++++++++++++++++++ src/todolistui.js | 29 +++++++ src/todolistutils.js | 56 +++++++++++++ src/utils.js | 32 +++++++ theme/icons/todolist.svg | 1 + theme/list.css | 70 ++++++++++++++++ 13 files changed, 511 insertions(+), 73 deletions(-) create mode 100644 src/todolist.js create mode 100644 src/todolistconverters.js create mode 100644 src/todolistediting.js create mode 100644 src/todolistui.js create mode 100644 src/todolistutils.js create mode 100644 theme/icons/todolist.svg create mode 100644 theme/list.css diff --git a/src/converters.js b/src/converters.js index 5b2ae0e..baf8461 100644 --- a/src/converters.js +++ b/src/converters.js @@ -8,6 +8,7 @@ */ import { createViewListItemElement } from './utils'; +import { addTodoElementsToListItem, removeTodoElementsFromListItem } from './todolistutils'; import TreeWalker from '@ckeditor/ckeditor5-engine/src/model/treewalker'; /** @@ -36,7 +37,7 @@ export function modelViewInsertion( model ) { consumable.consume( data.item, 'attribute:listIndent' ); const modelItem = data.item; - const viewItem = generateLiInUl( modelItem, conversionApi ); + const viewItem = generateLiInUl( modelItem, conversionApi, model ); injectViewList( modelItem, viewItem, conversionApi, model ); }; @@ -92,37 +93,44 @@ export function modelViewRemove( model ) { * by breaking view elements, changing their name and merging them. * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute - * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. - * @param {Object} data Additional information about the change. - * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface. + * @param {module:engine/model/model~Model} model Model instance. */ -export function modelViewChangeType( evt, data, conversionApi ) { - if ( !conversionApi.consumable.consume( data.item, 'attribute:listType' ) ) { - return; - } +export function modelViewChangeType( model ) { + return ( evt, data, conversionApi ) => { + if ( !conversionApi.consumable.consume( data.item, 'attribute:listType' ) ) { + return; + } - const viewItem = conversionApi.mapper.toViewElement( data.item ); - const viewWriter = conversionApi.writer; + const viewItem = conversionApi.mapper.toViewElement( data.item ); + const viewWriter = conversionApi.writer; - // 1. Break the container after and before the list item. - // This will create a view list with one view list item -- the one that changed type. - viewWriter.breakContainer( viewWriter.createPositionBefore( viewItem ) ); - viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) ); + // Break the container after and before the list item. + // This will create a view list with one view list item -- the one that changed type. + viewWriter.breakContainer( viewWriter.createPositionBefore( viewItem ) ); + viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) ); - // 2. Change name of the view list that holds the changed view item. - // We cannot just change name property, because that would not render properly. - let viewList = viewItem.parent; - const listName = data.attributeNewValue == 'numbered' ? 'ol' : 'ul'; - viewList = viewWriter.rename( listName, viewList ); + // Change name of the view list that holds the changed view item. + // We cannot just change name property, because that would not render properly. + let viewList = viewItem.parent; + const listName = data.attributeNewValue == 'numbered' ? 'ol' : 'ul'; + viewList = viewWriter.rename( listName, viewList ); + + // Add or remove checkbox for toto list. + if ( data.attributeNewValue == 'todo' ) { + addTodoElementsToListItem( viewWriter, viewItem, data.item, model ); + } else if ( data.attributeOldValue == 'todo' ) { + removeTodoElementsFromListItem( viewWriter, viewItem, data.item, model ); + } - // 3. Merge the changed view list with other lists, if possible. - mergeViewLists( viewWriter, viewList, viewList.nextSibling ); - mergeViewLists( viewWriter, viewList.previousSibling, viewList ); + // Merge the changed view list with other lists, if possible. + mergeViewLists( viewWriter, viewList, viewList.nextSibling ); + mergeViewLists( viewWriter, viewList.previousSibling, viewList ); - // 4. Consumable insertion of children inside the item. They are already handled by re-building the item in view. - for ( const child of data.item.getChildren() ) { - conversionApi.consumable.consume( child, 'insert' ); - } + // Consumable insertion of children inside the item. They are already handled by re-building the item in view. + for ( const child of data.item.getChildren() ) { + conversionApi.consumable.consume( child, 'insert' ); + } + }; } /** @@ -787,15 +795,20 @@ export function modelIndentPasteFixer( evt, [ content, selectable ] ) { // Helper function that creates a `` or (`
    `) structure out of given `modelItem` model `listItem` element. // Then, it binds created view list item (
  1. ) with model `listItem` element. // The function then returns created view list item (
  2. ). -function generateLiInUl( modelItem, conversionApi ) { +function generateLiInUl( modelItem, conversionApi, model ) { const mapper = conversionApi.mapper; const viewWriter = conversionApi.writer; const listType = modelItem.getAttribute( 'listType' ) == 'numbered' ? 'ol' : 'ul'; const viewItem = createViewListItemElement( viewWriter ); const viewList = viewWriter.createContainerElement( listType, null ); + viewWriter.insert( viewWriter.createPositionAt( viewList, 0 ), viewItem ); + if ( modelItem.getAttribute( 'listType' ) == 'todo' ) { + addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ); + } + mapper.bindElements( modelItem, viewItem ); return viewItem; @@ -915,13 +928,19 @@ function getSiblingListItem( modelItem, options ) { } // Helper function that takes two parameters, that are expected to be view list elements, and merges them. -// The merge happen only if both parameters are UL or OL elements. +// The merge happen only if both parameters are list elements of the same types (the same element name and the same class attributes). function mergeViewLists( viewWriter, firstList, secondList ) { - if ( firstList && secondList && ( firstList.name == 'ul' || firstList.name == 'ol' ) && firstList.name == secondList.name ) { - return viewWriter.mergeContainers( viewWriter.createPositionAfter( firstList ) ); + // Check if two lists are going to be merged. + if ( !firstList || !secondList || ( firstList.name != 'ul' && firstList.name != 'ol' ) ) { + return null; } - return null; + // Both parameters are list elements, so compare types now. + if ( firstList.name != secondList.name || firstList.getAttribute( 'class' ) !== secondList.getAttribute( 'class' ) ) { + return null; + } + + return viewWriter.mergeContainers( viewWriter.createPositionAfter( firstList ) ); } // Helper function that takes model list item element `modelItem`, corresponding view list item element `injectedItem` diff --git a/src/list.js b/src/list.js index 89bcd09..583c084 100644 --- a/src/list.js +++ b/src/list.js @@ -9,6 +9,7 @@ import ListEditing from './listediting'; import ListUI from './listui'; +import TodoList from './todolist'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; @@ -25,7 +26,7 @@ export default class List extends Plugin { * @inheritDoc */ static get requires() { - return [ ListEditing, ListUI ]; + return [ ListEditing, ListUI, TodoList ]; } /** diff --git a/src/listcommand.js b/src/listcommand.js index 73b3a60..070f319 100644 --- a/src/listcommand.js +++ b/src/listcommand.js @@ -29,9 +29,9 @@ export default class ListCommand extends Command { * The type of the list created by the command. * * @readonly - * @member {'numbered'|'bulleted'} + * @member {'numbered'|'bulleted'|'todo'} */ - this.type = type == 'bulleted' ? 'bulleted' : 'numbered'; + this.type = type; /** * A flag indicating whether the command is active, which means that the selection starts in a list of the same type. diff --git a/src/listediting.js b/src/listediting.js index 51c0387..0ead906 100644 --- a/src/listediting.js +++ b/src/listediting.js @@ -26,7 +26,7 @@ import { modelIndentPasteFixer, viewModelConverter, modelToViewPosition, - viewToModelPosition + viewToModelPosition, } from './converters'; /** @@ -56,7 +56,7 @@ export default class ListEditing extends Plugin { // If there are blocks allowed inside list item, algorithms using `getSelectedBlocks()` will have to be modified. editor.model.schema.register( 'listItem', { inheritAllFrom: '$block', - allowAttributes: [ 'listType', 'listIndent' ] + allowAttributes: [ 'listType', 'listIndent', 'listChecked' ] } ); // Converters. @@ -77,8 +77,8 @@ export default class ListEditing extends Plugin { data.downcastDispatcher.on( 'insert', modelViewSplitOnInsert, { priority: 'high' } ); data.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( editor.model ) ); - editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType ); - data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType ); + editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); + data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); editing.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); data.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); diff --git a/src/listui.js b/src/listui.js index a86705f..7f3c736 100644 --- a/src/listui.js +++ b/src/listui.js @@ -7,11 +7,12 @@ * @module list/listui */ +import { createUIComponent } from './utils'; + import numberedListIcon from '../theme/icons/numberedlist.svg'; import bulletedListIcon from '../theme/icons/bulletedlist.svg'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; /** * The list UI feature. It introduces the `'numberedList'` and `'bulletedList'` buttons that @@ -24,41 +25,10 @@ export default class ListUI extends Plugin { * @inheritDoc */ init() { - // Create two buttons and link them with numberedList and bulletedList commands. const t = this.editor.t; - this._addButton( 'numberedList', t( 'Numbered List' ), numberedListIcon ); - this._addButton( 'bulletedList', t( 'Bulleted List' ), bulletedListIcon ); - } - - /** - * Helper method for initializing a button and linking it with an appropriate command. - * - * @private - * @param {String} commandName The name of the command. - * @param {Object} label The button label. - * @param {String} icon The source of the icon. - */ - _addButton( commandName, label, icon ) { - const editor = this.editor; - editor.ui.componentFactory.add( commandName, locale => { - const command = editor.commands.get( commandName ); - - const buttonView = new ButtonView( locale ); - - buttonView.set( { - label, - icon, - tooltip: true - } ); - - // Bind button model to command. - buttonView.bind( 'isOn', 'isEnabled' ).to( command, 'value', 'isEnabled' ); - - // Execute command. - this.listenTo( buttonView, 'execute', () => editor.execute( commandName ) ); - - return buttonView; - } ); + // Create two buttons and link them with numberedList and bulletedList commands. + createUIComponent( this.editor, 'numberedList', t( 'Numbered List' ), numberedListIcon ); + createUIComponent( this.editor, 'bulletedList', t( 'Bulleted List' ), bulletedListIcon ); } } diff --git a/src/todolist.js b/src/todolist.js new file mode 100644 index 0000000..913cc53 --- /dev/null +++ b/src/todolist.js @@ -0,0 +1,34 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module list/todolist + */ + +import TodoListEditing from './todolistediting'; +import TodoListUI from './todolistui'; + +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; + +/** + * The todo list feature. + * + * @extends module:core/plugin~Plugin + */ +export default class List extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ TodoListEditing, TodoListUI ]; + } + + /** + * @inheritDoc + */ + static get pluginName() { + return 'TodoList'; + } +} diff --git a/src/todolistconverters.js b/src/todolistconverters.js new file mode 100644 index 0000000..42257dc --- /dev/null +++ b/src/todolistconverters.js @@ -0,0 +1,55 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module list/todolistconverters + */ + +import { createCheckMarkElement } from './todolistutils'; + +export function modelViewTextInsertion( evt, data, conversionApi ) { + const parent = data.range.start.parent; + + if ( parent.name != 'listItem' || parent.getAttribute( 'listType' ) != 'todo' ) { + return; + } + + conversionApi.consumable.consume( data.item, 'insert' ); + + const viewWriter = conversionApi.writer; + const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); + const viewText = viewWriter.createText( data.item.data ); + + viewWriter.insert( viewPosition.offset ? viewPosition : viewPosition.getShiftedBy( 1 ), viewText ); +} + +export function modelViewChangeChecked( model ) { + return ( evt, data, conversionApi ) => { + if ( !conversionApi.consumable.consume( data.item, 'attribute:listChecked' ) ) { + return; + } + + const { mapper, writer: viewWriter } = conversionApi; + const isChecked = !!data.item.getAttribute( 'listChecked' ); + const viewItem = mapper.toViewElement( data.item ); + const uiElement = findCheckmarkElement( viewWriter, viewItem ); + + viewWriter.insert( + viewWriter.createPositionAfter( uiElement ), + createCheckMarkElement( isChecked, viewWriter, isChecked => { + model.change( writer => writer.setAttribute( 'listChecked', isChecked, data.item ) ); + } ) + ); + viewWriter.remove( uiElement ); + }; +} + +function findCheckmarkElement( viewWriter, parent ) { + for ( const item of viewWriter.createRangeIn( parent ).getItems() ) { + if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) ) { + return item; + } + } +} diff --git a/src/todolistediting.js b/src/todolistediting.js new file mode 100644 index 0000000..7a9d502 --- /dev/null +++ b/src/todolistediting.js @@ -0,0 +1,171 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module list/todolistediting + */ + +import ListCommand from './listcommand'; +import ListEditing from './listediting'; + +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; + +import { + modelViewTextInsertion, + modelViewChangeChecked +} from './todolistconverters'; + +/** + * @extends module:core/plugin~Plugin + */ +export default class TodoListEditing extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ ListEditing ]; + } + + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const editing = editor.editing; + const viewDocument = editing.view.document; + + editor.model.schema.extend( 'listItem', { + allowAttributes: [ 'listChecked' ] + } ); + + // Converters. + editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } ); + editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( editor.model ) ); + + // Register command for todo list. + editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); + + // Move selection after a checkbox element. + viewDocument.registerPostFixer( writer => moveUIElementsAfterCheckmark( writer, this._getChangedCheckmarkElements() ) ); + + // Move all uiElements after a checkbox element. + viewDocument.registerPostFixer( writer => moveSelectionAfterCheckmark( writer, viewDocument.selection ) ); + + // Remove `listChecked` attribute when a host element is no longer a todo list item. + const listItemsToFix = new Set(); + + this.listenTo( editor.model, 'applyOperation', ( evt, args ) => { + const operation = args[ 0 ]; + + if ( operation.type != 'changeAttribute' && operation.key != 'listType' && operation.oldValue == 'todoList' ) { + for ( const item of operation.range.getItems() ) { + if ( item.name == 'listItem' && item.hasAttribute( 'listChecked' ) ) { + listItemsToFix.add( item ); + } + } + } + + if ( operation.type == 'rename' && operation.oldName == 'listItem' ) { + const item = operation.position.nodeAfter; + + if ( item.hasAttribute( 'listChecked' ) ) { + listItemsToFix.add( item ); + } + } + } ); + + editor.model.document.registerPostFixer( writer => { + let hasChanged = false; + + for ( const listItem of listItemsToFix ) { + writer.removeAttribute( 'listChecked', listItem ); + hasChanged = true; + } + + listItemsToFix.clear(); + + return hasChanged; + } ); + } + + /** + * Gets list of all checkmark elements that are going to be rendered. + * + * @private + * @returns {Array.} + */ + _getChangedCheckmarkElements() { + const editingView = this.editor.editing.view; + const elements = []; + + for ( const element of Array.from( editingView._renderer.markedChildren ) ) { + for ( const item of editingView.createRangeIn( element ).getItems() ) { + if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) && !elements.includes( item ) ) { + elements.push( item ); + } + } + } + + return elements; + } +} + +// @private +// @param {module:engine/view/downcastwriter~DowncastWriter} writer +// @param {Array.} uiElements +// @returns {Boolean} +function moveUIElementsAfterCheckmark( writer, uiElements ) { + let hasChanged = false; + + for ( const uiElement of uiElements ) { + const listItem = getListItemAncestor( uiElement ); + const positionAtListItem = writer.createPositionAt( listItem, 0 ); + const positionBeforeUiElement = writer.createPositionBefore( uiElement ); + + if ( positionAtListItem.isEqual( positionBeforeUiElement ) ) { + continue; + } + + const range = writer.createRange( positionAtListItem, positionBeforeUiElement ); + + for ( const item of Array.from( range.getItems() ) ) { + if ( item.is( 'uiElement' ) ) { + writer.move( writer.createRangeOn( item ), writer.createPositionAfter( uiElement ) ); + hasChanged = true; + } + } + } + + return hasChanged; +} + +// @private +// @param {module:engine/view/downcastwriter~DowncastWriter} writer +// @param {module:engine/view/documentselection~DocumentSelection} selection +function moveSelectionAfterCheckmark( writer, selection ) { + if ( !selection.isCollapsed ) { + return false; + } + + const position = selection.getFirstPosition(); + + if ( position.parent.name === 'li' && position.offset == 0 && position.nodeAfter && position.nodeAfter.is( 'uiElement' ) ) { + writer.setSelection( position.parent, 1 ); + + return true; + } + + return false; +} + +// @private +// @param {module:engine/view/uielement~UIElement} element +function getListItemAncestor( element ) { + for ( const parent of element.getAncestors() ) { + if ( parent.name == 'li' ) { + return parent; + } + } +} diff --git a/src/todolistui.js b/src/todolistui.js new file mode 100644 index 0000000..b02549f --- /dev/null +++ b/src/todolistui.js @@ -0,0 +1,29 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module list/todolistui + */ + +import { createUIComponent } from './utils'; + +import todoListIcon from '../theme/icons/todolist.svg'; +import '../theme/list.css'; + +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; + +/** + * @extends module:core/plugin~Plugin + */ +export default class TodoListUI extends Plugin { + /** + * @inheritDoc + */ + init() { + const t = this.editor.t; + + createUIComponent( this.editor, 'todoList', t( 'Todo List' ), todoListIcon ); + } +} diff --git a/src/todolistutils.js b/src/todolistutils.js new file mode 100644 index 0000000..c9da25b --- /dev/null +++ b/src/todolistutils.js @@ -0,0 +1,56 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module list/todolistutils + */ + +/* global document */ + +export function addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ) { + const isChecked = !!modelItem.getAttribute( 'listChecked' ); + + viewWriter.addClass( 'todo-list', viewItem.parent ); + + viewWriter.insert( + viewWriter.createPositionAt( viewItem, 0 ), + createCheckMarkElement( isChecked, viewWriter, isChecked => { + model.change( writer => writer.setAttribute( 'listChecked', isChecked, modelItem ) ); + } ) + ); +} + +export function removeTodoElementsFromListItem( viewWriter, viewItem, modelItem, model ) { + viewWriter.removeClass( 'todo-list', viewItem.parent ); + viewWriter.remove( viewItem.getChild( 0 ) ); + model.change( writer => writer.removeAttribute( 'listChecked', modelItem ) ); +} + +export function createCheckMarkElement( isChecked, viewWriter, onChange ) { + const uiElement = viewWriter.createUIElement( + 'label', + { + class: 'todo-list__checkmark', + contenteditable: false + }, + function( domDocument ) { + const domElement = this.toDomElement( domDocument ); + const checkbox = document.createElement( 'input' ); + + checkbox.type = 'checkbox'; + checkbox.checked = isChecked; + checkbox.addEventListener( 'change', evt => onChange( evt.target.checked ) ); + domElement.appendChild( checkbox ); + + return domElement; + } + ); + + if ( isChecked ) { + viewWriter.addClass( 'todo-list__checkmark_checked', uiElement ); + } + + return uiElement; +} diff --git a/src/utils.js b/src/utils.js index aa3e020..3529c6f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -8,6 +8,7 @@ */ import { getFillerOffset } from '@ckeditor/ckeditor5-engine/src/view/containerelement'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; /** * Creates list item {@link module:engine/view/containerelement~ContainerElement}. @@ -22,6 +23,37 @@ export function createViewListItemElement( writer ) { return viewItem; } +/** + * Helper method for creating an UI button and linking it with an appropriate command. + * + * @private + * @param {module:core/editor/editor~Editor} editor The editor instance to which UI component will be added. + * @param {String} commandName The name of the command. + * @param {Object} label The button label. + * @param {String} icon The source of the icon. + */ +export function createUIComponent( editor, commandName, label, icon ) { + editor.ui.componentFactory.add( commandName, locale => { + const command = editor.commands.get( commandName ); + + const buttonView = new ButtonView( locale ); + + buttonView.set( { + label, + icon, + tooltip: true + } ); + + // Bind button model to command. + buttonView.bind( 'isOn', 'isEnabled' ).to( command, 'value', 'isEnabled' ); + + // Execute command. + buttonView.on( 'execute', () => editor.execute( commandName ) ); + + return buttonView; + } ); +} + // Implementation of getFillerOffset for view list item element. // // @returns {Number|null} Block filler offset or `null` if block filler is not needed. diff --git a/theme/icons/todolist.svg b/theme/icons/todolist.svg new file mode 100644 index 0000000..d3c3b17 --- /dev/null +++ b/theme/icons/todolist.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/theme/list.css b/theme/list.css new file mode 100644 index 0000000..598aa7e --- /dev/null +++ b/theme/list.css @@ -0,0 +1,70 @@ +:root { + --ck-todo-list-checkmark-size: 14px; + --ck-color-todo-list-checkmark-background: hsl(120, 100%, 42%); + --ck-color-todo-list-checkmark-border: hsl( 120, 100%, 27%); +} + +.ck .todo-list { + list-style: none; + position: relative; +} + +.ck .todo-list > li { + margin-bottom: 5px; +} + +.ck .todo-list > li > .todo-list { + margin-top: 5px; +} + +/* Let's hide native checkbox and make a fancy one. */ +.ck .todo-list__checkmark input[type='checkbox'] { + display: block; + width: 100%; + height: 100%; + opacity: 0; +} + +.ck .todo-list__checkmark { + cursor: pointer; + width: var(--ck-todo-list-checkmark-size); + height: var(--ck-todo-list-checkmark-size); + position: relative; + display: inline-block; + left: -20px; + margin-right: -10px; +} + +.ck .todo-list__checkmark::before { + content: ''; + position: absolute; + display: block; + width: 100%; + height: 100%; + border: 1px solid var(--ck-color-base-text); + border-radius: var(--ck-border-radius); + transition: 250ms ease-in-out box-shadow, 250ms ease-in-out background, 250ms ease-in-out border; +} + +.ck .todo-list__checkmark::after { + pointer-events: none; + content: ''; + position: absolute; + left: 5px; + top: 3px; + display: block; + width: 3px; + height: 6px; + border: solid #fff; + border-width: 0 2px 2px 0; + transform: rotate(45deg); +} + +.ck .todo-list__checkmark:hover::before { + box-shadow: 0 0 0 5px hsla(0, 0%, 0%, 0.1) +} + +.ck .todo-list__checkmark_checked::before { + background: var(--ck-color-todo-list-checkmark-background); + border-color: var(--ck-color-todo-list-checkmark-border); +} From 70ff55b400fe88c371a17e879f77947d43bb5dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 5 Aug 2019 16:35:17 +0200 Subject: [PATCH 02/42] Move todo conversion out of list converters. --- src/converters.js | 251 ++++++++------------------------------ src/listediting.js | 8 +- src/todolistconverters.js | 71 ++++++++++- src/todolistediting.js | 9 +- src/utils.js | 194 ++++++++++++++++++++++++++++- 5 files changed, 325 insertions(+), 208 deletions(-) diff --git a/src/converters.js b/src/converters.js index baf8461..e17e48d 100644 --- a/src/converters.js +++ b/src/converters.js @@ -7,8 +7,13 @@ * @module list/converters */ -import { createViewListItemElement } from './utils'; -import { addTodoElementsToListItem, removeTodoElementsFromListItem } from './todolistutils'; +import { + generateLiInUl, + injectViewList, + mergeViewLists, + getSiblingListItem, + positionAfterUiElements +} from './utils'; import TreeWalker from '@ckeditor/ckeditor5-engine/src/model/treewalker'; /** @@ -37,7 +42,7 @@ export function modelViewInsertion( model ) { consumable.consume( data.item, 'attribute:listIndent' ); const modelItem = data.item; - const viewItem = generateLiInUl( modelItem, conversionApi, model ); + const viewItem = generateLiInUl( modelItem, conversionApi ); injectViewList( modelItem, viewItem, conversionApi, model ); }; @@ -90,47 +95,56 @@ export function modelViewRemove( model ) { * A model-to-view converter for `type` attribute change on `listItem` model element. * * This change means that `
  3. ` elements parent changes from `
      ` to `
        ` (or vice versa). This is accomplished - * by breaking view elements, changing their name and merging them. + * by breaking view elements and changing their name. Next {@link module:list/converters~modelViewMergeAfterChangeType} + * converter will try to merge split nodes. * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute - * @param {module:engine/model/model~Model} model Model instance. + * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. + * @param {Object} data Additional information about the change. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface. */ -export function modelViewChangeType( model ) { - return ( evt, data, conversionApi ) => { - if ( !conversionApi.consumable.consume( data.item, 'attribute:listType' ) ) { - return; - } +export function modelViewChangeType( evt, data, conversionApi ) { + if ( !conversionApi.consumable.consume( data.item, 'attribute:listType' ) ) { + return; + } - const viewItem = conversionApi.mapper.toViewElement( data.item ); - const viewWriter = conversionApi.writer; + const viewItem = conversionApi.mapper.toViewElement( data.item ); + const viewWriter = conversionApi.writer; - // Break the container after and before the list item. - // This will create a view list with one view list item -- the one that changed type. - viewWriter.breakContainer( viewWriter.createPositionBefore( viewItem ) ); - viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) ); + // Break the container after and before the list item. + // This will create a view list with one view list item -- the one that changed type. + viewWriter.breakContainer( viewWriter.createPositionBefore( viewItem ) ); + viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) ); - // Change name of the view list that holds the changed view item. - // We cannot just change name property, because that would not render properly. - let viewList = viewItem.parent; - const listName = data.attributeNewValue == 'numbered' ? 'ol' : 'ul'; - viewList = viewWriter.rename( listName, viewList ); - - // Add or remove checkbox for toto list. - if ( data.attributeNewValue == 'todo' ) { - addTodoElementsToListItem( viewWriter, viewItem, data.item, model ); - } else if ( data.attributeOldValue == 'todo' ) { - removeTodoElementsFromListItem( viewWriter, viewItem, data.item, model ); - } + // Change name of the view list that holds the changed view item. + // We cannot just change name property, because that would not render properly. + const viewList = viewItem.parent; + const listName = data.attributeNewValue == 'numbered' ? 'ol' : 'ul'; - // Merge the changed view list with other lists, if possible. - mergeViewLists( viewWriter, viewList, viewList.nextSibling ); - mergeViewLists( viewWriter, viewList.previousSibling, viewList ); + viewWriter.rename( listName, viewList ); +} - // Consumable insertion of children inside the item. They are already handled by re-building the item in view. - for ( const child of data.item.getChildren() ) { - conversionApi.consumable.consume( child, 'insert' ); - } - }; +/** + * A model-to-view converter that try to merge nodes split by {@link module:list/converters~modelViewChangeType}. + * + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute + * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. + * @param {Object} data Additional information about the change. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface. + */ +export function modelViewMergeAfterChangeType( evt, data, conversionApi ) { + const viewItem = conversionApi.mapper.toViewElement( data.item ); + const viewList = viewItem.parent; + const viewWriter = conversionApi.writer; + + // Merge the changed view list with other lists, if possible. + mergeViewLists( viewWriter, viewList, viewList.nextSibling ); + mergeViewLists( viewWriter, viewList.previousSibling, viewList ); + + // Consumable insertion of children inside the item. They are already handled by re-building the item in view. + for ( const child of data.item.getChildren() ) { + conversionApi.consumable.consume( child, 'insert' ); + } } /** @@ -792,28 +806,6 @@ export function modelIndentPasteFixer( evt, [ content, selectable ] ) { } } -// Helper function that creates a `
        ` or (`
          `) structure out of given `modelItem` model `listItem` element. -// Then, it binds created view list item (
        1. ) with model `listItem` element. -// The function then returns created view list item (
        2. ). -function generateLiInUl( modelItem, conversionApi, model ) { - const mapper = conversionApi.mapper; - const viewWriter = conversionApi.writer; - const listType = modelItem.getAttribute( 'listType' ) == 'numbered' ? 'ol' : 'ul'; - const viewItem = createViewListItemElement( viewWriter ); - - const viewList = viewWriter.createContainerElement( listType, null ); - - viewWriter.insert( viewWriter.createPositionAt( viewList, 0 ), viewItem ); - - if ( modelItem.getAttribute( 'listType' ) == 'todo' ) { - addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ); - } - - mapper.bindElements( modelItem, viewItem ); - - return viewItem; -} - // Helper function that converts children of a given `
        3. ` view element into corresponding model elements. // The function maintains proper order of elements if model `listItem` is split during the conversion // due to block children conversion. @@ -901,140 +893,6 @@ function findNextListItem( startPosition ) { return value.value.item; } -// Helper function that seeks for a previous list item sibling of given model item which meets given criteria. -// `options` object may contain one or more of given values (by default they are `false`): -// `options.sameIndent` - whether sought sibling should have same indent (default = no), -// `options.smallerIndent` - whether sought sibling should have smaller indent (default = no). -// `options.listIndent` - the reference indent. -// Either `options.sameIndent` or `options.smallerIndent` should be set to `true`. -function getSiblingListItem( modelItem, options ) { - const sameIndent = !!options.sameIndent; - const smallerIndent = !!options.smallerIndent; - const indent = options.listIndent; - - let item = modelItem; - - while ( item && item.name == 'listItem' ) { - const itemIndent = item.getAttribute( 'listIndent' ); - - if ( ( sameIndent && indent == itemIndent ) || ( smallerIndent && indent > itemIndent ) ) { - return item; - } - - item = item.previousSibling; - } - - return null; -} - -// Helper function that takes two parameters, that are expected to be view list elements, and merges them. -// The merge happen only if both parameters are list elements of the same types (the same element name and the same class attributes). -function mergeViewLists( viewWriter, firstList, secondList ) { - // Check if two lists are going to be merged. - if ( !firstList || !secondList || ( firstList.name != 'ul' && firstList.name != 'ol' ) ) { - return null; - } - - // Both parameters are list elements, so compare types now. - if ( firstList.name != secondList.name || firstList.getAttribute( 'class' ) !== secondList.getAttribute( 'class' ) ) { - return null; - } - - return viewWriter.mergeContainers( viewWriter.createPositionAfter( firstList ) ); -} - -// Helper function that takes model list item element `modelItem`, corresponding view list item element `injectedItem` -// that is not added to the view and is inside a view list element (`ul` or `ol`) and is that's list only child. -// The list is inserted at correct position (element breaking may be needed) and then merged with it's siblings. -// See comments below to better understand the algorithm. -function injectViewList( modelItem, injectedItem, conversionApi, model ) { - const injectedList = injectedItem.parent; - const mapper = conversionApi.mapper; - const viewWriter = conversionApi.writer; - - // Position where view list will be inserted. - let insertPosition = mapper.toViewPosition( model.createPositionBefore( modelItem ) ); - - // 1. Find previous list item that has same or smaller indent. Basically we are looking for a first model item - // that is "parent" or "sibling" of injected model item. - // If there is no such list item, it means that injected list item is the first item in "its list". - const refItem = getSiblingListItem( modelItem.previousSibling, { - sameIndent: true, - smallerIndent: true, - listIndent: modelItem.getAttribute( 'listIndent' ) - } ); - const prevItem = modelItem.previousSibling; - - if ( refItem && refItem.getAttribute( 'listIndent' ) == modelItem.getAttribute( 'listIndent' ) ) { - // There is a list item with same indent - we found same-level sibling. - // Break the list after it. Inserted view item will be inserted in the broken space. - const viewItem = mapper.toViewElement( refItem ); - insertPosition = viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) ); - } else { - // There is no list item with same indent. Check previous model item. - if ( prevItem && prevItem.name == 'listItem' ) { - // If it is a list item, it has to have lower indent. - // It means that inserted item should be added to it as its nested item. - insertPosition = mapper.toViewPosition( model.createPositionAt( prevItem, 'end' ) ); - } else { - // Previous item is not a list item (or does not exist at all). - // Just map the position and insert the view item at mapped position. - insertPosition = mapper.toViewPosition( model.createPositionBefore( modelItem ) ); - } - } - - insertPosition = positionAfterUiElements( insertPosition ); - - // Insert the view item. - viewWriter.insert( insertPosition, injectedList ); - - // 2. Handle possible children of injected model item. - if ( prevItem && prevItem.name == 'listItem' ) { - const prevView = mapper.toViewElement( prevItem ); - - const walkerBoundaries = viewWriter.createRange( viewWriter.createPositionAt( prevView, 0 ), insertPosition ); - const walker = walkerBoundaries.getWalker( { ignoreElementEnd: true } ); - - for ( const value of walker ) { - if ( value.item.is( 'li' ) ) { - const breakPosition = viewWriter.breakContainer( viewWriter.createPositionBefore( value.item ) ); - const viewList = value.item.parent; - - const targetPosition = viewWriter.createPositionAt( injectedItem, 'end' ); - mergeViewLists( viewWriter, targetPosition.nodeBefore, targetPosition.nodeAfter ); - viewWriter.move( viewWriter.createRangeOn( viewList ), targetPosition ); - - walker.position = breakPosition; - } - } - } else { - const nextViewList = injectedList.nextSibling; - - if ( nextViewList && ( nextViewList.is( 'ul' ) || nextViewList.is( 'ol' ) ) ) { - let lastSubChild = null; - - for ( const child of nextViewList.getChildren() ) { - const modelChild = mapper.toModelElement( child ); - - if ( modelChild && modelChild.getAttribute( 'listIndent' ) > modelItem.getAttribute( 'listIndent' ) ) { - lastSubChild = child; - } else { - break; - } - } - - if ( lastSubChild ) { - viewWriter.breakContainer( viewWriter.createPositionAfter( lastSubChild ) ); - viewWriter.move( viewWriter.createRangeOn( lastSubChild.parent ), viewWriter.createPositionAt( injectedItem, 'end' ) ); - } - } - } - - // Merge inserted view list with its possible neighbour lists. - mergeViewLists( viewWriter, injectedList, injectedList.nextSibling ); - mergeViewLists( viewWriter, injectedList.previousSibling, injectedList ); -} - // Helper function that takes all children of given `viewRemovedItem` and moves them in a correct place, according // to other given parameters. function hoistNestedLists( nextIndent, modelRemoveStartPosition, viewRemoveStartPosition, viewRemovedItem, conversionApi, model ) { @@ -1131,12 +989,3 @@ function hoistNestedLists( nextIndent, modelRemoveStartPosition, viewRemoveStart } } } - -// Helper function that for given `view.Position`, returns a `view.Position` that is after all `view.UIElement`s that -// are after given position. -// For example: -// foo^bar -// For position ^, a position before "bar" will be returned. -function positionAfterUiElements( viewPosition ) { - return viewPosition.getLastMatchingPosition( value => value.item.is( 'uiElement' ) ); -} diff --git a/src/listediting.js b/src/listediting.js index 0ead906..21d3ef2 100644 --- a/src/listediting.js +++ b/src/listediting.js @@ -18,6 +18,7 @@ import { cleanListItem, modelViewInsertion, modelViewChangeType, + modelViewMergeAfterChangeType, modelViewMergeAfter, modelViewRemove, modelViewSplitOnInsert, @@ -77,8 +78,9 @@ export default class ListEditing extends Plugin { data.downcastDispatcher.on( 'insert', modelViewSplitOnInsert, { priority: 'high' } ); data.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( editor.model ) ); - editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); - data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); + editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType, { priority: 'high' } ); + editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewMergeAfterChangeType, { priority: 'low' } ); + data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType ); editing.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); data.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); @@ -103,7 +105,7 @@ export default class ListEditing extends Plugin { editor.commands.add( 'indentList', new IndentCommand( editor, 'forward' ) ); editor.commands.add( 'outdentList', new IndentCommand( editor, 'backward' ) ); - const viewDocument = this.editor.editing.view.document; + const viewDocument = editing.view.document; // Overwrite default Enter key behavior. // If Enter key is pressed with selection collapsed in empty list item, outdent it instead of breaking it. diff --git a/src/todolistconverters.js b/src/todolistconverters.js index 42257dc..f7e1a8e 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -7,8 +7,53 @@ * @module list/todolistconverters */ -import { createCheckMarkElement } from './todolistutils'; +import { addTodoElementsToListItem, createCheckMarkElement, removeTodoElementsFromListItem } from './todolistutils'; +import { generateLiInUl, injectViewList } from './utils'; +/** + * A model-to-view converter for `listItem` model element insertion. + * + * It creates a `
            • ` (or `
                `) view structure out of a `listItem` model element, inserts it at the correct + * position, and merges the list with surrounding lists (if available). + * + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert + * @param {module:engine/model/model~Model} model Model instance. + * @returns {Function} Returns a conversion callback. + */ +export function modelViewInsertion( model ) { + return ( evt, data, conversionApi ) => { + const consumable = conversionApi.consumable; + + if ( !consumable.test( data.item, 'insert' ) || + !consumable.test( data.item, 'attribute:listType' ) || + !consumable.test( data.item, 'attribute:listIndent' ) + ) { + return; + } + + if ( data.item.getAttribute( 'listType' ) != 'todo' ) { + return; + } + + consumable.consume( data.item, 'insert' ); + consumable.consume( data.item, 'attribute:listType' ); + consumable.consume( data.item, 'attribute:listIndent' ); + + const viewWriter = conversionApi.writer; + const modelItem = data.item; + const viewItem = generateLiInUl( modelItem, conversionApi ); + + addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ); + injectViewList( modelItem, viewItem, conversionApi, model ); + }; +} + +/** + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute + * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. + * @param {Object} data Additional information about the change. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface. + */ export function modelViewTextInsertion( evt, data, conversionApi ) { const parent = data.range.start.parent; @@ -25,6 +70,30 @@ export function modelViewTextInsertion( evt, data, conversionApi ) { viewWriter.insert( viewPosition.offset ? viewPosition : viewPosition.getShiftedBy( 1 ), viewText ); } +/** + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute + * @param {module:engine/model/model~Model} model Model instance. + * @returns {Function} Returns a conversion callback. + */ +export function modelViewChangeType( model ) { + return ( evt, data, conversionApi ) => { + const viewItem = conversionApi.mapper.toViewElement( data.item ); + const viewWriter = conversionApi.writer; + + // Add or remove checkbox for toto list. + if ( data.attributeNewValue == 'todo' ) { + addTodoElementsToListItem( viewWriter, viewItem, data.item, model ); + } else if ( data.attributeOldValue == 'todo' ) { + removeTodoElementsFromListItem( viewWriter, viewItem, data.item, model ); + } + }; +} + +/** + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute + * @param {module:engine/model/model~Model} model Model instance. + * @returns {Function} Returns a conversion callback. + */ export function modelViewChangeChecked( model ) { return ( evt, data, conversionApi ) => { if ( !conversionApi.consumable.consume( data.item, 'attribute:listChecked' ) ) { diff --git a/src/todolistediting.js b/src/todolistediting.js index 7a9d502..0e800b8 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -13,8 +13,10 @@ import ListEditing from './listediting'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import { + modelViewInsertion, modelViewTextInsertion, - modelViewChangeChecked + modelViewChangeChecked, + modelViewChangeType } from './todolistconverters'; /** @@ -33,7 +35,7 @@ export default class TodoListEditing extends Plugin { */ init() { const editor = this.editor; - const editing = editor.editing; + const { editing } = editor; const viewDocument = editing.view.document; editor.model.schema.extend( 'listItem', { @@ -41,7 +43,10 @@ export default class TodoListEditing extends Plugin { } ); // Converters. + editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( editor.model ), { priority: 'high' } ); editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } ); + + editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( editor.model ) ); // Register command for todo list. diff --git a/src/utils.js b/src/utils.js index 3529c6f..7aa571f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -18,11 +18,204 @@ import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; */ export function createViewListItemElement( writer ) { const viewItem = writer.createContainerElement( 'li' ); + viewItem.getFillerOffset = getListItemFillerOffset; return viewItem; } +/** + * Helper function that creates a `
                ` or (`
                  `) structure out of given `modelItem` model `listItem` element. + * Then, it binds created view list item (
                1. ) with model `listItem` element. + * The function then returns created view list item (
                2. ). + * + * @param {module:engine/model/item~Item} modelItem Model list item. + * @param {module:engine/conversion/upcastdispatcher~UpcastConversionApi} conversionApi Conversion interface. + * @returns {module:engine/view/containerelement~ContainerElement} View list element. + */ +export function generateLiInUl( modelItem, conversionApi ) { + const mapper = conversionApi.mapper; + const viewWriter = conversionApi.writer; + const listType = modelItem.getAttribute( 'listType' ) == 'numbered' ? 'ol' : 'ul'; + const viewItem = createViewListItemElement( viewWriter ); + + const viewList = viewWriter.createContainerElement( listType, null ); + + viewWriter.insert( viewWriter.createPositionAt( viewList, 0 ), viewItem ); + + mapper.bindElements( modelItem, viewItem ); + + return viewItem; +} + +/** + * Helper function that takes model list item element `modelItem`, corresponding view list item element `injectedItem` + * that is not added to the view and is inside a view list element (`ul` or `ol`) and is that's list only child. + * The list is inserted at correct position (element breaking may be needed) and then merged with it's siblings. + * See comments below to better understand the algorithm. + * + * @param {module:engine/view/item~Item} modelItem Model list item. + * @param {module:engine/view/containerelement~ContainerElement} injectedItem + * @param {module:engine/conversion/upcastdispatcher~UpcastConversionApi} conversionApi Conversion interface. + * @param {module:engine/model/model~Model} model The model instance. + */ +export function injectViewList( modelItem, injectedItem, conversionApi, model ) { + const injectedList = injectedItem.parent; + const mapper = conversionApi.mapper; + const viewWriter = conversionApi.writer; + + // Position where view list will be inserted. + let insertPosition = mapper.toViewPosition( model.createPositionBefore( modelItem ) ); + + // 1. Find previous list item that has same or smaller indent. Basically we are looking for a first model item + // that is "parent" or "sibling" of injected model item. + // If there is no such list item, it means that injected list item is the first item in "its list". + const refItem = getSiblingListItem( modelItem.previousSibling, { + sameIndent: true, + smallerIndent: true, + listIndent: modelItem.getAttribute( 'listIndent' ) + } ); + const prevItem = modelItem.previousSibling; + + if ( refItem && refItem.getAttribute( 'listIndent' ) == modelItem.getAttribute( 'listIndent' ) ) { + // There is a list item with same indent - we found same-level sibling. + // Break the list after it. Inserted view item will be inserted in the broken space. + const viewItem = mapper.toViewElement( refItem ); + insertPosition = viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) ); + } else { + // There is no list item with same indent. Check previous model item. + if ( prevItem && prevItem.name == 'listItem' ) { + // If it is a list item, it has to have lower indent. + // It means that inserted item should be added to it as its nested item. + insertPosition = mapper.toViewPosition( model.createPositionAt( prevItem, 'end' ) ); + } else { + // Previous item is not a list item (or does not exist at all). + // Just map the position and insert the view item at mapped position. + insertPosition = mapper.toViewPosition( model.createPositionBefore( modelItem ) ); + } + } + + insertPosition = positionAfterUiElements( insertPosition ); + + // Insert the view item. + viewWriter.insert( insertPosition, injectedList ); + + // 2. Handle possible children of injected model item. + if ( prevItem && prevItem.name == 'listItem' ) { + const prevView = mapper.toViewElement( prevItem ); + + const walkerBoundaries = viewWriter.createRange( viewWriter.createPositionAt( prevView, 0 ), insertPosition ); + const walker = walkerBoundaries.getWalker( { ignoreElementEnd: true } ); + + for ( const value of walker ) { + if ( value.item.is( 'li' ) ) { + const breakPosition = viewWriter.breakContainer( viewWriter.createPositionBefore( value.item ) ); + const viewList = value.item.parent; + + const targetPosition = viewWriter.createPositionAt( injectedItem, 'end' ); + mergeViewLists( viewWriter, targetPosition.nodeBefore, targetPosition.nodeAfter ); + viewWriter.move( viewWriter.createRangeOn( viewList ), targetPosition ); + + walker.position = breakPosition; + } + } + } else { + const nextViewList = injectedList.nextSibling; + + if ( nextViewList && ( nextViewList.is( 'ul' ) || nextViewList.is( 'ol' ) ) ) { + let lastSubChild = null; + + for ( const child of nextViewList.getChildren() ) { + const modelChild = mapper.toModelElement( child ); + + if ( modelChild && modelChild.getAttribute( 'listIndent' ) > modelItem.getAttribute( 'listIndent' ) ) { + lastSubChild = child; + } else { + break; + } + } + + if ( lastSubChild ) { + viewWriter.breakContainer( viewWriter.createPositionAfter( lastSubChild ) ); + viewWriter.move( viewWriter.createRangeOn( lastSubChild.parent ), viewWriter.createPositionAt( injectedItem, 'end' ) ); + } + } + } + + // Merge inserted view list with its possible neighbour lists. + mergeViewLists( viewWriter, injectedList, injectedList.nextSibling ); + mergeViewLists( viewWriter, injectedList.previousSibling, injectedList ); +} + +/** + * Helper function that takes two parameters, that are expected to be view list elements, and merges them. + * The merge happen only if both parameters are list elements of the same types (the same element name and the same class attributes). + * + * @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter The writer instance. + * @param {module:engine/view/item~Item} firstList First element o compare. + * @param {module:engine/view/item~Item} secondList Second parameter to compare. + * @returns {module:engine/view/position~Position|null} Position after merge or `null` when there was no merge. + */ +export function mergeViewLists( viewWriter, firstList, secondList ) { + // Check if two lists are going to be merged. + if ( !firstList || !secondList || ( firstList.name != 'ul' && firstList.name != 'ol' ) ) { + return null; + } + + // Both parameters are list elements, so compare types now. + if ( firstList.name != secondList.name || firstList.getAttribute( 'class' ) !== secondList.getAttribute( 'class' ) ) { + return null; + } + + return viewWriter.mergeContainers( viewWriter.createPositionAfter( firstList ) ); +} + +/** + * Helper function that for given `view.Position`, returns a `view.Position` that is after all `view.UIElement`s that + * are after given position. + * + * For example: + * foo^bar + * For position ^, a position before "bar" will be returned. + * + * @param {module:engine/view/position~Position} viewPosition + * @returns {module:engine/view/position~Position} + */ +export function positionAfterUiElements( viewPosition ) { + return viewPosition.getLastMatchingPosition( value => value.item.is( 'uiElement' ) ); +} + +/** + * Helper function that seeks for a previous list item sibling of given model item which meets given criteria + * passed by an options object. + * + * @param {module:engine/model/item~Item} modelItem + * @param {Object} options Search criteria. + * @param {Boolean} [options.sameIndent=false] Whether sought sibling should have same indent. + * @param {Boolean} [options.smallerIndent=false] Whether sought sibling should have smaller indent. + * @param {Number} [options.listIndent] The reference indent. + * @returns {module:engine/model/item~Item|null} + */ +export function getSiblingListItem( modelItem, options ) { + const sameIndent = !!options.sameIndent; + const smallerIndent = !!options.smallerIndent; + const indent = options.listIndent; + + let item = modelItem; + + while ( item && item.name == 'listItem' ) { + const itemIndent = item.getAttribute( 'listIndent' ); + + if ( ( sameIndent && indent == itemIndent ) || ( smallerIndent && indent > itemIndent ) ) { + return item; + } + + item = item.previousSibling; + } + + return null; +} + /** * Helper method for creating an UI button and linking it with an appropriate command. * @@ -35,7 +228,6 @@ export function createViewListItemElement( writer ) { export function createUIComponent( editor, commandName, label, icon ) { editor.ui.componentFactory.add( commandName, locale => { const command = editor.commands.get( commandName ); - const buttonView = new ButtonView( locale ); buttonView.set( { From 4f562b4f8c415b60a2945e9a013b918261c0ad47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 5 Aug 2019 23:40:44 +0200 Subject: [PATCH 03/42] Added data pipeline conversion for todo lists. --- src/listediting.js | 3 +- src/todolistconverters.js | 76 ++++++++++++++++++++++++++++++++++++++- src/todolistediting.js | 6 +++- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/listediting.js b/src/listediting.js index 21d3ef2..3f240d3 100644 --- a/src/listediting.js +++ b/src/listediting.js @@ -80,7 +80,8 @@ export default class ListEditing extends Plugin { editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType, { priority: 'high' } ); editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewMergeAfterChangeType, { priority: 'low' } ); - data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType ); + data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType, { priority: 'high' } ); + data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewMergeAfterChangeType, { priority: 'low' } ); editing.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); data.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); diff --git a/src/todolistconverters.js b/src/todolistconverters.js index f7e1a8e..7cda480 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -49,7 +49,7 @@ export function modelViewInsertion( model ) { } /** - * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. * @param {Object} data Additional information about the change. * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface. @@ -70,6 +70,80 @@ export function modelViewTextInsertion( evt, data, conversionApi ) { viewWriter.insert( viewPosition.offset ? viewPosition : viewPosition.getShiftedBy( 1 ), viewText ); } +/** + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert + * @param {module:engine/model/model~Model} model Model instance. + * @returns {Function} Returns a conversion callback. + */ +export function dataModelViewInsertion( model ) { + return ( evt, data, conversionApi ) => { + const consumable = conversionApi.consumable; + + if ( !consumable.test( data.item, 'insert' ) || + !consumable.test( data.item, 'attribute:listType' ) || + !consumable.test( data.item, 'attribute:listIndent' ) + ) { + return; + } + + if ( data.item.getAttribute( 'listType' ) != 'todo' ) { + return; + } + + consumable.consume( data.item, 'insert' ); + consumable.consume( data.item, 'attribute:listType' ); + consumable.consume( data.item, 'attribute:listIndent' ); + + const viewWriter = conversionApi.writer; + const modelItem = data.item; + const viewItem = generateLiInUl( modelItem, conversionApi ); + + viewWriter.addClass( 'todo-list', viewItem.parent ); + + const label = viewWriter.createAttributeElement( 'label' ); + const checkbox = viewWriter.createEmptyElement( 'input', { + type: 'checkbox', + disabled: 'disabled', + class: 'todo-list__checkmark' + } ); + + if ( data.item.getAttribute( 'listChecked' ) ) { + viewWriter.setAttribute( 'checked', 'checked', checkbox ); + } + + viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkbox ); + viewWriter.wrap( viewWriter.createRangeOn( checkbox ), label ); + + injectViewList( modelItem, viewItem, conversionApi, model ); + }; +} + +/** + * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert + * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. + * @param {Object} data Additional information about the change. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface. + */ +export function dataModelViewTextInsertion( evt, data, conversionApi ) { + const parent = data.range.start.parent; + + if ( parent.name != 'listItem' || parent.getAttribute( 'listType' ) != 'todo' ) { + return; + } + + conversionApi.consumable.consume( data.item, 'insert' ); + + const viewWriter = conversionApi.writer; + const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); + const viewText = viewWriter.createText( data.item.data ); + const span = viewWriter.createAttributeElement( 'span', { class: 'todo-list__label' } ); + const label = viewWriter.createAttributeElement( 'label' ); + + viewWriter.insert( viewWriter.createPositionAt( viewPosition.parent, 'end' ), viewText ); + viewWriter.wrap( viewWriter.createRangeOn( viewText ), span ); + viewWriter.wrap( viewWriter.createRangeOn( viewText.parent ), label ); +} + /** * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute * @param {module:engine/model/model~Model} model Model instance. diff --git a/src/todolistediting.js b/src/todolistediting.js index 0e800b8..25c0e2b 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -14,7 +14,9 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import { modelViewInsertion, + dataModelViewInsertion, modelViewTextInsertion, + dataModelViewTextInsertion, modelViewChangeChecked, modelViewChangeType } from './todolistconverters'; @@ -35,7 +37,7 @@ export default class TodoListEditing extends Plugin { */ init() { const editor = this.editor; - const { editing } = editor; + const { editing, data } = editor; const viewDocument = editing.view.document; editor.model.schema.extend( 'listItem', { @@ -45,6 +47,8 @@ export default class TodoListEditing extends Plugin { // Converters. editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( editor.model ), { priority: 'high' } ); editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } ); + data.downcastDispatcher.on( 'insert:listItem', dataModelViewInsertion( editor.model ), { priority: 'high' } ); + data.downcastDispatcher.on( 'insert:$text', dataModelViewTextInsertion, { priority: 'high' } ); editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( editor.model ) ); From ca050396a51c48f28e042e24cd70e07c981a630a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 6 Aug 2019 09:50:43 +0200 Subject: [PATCH 04/42] Moved helpers from todolistutils to todolistconverters. --- src/todolistconverters.js | 49 +++++++++++++++++++++++++++++++++- src/todolistediting.js | 5 ++-- src/todolistutils.js | 56 --------------------------------------- 3 files changed, 51 insertions(+), 59 deletions(-) delete mode 100644 src/todolistutils.js diff --git a/src/todolistconverters.js b/src/todolistconverters.js index 7cda480..5daf5c0 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -7,7 +7,8 @@ * @module list/todolistconverters */ -import { addTodoElementsToListItem, createCheckMarkElement, removeTodoElementsFromListItem } from './todolistutils'; +/* global document */ + import { generateLiInUl, injectViewList } from './utils'; /** @@ -189,6 +190,52 @@ export function modelViewChangeChecked( model ) { }; } +function addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ) { + const isChecked = !!modelItem.getAttribute( 'listChecked' ); + + viewWriter.addClass( 'todo-list', viewItem.parent ); + + viewWriter.insert( + viewWriter.createPositionAt( viewItem, 0 ), + createCheckMarkElement( isChecked, viewWriter, isChecked => { + model.change( writer => writer.setAttribute( 'listChecked', isChecked, modelItem ) ); + } ) + ); +} + +function removeTodoElementsFromListItem( viewWriter, viewItem, modelItem, model ) { + viewWriter.removeClass( 'todo-list', viewItem.parent ); + viewWriter.remove( viewItem.getChild( 0 ) ); + model.change( writer => writer.removeAttribute( 'listChecked', modelItem ) ); +} + +function createCheckMarkElement( isChecked, viewWriter, onChange ) { + const uiElement = viewWriter.createUIElement( + 'label', + { + class: 'todo-list__checkmark', + contenteditable: false + }, + function( domDocument ) { + const domElement = this.toDomElement( domDocument ); + const checkbox = document.createElement( 'input' ); + + checkbox.type = 'checkbox'; + checkbox.checked = isChecked; + checkbox.addEventListener( 'change', evt => onChange( evt.target.checked ) ); + domElement.appendChild( checkbox ); + + return domElement; + } + ); + + if ( isChecked ) { + viewWriter.addClass( 'todo-list__checkmark_checked', uiElement ); + } + + return uiElement; +} + function findCheckmarkElement( viewWriter, parent ) { for ( const item of viewWriter.createRangeIn( parent ).getItems() ) { if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) ) { diff --git a/src/todolistediting.js b/src/todolistediting.js index 25c0e2b..d370ee2 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -14,8 +14,8 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import { modelViewInsertion, - dataModelViewInsertion, modelViewTextInsertion, + dataModelViewInsertion, dataModelViewTextInsertion, modelViewChangeChecked, modelViewChangeType @@ -51,6 +51,7 @@ export default class TodoListEditing extends Plugin { data.downcastDispatcher.on( 'insert:$text', dataModelViewTextInsertion, { priority: 'high' } ); editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); + editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( editor.model ) ); // Register command for todo list. @@ -172,7 +173,7 @@ function moveSelectionAfterCheckmark( writer, selection ) { // @private // @param {module:engine/view/uielement~UIElement} element function getListItemAncestor( element ) { - for ( const parent of element.getAncestors() ) { + for ( const parent of element.getAncestors( { parentFirst: true } ) ) { if ( parent.name == 'li' ) { return parent; } diff --git a/src/todolistutils.js b/src/todolistutils.js deleted file mode 100644 index c9da25b..0000000 --- a/src/todolistutils.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license - */ - -/** - * @module list/todolistutils - */ - -/* global document */ - -export function addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ) { - const isChecked = !!modelItem.getAttribute( 'listChecked' ); - - viewWriter.addClass( 'todo-list', viewItem.parent ); - - viewWriter.insert( - viewWriter.createPositionAt( viewItem, 0 ), - createCheckMarkElement( isChecked, viewWriter, isChecked => { - model.change( writer => writer.setAttribute( 'listChecked', isChecked, modelItem ) ); - } ) - ); -} - -export function removeTodoElementsFromListItem( viewWriter, viewItem, modelItem, model ) { - viewWriter.removeClass( 'todo-list', viewItem.parent ); - viewWriter.remove( viewItem.getChild( 0 ) ); - model.change( writer => writer.removeAttribute( 'listChecked', modelItem ) ); -} - -export function createCheckMarkElement( isChecked, viewWriter, onChange ) { - const uiElement = viewWriter.createUIElement( - 'label', - { - class: 'todo-list__checkmark', - contenteditable: false - }, - function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - const checkbox = document.createElement( 'input' ); - - checkbox.type = 'checkbox'; - checkbox.checked = isChecked; - checkbox.addEventListener( 'change', evt => onChange( evt.target.checked ) ); - domElement.appendChild( checkbox ); - - return domElement; - } - ); - - if ( isChecked ) { - viewWriter.addClass( 'todo-list__checkmark_checked', uiElement ); - } - - return uiElement; -} From 7d0c1d159d40fc594dcdddd97c5c206cdbb644c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 6 Aug 2019 13:53:01 +0200 Subject: [PATCH 05/42] Unify order of parameters in helper functions. --- src/todolistconverters.js | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index 5daf5c0..d0b695d 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -12,11 +12,6 @@ import { generateLiInUl, injectViewList } from './utils'; /** - * A model-to-view converter for `listItem` model element insertion. - * - * It creates a `
                    • ` (or `
                        `) view structure out of a `listItem` model element, inserts it at the correct - * position, and merges the list with surrounding lists (if available). - * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert * @param {module:engine/model/model~Model} model Model instance. * @returns {Function} Returns a conversion callback. @@ -44,7 +39,7 @@ export function modelViewInsertion( model ) { const modelItem = data.item; const viewItem = generateLiInUl( modelItem, conversionApi ); - addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ); + addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ); injectViewList( modelItem, viewItem, conversionApi, model ); }; } @@ -157,9 +152,9 @@ export function modelViewChangeType( model ) { // Add or remove checkbox for toto list. if ( data.attributeNewValue == 'todo' ) { - addTodoElementsToListItem( viewWriter, viewItem, data.item, model ); + addTodoElementsToListItem( data.item, viewItem, viewWriter, model ); } else if ( data.attributeOldValue == 'todo' ) { - removeTodoElementsFromListItem( viewWriter, viewItem, data.item, model ); + removeTodoElementsFromListItem( data.item, viewItem, viewWriter, model ); } }; } @@ -178,7 +173,7 @@ export function modelViewChangeChecked( model ) { const { mapper, writer: viewWriter } = conversionApi; const isChecked = !!data.item.getAttribute( 'listChecked' ); const viewItem = mapper.toViewElement( data.item ); - const uiElement = findCheckmarkElement( viewWriter, viewItem ); + const uiElement = findCheckmarkElement( viewItem, viewWriter ); viewWriter.insert( viewWriter.createPositionAfter( uiElement ), @@ -190,7 +185,7 @@ export function modelViewChangeChecked( model ) { }; } -function addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ) { +function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { const isChecked = !!modelItem.getAttribute( 'listChecked' ); viewWriter.addClass( 'todo-list', viewItem.parent ); @@ -203,7 +198,7 @@ function addTodoElementsToListItem( viewWriter, viewItem, modelItem, model ) { ); } -function removeTodoElementsFromListItem( viewWriter, viewItem, modelItem, model ) { +function removeTodoElementsFromListItem( modelItem, viewItem, viewWriter, model ) { viewWriter.removeClass( 'todo-list', viewItem.parent ); viewWriter.remove( viewItem.getChild( 0 ) ); model.change( writer => writer.removeAttribute( 'listChecked', modelItem ) ); @@ -236,8 +231,8 @@ function createCheckMarkElement( isChecked, viewWriter, onChange ) { return uiElement; } -function findCheckmarkElement( viewWriter, parent ) { - for ( const item of viewWriter.createRangeIn( parent ).getItems() ) { +function findCheckmarkElement( element, viewWriter ) { + for ( const item of viewWriter.createRangeIn( element ).getItems() ) { if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) ) { return item; } From 05dcbf58cf289b420eb9680f11cc54d010cc5e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 6 Aug 2019 13:53:41 +0200 Subject: [PATCH 06/42] Jump over checkbox element on left arrow key press. --- src/todolistediting.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/todolistediting.js b/src/todolistediting.js index d370ee2..716c760 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -98,6 +98,37 @@ export default class TodoListEditing extends Plugin { return hasChanged; } ); + + // Jump at the end of the previous node on left arrow key press, when selection is after the checkbox. + // + //

                        Foo

                        + //
                        • {}Bar
                        + // + // press: `<-` + // + //

                        Foo{}

                        + //
                        • Bar
                        + editor.keystrokes.set( 'arrowleft', ( evt, stop ) => { + const schema = editor.model.schema; + const selection = editor.model.document.selection; + + if ( !selection.isCollapsed ) { + return; + } + + const position = selection.getFirstPosition(); + const parent = position.parent; + + if ( parent.name === 'listItem' && parent.getAttribute( 'listType' ) == 'todo' && position.isAtStart ) { + stop(); + + const newRange = schema.getNearestSelectionRange( editor.model.createPositionBefore( parent ), 'backward' ); + + if ( newRange ) { + editor.model.change( writer => writer.setSelection( newRange ) ); + } + } + } ); } /** From 09a75086354972e292e0d232999e79a1eb28693d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 7 Aug 2019 08:50:03 +0200 Subject: [PATCH 07/42] Minor code reorganisation. --- src/todolistediting.js | 136 +++++++++++++++++++++-------------------- 1 file changed, 70 insertions(+), 66 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index 716c760..88b79c3 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -37,36 +37,46 @@ export default class TodoListEditing extends Plugin { */ init() { const editor = this.editor; - const { editing, data } = editor; + const { editing, data, model } = editor; const viewDocument = editing.view.document; - editor.model.schema.extend( 'listItem', { + model.schema.extend( 'listItem', { allowAttributes: [ 'listChecked' ] } ); // Converters. - editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( editor.model ), { priority: 'high' } ); + editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( model ), { priority: 'high' } ); editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } ); - data.downcastDispatcher.on( 'insert:listItem', dataModelViewInsertion( editor.model ), { priority: 'high' } ); + data.downcastDispatcher.on( 'insert:listItem', dataModelViewInsertion( model ), { priority: 'high' } ); data.downcastDispatcher.on( 'insert:$text', dataModelViewTextInsertion, { priority: 'high' } ); - editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( editor.model ) ); - - editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( editor.model ) ); + editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); + editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( model ) ); // Register command for todo list. editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); // Move selection after a checkbox element. - viewDocument.registerPostFixer( writer => moveUIElementsAfterCheckmark( writer, this._getChangedCheckmarkElements() ) ); + viewDocument.registerPostFixer( writer => moveUIElementsAfterCheckmark( writer, getChangedCheckmarkElements( editing.view ) ) ); // Move all uiElements after a checkbox element. viewDocument.registerPostFixer( writer => moveSelectionAfterCheckmark( writer, viewDocument.selection ) ); + // Jump at the end of the previous node on left arrow key press, when selection is after the checkbox. + // + //

                        Foo

                        + //
                        • {}Bar
                        + // + // press: `<-` + // + //

                        Foo{}

                        + //
                        • Bar
                        + editor.keystrokes.set( 'arrowleft', ( evt, stop ) => jumpOverCheckmarkOnLeftArrowKeyPress( stop, model ) ); + // Remove `listChecked` attribute when a host element is no longer a todo list item. const listItemsToFix = new Set(); - this.listenTo( editor.model, 'applyOperation', ( evt, args ) => { + this.listenTo( model, 'applyOperation', ( evt, args ) => { const operation = args[ 0 ]; if ( operation.type != 'changeAttribute' && operation.key != 'listType' && operation.oldValue == 'todoList' ) { @@ -86,7 +96,7 @@ export default class TodoListEditing extends Plugin { } } ); - editor.model.document.registerPostFixer( writer => { + model.document.registerPostFixer( writer => { let hasChanged = false; for ( const listItem of listItemsToFix ) { @@ -98,58 +108,6 @@ export default class TodoListEditing extends Plugin { return hasChanged; } ); - - // Jump at the end of the previous node on left arrow key press, when selection is after the checkbox. - // - //

                        Foo

                        - //
                        • {}Bar
                        - // - // press: `<-` - // - //

                        Foo{}

                        - //
                        • Bar
                        - editor.keystrokes.set( 'arrowleft', ( evt, stop ) => { - const schema = editor.model.schema; - const selection = editor.model.document.selection; - - if ( !selection.isCollapsed ) { - return; - } - - const position = selection.getFirstPosition(); - const parent = position.parent; - - if ( parent.name === 'listItem' && parent.getAttribute( 'listType' ) == 'todo' && position.isAtStart ) { - stop(); - - const newRange = schema.getNearestSelectionRange( editor.model.createPositionBefore( parent ), 'backward' ); - - if ( newRange ) { - editor.model.change( writer => writer.setSelection( newRange ) ); - } - } - } ); - } - - /** - * Gets list of all checkmark elements that are going to be rendered. - * - * @private - * @returns {Array.} - */ - _getChangedCheckmarkElements() { - const editingView = this.editor.editing.view; - const elements = []; - - for ( const element of Array.from( editingView._renderer.markedChildren ) ) { - for ( const item of editingView.createRangeIn( element ).getItems() ) { - if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) && !elements.includes( item ) ) { - elements.push( item ); - } - } - } - - return elements; } } @@ -161,7 +119,7 @@ function moveUIElementsAfterCheckmark( writer, uiElements ) { let hasChanged = false; for ( const uiElement of uiElements ) { - const listItem = getListItemAncestor( uiElement ); + const listItem = findListItemAncestor( uiElement ); const positionAtListItem = writer.createPositionAt( listItem, 0 ); const positionBeforeUiElement = writer.createPositionBefore( uiElement ); @@ -202,9 +160,55 @@ function moveSelectionAfterCheckmark( writer, selection ) { } // @private -// @param {module:engine/view/uielement~UIElement} element -function getListItemAncestor( element ) { - for ( const parent of element.getAncestors( { parentFirst: true } ) ) { +// @param {Function} stopKeyEvent +// @param {module:engine/model/model~Model} model +function jumpOverCheckmarkOnLeftArrowKeyPress( stopKeyEvent, model ) { + const schema = model.schema; + const selection = model.document.selection; + + if ( !selection.isCollapsed ) { + return; + } + + const position = selection.getFirstPosition(); + const parent = position.parent; + + if ( parent.name === 'listItem' && parent.getAttribute( 'listType' ) == 'todo' && position.isAtStart ) { + stopKeyEvent(); + + const newRange = schema.getNearestSelectionRange( model.createPositionBefore( parent ), 'backward' ); + + if ( newRange ) { + model.change( writer => writer.setSelection( newRange ) ); + } + } +} + +// Gets list of all checkmark elements that are going to be rendered. +// +// @private +// @returns {Array.} +function getChangedCheckmarkElements( editingView ) { + const elements = []; + + for ( const element of Array.from( editingView._renderer.markedChildren ) ) { + for ( const item of editingView.createRangeIn( element ).getItems() ) { + if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) && !elements.includes( item ) ) { + elements.push( item ); + } + } + } + + return elements; +} + +// Returns list item ancestor of given element. +// +// @private +// @param {module:engine/view/item~Item} item +// @returns {module:engine/view/element~Element} +function findListItemAncestor( item ) { + for ( const parent of item.getAncestors( { parentFirst: true } ) ) { if ( parent.name == 'li' ) { return parent; } From f4669ddbecbabea70f33cdd9917fa8e4abee8168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 7 Aug 2019 08:54:52 +0200 Subject: [PATCH 08/42] Renamed model attribute from listChecked to todoListChecked. --- src/listediting.js | 5 ++++- src/todolistconverters.js | 14 +++++++------- src/todolistediting.js | 12 ++++++------ 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/listediting.js b/src/listediting.js index 3f240d3..39322de 100644 --- a/src/listediting.js +++ b/src/listediting.js @@ -57,7 +57,7 @@ export default class ListEditing extends Plugin { // If there are blocks allowed inside list item, algorithms using `getSelectedBlocks()` will have to be modified. editor.model.schema.register( 'listItem', { inheritAllFrom: '$block', - allowAttributes: [ 'listType', 'listIndent', 'listChecked' ] + allowAttributes: [ 'listType', 'listIndent' ] } ); // Converters. @@ -175,6 +175,9 @@ export default class ListEditing extends Plugin { editor.keystrokes.set( 'Shift+Tab', getCommandExecuter( 'outdentList' ) ); } + /** + * @inheritDoc + */ afterInit() { const commands = this.editor.commands; diff --git a/src/todolistconverters.js b/src/todolistconverters.js index d0b695d..21b8674 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -103,7 +103,7 @@ export function dataModelViewInsertion( model ) { class: 'todo-list__checkmark' } ); - if ( data.item.getAttribute( 'listChecked' ) ) { + if ( data.item.getAttribute( 'todoListChecked' ) ) { viewWriter.setAttribute( 'checked', 'checked', checkbox ); } @@ -166,19 +166,19 @@ export function modelViewChangeType( model ) { */ export function modelViewChangeChecked( model ) { return ( evt, data, conversionApi ) => { - if ( !conversionApi.consumable.consume( data.item, 'attribute:listChecked' ) ) { + if ( !conversionApi.consumable.consume( data.item, 'attribute:todoListChecked' ) ) { return; } const { mapper, writer: viewWriter } = conversionApi; - const isChecked = !!data.item.getAttribute( 'listChecked' ); + const isChecked = !!data.item.getAttribute( 'todoListChecked' ); const viewItem = mapper.toViewElement( data.item ); const uiElement = findCheckmarkElement( viewItem, viewWriter ); viewWriter.insert( viewWriter.createPositionAfter( uiElement ), createCheckMarkElement( isChecked, viewWriter, isChecked => { - model.change( writer => writer.setAttribute( 'listChecked', isChecked, data.item ) ); + model.change( writer => writer.setAttribute( 'todoListChecked', isChecked, data.item ) ); } ) ); viewWriter.remove( uiElement ); @@ -186,14 +186,14 @@ export function modelViewChangeChecked( model ) { } function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { - const isChecked = !!modelItem.getAttribute( 'listChecked' ); + const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); viewWriter.addClass( 'todo-list', viewItem.parent ); viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), createCheckMarkElement( isChecked, viewWriter, isChecked => { - model.change( writer => writer.setAttribute( 'listChecked', isChecked, modelItem ) ); + model.change( writer => writer.setAttribute( 'todoListChecked', isChecked, modelItem ) ); } ) ); } @@ -201,7 +201,7 @@ function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { function removeTodoElementsFromListItem( modelItem, viewItem, viewWriter, model ) { viewWriter.removeClass( 'todo-list', viewItem.parent ); viewWriter.remove( viewItem.getChild( 0 ) ); - model.change( writer => writer.removeAttribute( 'listChecked', modelItem ) ); + model.change( writer => writer.removeAttribute( 'todoListChecked', modelItem ) ); } function createCheckMarkElement( isChecked, viewWriter, onChange ) { diff --git a/src/todolistediting.js b/src/todolistediting.js index 88b79c3..bedc6f2 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -41,7 +41,7 @@ export default class TodoListEditing extends Plugin { const viewDocument = editing.view.document; model.schema.extend( 'listItem', { - allowAttributes: [ 'listChecked' ] + allowAttributes: [ 'todoListChecked' ] } ); // Converters. @@ -51,7 +51,7 @@ export default class TodoListEditing extends Plugin { data.downcastDispatcher.on( 'insert:$text', dataModelViewTextInsertion, { priority: 'high' } ); editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); - editing.downcastDispatcher.on( 'attribute:listChecked:listItem', modelViewChangeChecked( model ) ); + editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) ); // Register command for todo list. editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); @@ -73,7 +73,7 @@ export default class TodoListEditing extends Plugin { //
                        • Bar
                        editor.keystrokes.set( 'arrowleft', ( evt, stop ) => jumpOverCheckmarkOnLeftArrowKeyPress( stop, model ) ); - // Remove `listChecked` attribute when a host element is no longer a todo list item. + // Remove `todoListChecked` attribute when a host element is no longer a todo list item. const listItemsToFix = new Set(); this.listenTo( model, 'applyOperation', ( evt, args ) => { @@ -81,7 +81,7 @@ export default class TodoListEditing extends Plugin { if ( operation.type != 'changeAttribute' && operation.key != 'listType' && operation.oldValue == 'todoList' ) { for ( const item of operation.range.getItems() ) { - if ( item.name == 'listItem' && item.hasAttribute( 'listChecked' ) ) { + if ( item.name == 'listItem' && item.hasAttribute( 'todoListChecked' ) ) { listItemsToFix.add( item ); } } @@ -90,7 +90,7 @@ export default class TodoListEditing extends Plugin { if ( operation.type == 'rename' && operation.oldName == 'listItem' ) { const item = operation.position.nodeAfter; - if ( item.hasAttribute( 'listChecked' ) ) { + if ( item.hasAttribute( 'todoListChecked' ) ) { listItemsToFix.add( item ); } } @@ -100,7 +100,7 @@ export default class TodoListEditing extends Plugin { let hasChanged = false; for ( const listItem of listItemsToFix ) { - writer.removeAttribute( 'listChecked', listItem ); + writer.removeAttribute( 'todoListChecked', listItem ); hasChanged = true; } From c787683da3fa0d76608a5715ed6bbc9d4d5e803c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 7 Aug 2019 11:26:16 +0200 Subject: [PATCH 09/42] Added data pipeline v -> m conversion. --- src/todolistconverters.js | 25 +++++++++++++++++++++++++ src/todolistediting.js | 5 ++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index 21b8674..b7eb740 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -185,6 +185,31 @@ export function modelViewChangeChecked( model ) { }; } +/** + * @see module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element + * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. + * @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values. + * @param {module:engine/conversion/upcastdispatcher~UpcastConversionApi} conversionApi Conversion interface to be used by the callback. + */ +export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) { + if ( data.viewItem.getAttribute( 'type' ) != 'checkbox' || data.modelCursor.parent.name != 'listItem' ) { + return; + } + + if ( !conversionApi.consumable.consume( data.viewItem, { name: true } ) ) { + return; + } + + const { writer } = conversionApi; + const modelItem = data.modelCursor.parent; + + writer.setAttribute( 'listType', 'todo', modelItem ); + + if ( data.viewItem.getAttribute( 'checked' ) == 'checked' ) { + writer.setAttribute( 'todoListChecked', true, modelItem ); + } +} + function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); diff --git a/src/todolistediting.js b/src/todolistediting.js index bedc6f2..1dfbb90 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -18,7 +18,8 @@ import { dataModelViewInsertion, dataModelViewTextInsertion, modelViewChangeChecked, - modelViewChangeType + modelViewChangeType, + dataViewModelCheckmarkInsertion } from './todolistconverters'; /** @@ -53,6 +54,8 @@ export default class TodoListEditing extends Plugin { editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) ); + data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } ); + // Register command for todo list. editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); From 0d9555878b9e36003fe54f5396ac2159231eca08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 7 Aug 2019 11:28:39 +0200 Subject: [PATCH 10/42] Removed redundant converters. --- src/listediting.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/listediting.js b/src/listediting.js index 39322de..cdc4e5f 100644 --- a/src/listediting.js +++ b/src/listediting.js @@ -80,15 +80,10 @@ export default class ListEditing extends Plugin { editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType, { priority: 'high' } ); editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewMergeAfterChangeType, { priority: 'low' } ); - data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType, { priority: 'high' } ); - data.downcastDispatcher.on( 'attribute:listType:listItem', modelViewMergeAfterChangeType, { priority: 'low' } ); editing.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); - data.downcastDispatcher.on( 'attribute:listIndent:listItem', modelViewChangeIndent( editor.model ) ); editing.downcastDispatcher.on( 'remove:listItem', modelViewRemove( editor.model ) ); editing.downcastDispatcher.on( 'remove', modelViewMergeAfter, { priority: 'low' } ); - data.downcastDispatcher.on( 'remove:listItem', modelViewRemove( editor.model ) ); - data.downcastDispatcher.on( 'remove', modelViewMergeAfter, { priority: 'low' } ); data.upcastDispatcher.on( 'element:ul', cleanList, { priority: 'high' } ); data.upcastDispatcher.on( 'element:ol', cleanList, { priority: 'high' } ); From 9353588137cb138f9f9323f667f97ea50190a881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 7 Aug 2019 11:32:09 +0200 Subject: [PATCH 11/42] Removed TodoList from List dependencies. --- src/list.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/list.js b/src/list.js index 583c084..89bcd09 100644 --- a/src/list.js +++ b/src/list.js @@ -9,7 +9,6 @@ import ListEditing from './listediting'; import ListUI from './listui'; -import TodoList from './todolist'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; @@ -26,7 +25,7 @@ export default class List extends Plugin { * @inheritDoc */ static get requires() { - return [ ListEditing, ListUI, TodoList ]; + return [ ListEditing, ListUI ]; } /** From 0d448f10e6eb3bd952263025495e1203789b0f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 7 Aug 2019 12:57:39 +0200 Subject: [PATCH 12/42] Added tests for TodoList and TodoListUI plugins. --- tests/todolist.js | 18 +++++++++++++ tests/todolistui.js | 61 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 tests/todolist.js create mode 100644 tests/todolistui.js diff --git a/tests/todolist.js b/tests/todolist.js new file mode 100644 index 0000000..b46fdd8 --- /dev/null +++ b/tests/todolist.js @@ -0,0 +1,18 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import TodoList from '../src/todolist'; +import TodoListEditing from '../src/todolistediting'; +import TodoListUI from '../src/todolistui'; + +describe( 'TodoList', () => { + it( 'should be named', () => { + expect( TodoList.pluginName ).to.equal( 'TodoList' ); + } ); + + it( 'should require TodoListEditing and TodoListUI', () => { + expect( TodoList.requires ).to.deep.equal( [ TodoListEditing, TodoListUI ] ); + } ); +} ); diff --git a/tests/todolistui.js b/tests/todolistui.js new file mode 100644 index 0000000..c67ac2d --- /dev/null +++ b/tests/todolistui.js @@ -0,0 +1,61 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals document */ + +import TodoListEditing from '../src/todolistediting'; +import TodoListUI from '../src/todolistui'; + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; +import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +describe( 'TodoListUI', () => { + let editor, model, button; + + beforeEach( () => { + const editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor.create( editorElement, { plugins: [ Paragraph, TodoListEditing, TodoListUI ] } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + + button = editor.ui.componentFactory.create( 'todoList' ); + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + it( 'should set up buttons for bulleted list and numbered list', () => { + expect( button ).to.be.instanceOf( ButtonView ); + } ); + + it( 'should execute proper commands when buttons are used', () => { + sinon.spy( editor, 'execute' ); + + button.fire( 'execute' ); + sinon.assert.calledWithExactly( editor.execute, 'todoList' ); + } ); + + it( 'should bind button to command', () => { + setData( model, '[]foo' ); + + const command = editor.commands.get( 'todoList' ); + + expect( button.isOn ).to.be.true; + expect( button.isEnabled ).to.be.true; + + command.value = false; + expect( button.isOn ).to.be.false; + + command.isEnabled = false; + expect( button.isEnabled ).to.be.false; + } ); +} ); From d8c6e23905cd26d9c2b973a663c5a9561f7c2f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 8 Aug 2019 00:34:18 +0200 Subject: [PATCH 13/42] Added TodoListEditing tests and some minor improvements. --- src/todolistconverters.js | 47 +--- src/todolistediting.js | 24 +- src/utils.js | 10 + tests/todolistediting.js | 575 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 613 insertions(+), 43 deletions(-) create mode 100644 tests/todolistediting.js diff --git a/src/todolistconverters.js b/src/todolistconverters.js index b7eb740..c6059f0 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -9,7 +9,7 @@ /* global document */ -import { generateLiInUl, injectViewList } from './utils'; +import { generateLiInUl, injectViewList, findInRange } from './utils'; /** * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert @@ -34,6 +34,7 @@ export function modelViewInsertion( model ) { consumable.consume( data.item, 'insert' ); consumable.consume( data.item, 'attribute:listType' ); consumable.consume( data.item, 'attribute:listIndent' ); + consumable.consume( data.item, 'attribute:todoListChecked' ); const viewWriter = conversionApi.writer; const modelItem = data.item; @@ -57,7 +58,9 @@ export function modelViewTextInsertion( evt, data, conversionApi ) { return; } - conversionApi.consumable.consume( data.item, 'insert' ); + if ( !conversionApi.consumable.consume( data.item, 'insert' ) ) { + return; + } const viewWriter = conversionApi.writer; const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); @@ -127,7 +130,9 @@ export function dataModelViewTextInsertion( evt, data, conversionApi ) { return; } - conversionApi.consumable.consume( data.item, 'insert' ); + if ( !conversionApi.consumable.consume( data.item, 'insert' ) ) { + return; + } const viewWriter = conversionApi.writer; const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); @@ -173,7 +178,8 @@ export function modelViewChangeChecked( model ) { const { mapper, writer: viewWriter } = conversionApi; const isChecked = !!data.item.getAttribute( 'todoListChecked' ); const viewItem = mapper.toViewElement( data.item ); - const uiElement = findCheckmarkElement( viewItem, viewWriter ); + const itemRange = viewWriter.createRangeIn( viewItem ); + const uiElement = findInRange( itemRange, item => item.is( 'uiElement' ) ? item : false ); viewWriter.insert( viewWriter.createPositionAfter( uiElement ), @@ -185,31 +191,6 @@ export function modelViewChangeChecked( model ) { }; } -/** - * @see module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element - * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. - * @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values. - * @param {module:engine/conversion/upcastdispatcher~UpcastConversionApi} conversionApi Conversion interface to be used by the callback. - */ -export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) { - if ( data.viewItem.getAttribute( 'type' ) != 'checkbox' || data.modelCursor.parent.name != 'listItem' ) { - return; - } - - if ( !conversionApi.consumable.consume( data.viewItem, { name: true } ) ) { - return; - } - - const { writer } = conversionApi; - const modelItem = data.modelCursor.parent; - - writer.setAttribute( 'listType', 'todo', modelItem ); - - if ( data.viewItem.getAttribute( 'checked' ) == 'checked' ) { - writer.setAttribute( 'todoListChecked', true, modelItem ); - } -} - function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); @@ -255,11 +236,3 @@ function createCheckMarkElement( isChecked, viewWriter, onChange ) { return uiElement; } - -function findCheckmarkElement( element, viewWriter ) { - for ( const item of viewWriter.createRangeIn( element ).getItems() ) { - if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) ) { - return item; - } - } -} diff --git a/src/todolistediting.js b/src/todolistediting.js index 1dfbb90..45b223f 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -18,10 +18,11 @@ import { dataModelViewInsertion, dataModelViewTextInsertion, modelViewChangeChecked, - modelViewChangeType, - dataViewModelCheckmarkInsertion + modelViewChangeType } from './todolistconverters'; +import { findInRange } from './utils'; + /** * @extends module:core/plugin~Plugin */ @@ -54,8 +55,6 @@ export default class TodoListEditing extends Plugin { editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) ); - data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } ); - // Register command for todo list. editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); @@ -153,8 +152,21 @@ function moveSelectionAfterCheckmark( writer, selection ) { const position = selection.getFirstPosition(); - if ( position.parent.name === 'li' && position.offset == 0 && position.nodeAfter && position.nodeAfter.is( 'uiElement' ) ) { - writer.setSelection( position.parent, 1 ); + if ( position.parent.name != 'li' || !position.parent.parent.hasClass( 'todo-list' ) ) { + return false; + } + + const parentEndPosition = writer.createPositionAt( position.parent, 'end' ); + const uiElement = findInRange( writer.createRange( position, parentEndPosition ), item => { + return ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) ) ? item : false; + } ); + + if ( uiElement && !position.isAfter( writer.createPositionBefore( uiElement ) ) ) { + const range = writer.createRange( writer.createPositionAfter( uiElement ), parentEndPosition ); + const text = findInRange( range, item => item.is( 'textProxy' ) ? item.textNode : false ); + const nextPosition = text ? writer.createPositionAt( text, 0 ) : parentEndPosition; + + writer.setSelection( nextPosition ); return true; } diff --git a/src/utils.js b/src/utils.js index 7aa571f..64160d9 100644 --- a/src/utils.js +++ b/src/utils.js @@ -216,6 +216,16 @@ export function getSiblingListItem( modelItem, options ) { return null; } +export function findInRange( range, comparator ) { + for ( const item of range.getItems() ) { + const result = comparator( item ); + + if ( result ) { + return result; + } + } +} + /** * Helper method for creating an UI button and linking it with an appropriate command. * diff --git a/tests/todolistediting.js b/tests/todolistediting.js new file mode 100644 index 0000000..0d345dc --- /dev/null +++ b/tests/todolistediting.js @@ -0,0 +1,575 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import TodoListEditing from '../src/todolistediting'; +import ListEditing from '../src/listediting'; +import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import ListCommand from '../src/listcommand'; + +import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; +import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; + +describe( 'TodoListEditing', () => { + let editor, model, modelDoc, modelRoot, view, viewDoc, viewRoot; + + beforeEach( () => { + return VirtualTestEditor + .create( { + plugins: [ TodoListEditing, Typing, BoldEditing ] + } ) + .then( newEditor => { + editor = newEditor; + + model = editor.model; + modelDoc = model.document; + modelRoot = modelDoc.getRoot(); + + view = editor.editing.view; + viewDoc = view.document; + viewRoot = viewDoc.getRoot(); + + model.schema.register( 'foo', { + allowWhere: '$block', + allowAttributes: [ 'listIndent', 'listType' ], + isBlock: true, + isObject: true + } ); + } ); + } ); + + it( 'should load ListEditing', () => { + expect( TodoListEditing.requires ).to.have.members( [ ListEditing ] ); + } ); + + it( 'should set proper schema rules', () => { + expect( model.schema.checkAttribute( [ '$root', 'listItem' ], 'todoListChecked' ) ).to.be.true; + } ); + + describe( 'command', () => { + it( 'should register todoList list command', () => { + const command = editor.commands.get( 'todoList' ); + + expect( command ).to.be.instanceOf( ListCommand ); + expect( command ).to.have.property( 'type', 'todo' ); + } ); + + it( 'should create todo list item and change to paragraph in normal usage flow', () => { + expect( getViewData( view ) ).to.equal( '

                        []

                        ' ); + expect( getModelData( model ) ).to.equal( '[]' ); + + editor.execute( 'todoList' ); + + expect( getModelData( model ) ).to.equal( '[]' ); + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • []
                          • ' + + '
                        ' + ); + + editor.execute( 'input', { text: 'a' } ); + + expect( getModelData( model ) ).to.equal( 'a[]' ); + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • a{}
                          • ' + + '
                        ' + ); + + editor.execute( 'input', { text: 'b' } ); + + expect( getModelData( model ) ).to.equal( 'ab[]' ); + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • ab{}
                          • ' + + '
                        ' + ); + + editor.execute( 'todoList' ); + + expect( getModelData( model ) ).to.equal( 'ab[]' ); + expect( getViewData( view ) ).to.equal( '

                        ab{}

                        ' ); + } ); + } ); + + describe( 'editing pipeline', () => { + it( 'should convert todo list item', () => { + setModelData( model, + '1' + + '2' + ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}1
                          • ' + + '
                        • 2
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert nested todo list items', () => { + setModelData( model, + '1.0' + + '2.1' + + '3.1' + + '4.2' + + '5.2' + + '6.1' + ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • ' + + '{}1.0' + + '
                            ' + + '
                          • 2.1
                            • ' + + '
                          • ' + + '3.1' + + '
                              ' + + '
                            • 4.2
                              • ' + + '
                            • 5.2
                              • ' + + '
                            ' + + '
                            • ' + + '
                          • 6.1
                            • ' + + '
                          ' + + '
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert todo list items mixed with bulleted list items', () => { + setModelData( model, + '1.0' + + '2.0' + + '3.1' + + '4.2' + + '5.1' + ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}1.0
                          • ' + + '
                        ' + + '
                          ' + + '
                        • 2.0' + + '
                            ' + + '
                          • ' + + '3.1' + + '
                              ' + + '
                            • 4.2
                              • ' + + '
                            ' + + '
                            • ' + + '
                          • 5.1
                            • ' + + '
                          ' + + '
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert todo list items mixed with numbered list items', () => { + setModelData( model, + '1.0' + + '2.0' + + '3.1' + + '4.2' + + '5.1' + ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}1.0
                          • ' + + '
                        ' + + '
                          ' + + '
                        1. 2.0' + + '
                            ' + + '
                          • ' + + '3.1' + + '
                              ' + + '
                            1. 4.2
                              2. ' + + '
                            ' + + '
                            • ' + + '
                          • 5.1
                            • ' + + '
                          ' + + '
                          2. ' + + '
                        ' + ); + } ); + + it( 'should properly convert list type change #1', () => { + setModelData( model, + '1.0' + + '[]2.0' + + '3.0' + ); + + editor.execute( 'todoList' ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        1. 1.0
                          2. ' + + '
                        ' + + '
                          ' + + '
                        • {}2.0
                          • ' + + '
                        ' + + '
                          ' + + '
                        1. 3.0
                          2. ' + + '
                        ' + ); + } ); + + it( 'should properly convert list type change #2', () => { + setModelData( model, + '1.0' + + '[]2.0' + + '3.0' + ); + + editor.execute( 'numberedList' ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • 1.0
                          • ' + + '
                        ' + + '
                          ' + + '
                        1. {}2.0
                          2. ' + + '
                        ' + + '
                          ' + + '
                        • 3.0
                          • ' + + '
                        ' + ); + } ); + + it( 'should properly convert list type change #3', () => { + setModelData( model, + '1.0' + + '[]2.0' + + '3.0' + ); + + editor.execute( 'bulletedList' ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • 1.0
                          • ' + + '
                        ' + + '
                          ' + + '
                        • {}2.0
                          • ' + + '
                        ' + + '
                          ' + + '
                        • 3.0
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert todoListChecked attribute change', () => { + setModelData( model, '1.0' ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}1.0
                          • ' + + '
                        ' + ); + + model.change( writer => { + writer.setAttribute( 'todoListChecked', true, modelRoot.getChild( 0 ) ); + } ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}1.0
                          • ' + + '
                        ' + ); + + model.change( writer => { + writer.setAttribute( 'todoListChecked', false, modelRoot.getChild( 0 ) ); + } ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}1.0
                          • ' + + '
                        ' + ); + } ); + + it( 'should be overwritable', () => { + editor.editing.downcastDispatcher.on( 'insert:listItem', ( evt, data, api ) => { + const { consumable, writer, mapper } = api; + + consumable.consume( data.item, 'insert' ); + consumable.consume( data.item, 'attribute:listType' ); + consumable.consume( data.item, 'attribute:listIndent' ); + + const insertPosition = mapper.toViewPosition( model.createPositionBefore( data.item ) ); + const element = writer.createContainerElement( 'test' ); + + mapper.bindElements( data.item, element ); + writer.insert( insertPosition, element ); + }, { priority: 'highest' } ); + + editor.editing.downcastDispatcher.on( 'insert:$text', ( evt, data, api ) => { + const { consumable, writer, mapper } = api; + + consumable.consume( data.item, 'insert' ); + + const insertPosition = mapper.toViewPosition( model.createPositionBefore( data.item ) ); + const element = writer.createText( data.item.data ); + + writer.insert( insertPosition, element ); + mapper.bindElements( data.item, element ); + }, { priority: 'highest' } ); + + editor.editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', ( evt, data, api ) => { + const { consumable, writer, mapper } = api; + + consumable.consume( data.item, 'attribute:todoListChecked' ); + + const viewElement = mapper.toViewElement( data.item ); + + writer.addClass( 'checked', viewElement ); + }, { priority: 'highest' } ); + + setModelData( model, 'Foo' ); + expect( getViewData( view ) ).to.equal( '{}Foo' ); + + model.change( writer => writer.setAttribute( 'todoListChecked', true, modelRoot.getChild( 0 ) ) ); + expect( getViewData( view ) ).to.equal( '{}Foo' ); + } ); + } ); + + describe( 'data pipeline', () => { + it( 'should convert todo list item', () => { + setModelData( model, + '1' + + '2' + ); + + expect( editor.getData() ).to.equal( + '
                          ' + + '
                        • ' + + '' + + '
                          • ' + + '
                        • ' + + '' + + '
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert nested todo list item', () => { + setModelData( model, + '1.0' + + '2.1' + ); + + expect( editor.getData() ).to.equal( + '
                          ' + + '
                        • ' + + '' + + '
                            ' + + '
                          • ' + + '' + + '
                            • ' + + '
                          ' + + '
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert todo list item mixed with bulleted list items', () => { + setModelData( model, + '1.0' + + '2.0' + + '3.1' + + '4.2' + + '5.1' + ); + + expect( editor.getData() ).to.equal( + '
                          ' + + '
                        • ' + + '' + + '
                          • ' + + '
                        ' + + '
                          ' + + '
                        • 2.0' + + '
                            ' + + '
                          • ' + + '' + + '
                              ' + + '
                            • 4.2
                              • ' + + '
                            ' + + '
                            • ' + + '
                          • ' + + '' + + '
                            • ' + + '
                          ' + + '
                          • ' + + '
                        ' + ); + } ); + + it( 'should convert todo list item mixed with numbered list items', () => { + setModelData( model, + '1.0' + + '2.0' + + '3.1' + + '4.2' + + '5.1' + ); + + expect( editor.getData() ).to.equal( + '
                          ' + + '
                        • ' + + '' + + '
                          • ' + + '
                        ' + + '
                          ' + + '
                        1. 2.0' + + '
                            ' + + '
                          • ' + + '' + + '
                              ' + + '
                            1. 4.2
                              2. ' + + '
                            ' + + '
                            • ' + + '
                          • ' + + '' + + '
                            • ' + + '
                          ' + + '
                          2. ' + + '
                        ' + ); + } ); + + it( 'should be overwritable', () => { + editor.data.downcastDispatcher.on( 'insert:listItem', ( evt, data, api ) => { + const { consumable, writer, mapper } = api; + + consumable.consume( data.item, 'insert' ); + consumable.consume( data.item, 'attribute:listType' ); + consumable.consume( data.item, 'attribute:listIndent' ); + + const element = writer.createContainerElement( 'test' ); + const insertPosition = mapper.toViewPosition( model.createPositionBefore( data.item ) ); + + writer.insert( insertPosition, element ); + mapper.bindElements( data.item, element ); + }, { priority: 'highest' } ); + + editor.data.downcastDispatcher.on( 'insert:$text', ( evt, data, api ) => { + const { consumable, writer, mapper } = api; + + consumable.consume( data.item, 'insert' ); + + const insertPosition = mapper.toViewPosition( model.createPositionBefore( data.item ) ); + const element = writer.createText( data.item.data ); + + writer.insert( insertPosition, element ); + mapper.bindElements( data.item, element ); + }, { priority: 'highest' } ); + + setModelData( model, 'Foo' ); + + expect( editor.getData() ).to.equal( 'Foo' ); + } ); + } ); + + describe( 'selection view post-fixer', () => { + it( 'should move selection after checkmark element to the first text node', () => { + setModelData( model, 'Foo' ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • {}Foo
                          • ' + + '
                        ' + ); + } ); + + it( 'should move selection after checkmark element when list item does not contain any text node', () => { + setModelData( model, '[]' ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • []
                          • ' + + '
                        ' + ); + } ); + } ); + + describe( 'uiElements view post-fixer', () => { + it( 'should move all UIElements from before a checkmark after the checkmark element', () => { + setModelData( model, 'foo' ); + + editor.conversion.for( 'downcast' ).markerToElement( { + model: 'foo', + view: ( data, writer ) => writer.createUIElement( 'something' ) + } ); + + editor.conversion.for( 'downcast' ).markerToHighlight( { + model: 'bar', + view: { classes: 'bar' } + } ); + + model.change( writer => { + writer.addMarker( 'foo', { + range: writer.createRangeIn( writer.createPositionAt( modelRoot.getChild( 0 ), 0 ) ), + usingOperation: false + } ); + + writer.addMarker( 'bar', { + range: writer.createRangeIn( modelRoot.getChild( 0 ) ), + usingOperation: false + } ); + + // VirtualTestEeditor does not render V to DOM, so we need to mock element market to be rendered + // because view post-fixer uses it. + view._renderer.markedChildren = new Set( [ viewRoot.getChild( 0 ).getChild( 0 ) ] ); + } ); + + expect( getViewData( view ) ).to.equal( + '
                          ' + + '
                        • ' + + '' + + '' + + '' + + '{}foo' + + '' + + '
                          • ' + + '
                        ' + ); + } ); + } ); +} ); From 5687d005638cddb7604c3b990e0d3ca1c70dc4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 8 Aug 2019 12:14:19 +0200 Subject: [PATCH 14/42] Added more tests. --- src/todolistediting.js | 39 +++++------ tests/todolistediting.js | 147 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 153 insertions(+), 33 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index 45b223f..15baf2c 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -81,14 +81,6 @@ export default class TodoListEditing extends Plugin { this.listenTo( model, 'applyOperation', ( evt, args ) => { const operation = args[ 0 ]; - if ( operation.type != 'changeAttribute' && operation.key != 'listType' && operation.oldValue == 'todoList' ) { - for ( const item of operation.range.getItems() ) { - if ( item.name == 'listItem' && item.hasAttribute( 'todoListChecked' ) ) { - listItemsToFix.add( item ); - } - } - } - if ( operation.type == 'rename' && operation.oldName == 'listItem' ) { const item = operation.position.nodeAfter; @@ -146,27 +138,31 @@ function moveUIElementsAfterCheckmark( writer, uiElements ) { // @param {module:engine/view/downcastwriter~DowncastWriter} writer // @param {module:engine/view/documentselection~DocumentSelection} selection function moveSelectionAfterCheckmark( writer, selection ) { - if ( !selection.isCollapsed ) { - return false; - } + const positionToChange = selection.getFirstPosition(); - const position = selection.getFirstPosition(); - - if ( position.parent.name != 'li' || !position.parent.parent.hasClass( 'todo-list' ) ) { + if ( positionToChange.parent.name != 'li' || !positionToChange.parent.parent.hasClass( 'todo-list' ) ) { return false; } - const parentEndPosition = writer.createPositionAt( position.parent, 'end' ); - const uiElement = findInRange( writer.createRange( position, parentEndPosition ), item => { + const parentEndPosition = writer.createPositionAt( positionToChange.parent, 'end' ); + const uiElement = findInRange( writer.createRange( positionToChange, parentEndPosition ), item => { return ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) ) ? item : false; } ); - if ( uiElement && !position.isAfter( writer.createPositionBefore( uiElement ) ) ) { - const range = writer.createRange( writer.createPositionAfter( uiElement ), parentEndPosition ); - const text = findInRange( range, item => item.is( 'textProxy' ) ? item.textNode : false ); + if ( uiElement && !positionToChange.isAfter( writer.createPositionBefore( uiElement ) ) ) { + const boundaries = writer.createRange( writer.createPositionAfter( uiElement ), parentEndPosition ); + const text = findInRange( boundaries, item => item.is( 'textProxy' ) ? item.textNode : false ); const nextPosition = text ? writer.createPositionAt( text, 0 ) : parentEndPosition; - writer.setSelection( nextPosition ); + let range; + + if ( selection.isCollapsed ) { + range = writer.createRange( nextPosition ); + } else { + range = writer.createRange( nextPosition, selection.getLastPosition() ); + } + + writer.setSelection( range, { isBackward: selection.isBackward } ); return true; } @@ -189,11 +185,10 @@ function jumpOverCheckmarkOnLeftArrowKeyPress( stopKeyEvent, model ) { const parent = position.parent; if ( parent.name === 'listItem' && parent.getAttribute( 'listType' ) == 'todo' && position.isAtStart ) { - stopKeyEvent(); - const newRange = schema.getNearestSelectionRange( model.createPositionBefore( parent ), 'backward' ); if ( newRange ) { + stopKeyEvent(); model.change( writer => writer.setSelection( newRange ) ); } } diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 0d345dc..0591ab4 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -6,12 +6,14 @@ import TodoListEditing from '../src/todolistediting'; import ListEditing from '../src/listediting'; import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting'; import Typing from '@ckeditor/ckeditor5-typing/src/typing'; import ListCommand from '../src/listcommand'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; +import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; describe( 'TodoListEditing', () => { let editor, model, modelDoc, modelRoot, view, viewDoc, viewRoot; @@ -19,7 +21,7 @@ describe( 'TodoListEditing', () => { beforeEach( () => { return VirtualTestEditor .create( { - plugins: [ TodoListEditing, Typing, BoldEditing ] + plugins: [ TodoListEditing, Typing, BoldEditing, BlockQuoteEditing ] } ) .then( newEditor => { editor = newEditor; @@ -294,6 +296,14 @@ describe( 'TodoListEditing', () => { ); } ); + it( 'should remove todoListChecked attribute when checked todoListItem is changed to regular list item', () => { + setModelData( model, 'fo[]o' ); + + editor.execute( 'bulletedList' ); + + expect( getModelData( model ) ).to.equal( 'fo[]o' ); + } ); + it( 'should be overwritable', () => { editor.editing.downcastDispatcher.on( 'insert:listItem', ( evt, data, api ) => { const { consumable, writer, mapper } = api; @@ -527,49 +537,164 @@ describe( 'TodoListEditing', () => { '
                    ' ); } ); + + it( 'should move start of none-collapsed selection after checkmark element to the first text node', () => { + setModelData( model, '[Foo]' ); + + expect( getViewData( view ) ).to.equal( + '
                      ' + + '
                    • {Foo}
                      • ' + + '
                    ' + ); + } ); + + it( 'should move start of none-collapsed, backward selection after checkmark element to the first text node', () => { + setModelData( model, '[Foo]', { lastRangeBackward: true } ); + + expect( getViewData( view ) ).to.equal( + '
                      ' + + '
                    • {Foo}
                      • ' + + '
                    ' + ); + } ); } ); describe( 'uiElements view post-fixer', () => { it( 'should move all UIElements from before a checkmark after the checkmark element', () => { - setModelData( model, 'foo' ); + setModelData( model, + 'foo' + + 'bar' + ); editor.conversion.for( 'downcast' ).markerToElement( { - model: 'foo', - view: ( data, writer ) => writer.createUIElement( 'something' ) + model: 'element1', + view: ( data, writer ) => writer.createUIElement( 'element1' ) + } ); + + editor.conversion.for( 'downcast' ).markerToElement( { + model: 'element2', + view: ( data, writer ) => writer.createUIElement( 'element2' ) } ); editor.conversion.for( 'downcast' ).markerToHighlight( { - model: 'bar', - view: { classes: 'bar' } + model: 'highlight', + view: { classes: 'highlight' } } ); model.change( writer => { - writer.addMarker( 'foo', { + writer.addMarker( 'element1', { range: writer.createRangeIn( writer.createPositionAt( modelRoot.getChild( 0 ), 0 ) ), usingOperation: false } ); - writer.addMarker( 'bar', { + writer.addMarker( 'element2', { + range: writer.createRangeIn( writer.createPositionAt( modelRoot.getChild( 0 ), 0 ) ), + usingOperation: false + } ); + + writer.addMarker( 'highlight', { range: writer.createRangeIn( modelRoot.getChild( 0 ) ), usingOperation: false } ); // VirtualTestEeditor does not render V to DOM, so we need to mock element market to be rendered // because view post-fixer uses it. - view._renderer.markedChildren = new Set( [ viewRoot.getChild( 0 ).getChild( 0 ) ] ); + view._renderer.markedChildren = new Set( [ + viewRoot.getChild( 0 ).getChild( 0 ), + viewRoot.getChild( 0 ).getChild( 1 ) + ] ); } ); expect( getViewData( view ) ).to.equal( '
                      ' + '
                    • ' + - '' + + '' + '' + - '' + + '' + + '' + '{}foo' + '' + '
                      • ' + + '
                    • ' + + 'bar' + + '
                      • ' + '
                    ' ); } ); } ); + + describe( 'todoListChecked attribute model post-fixer', () => { + it( 'should remove todoListChecked attribute when checked todoListItem is renamed', () => { + setModelData( model, 'fo[]o' ); + + editor.execute( 'todoList' ); + + expect( getModelData( model ) ).to.equal( 'fo[]o' ); + } ); + } ); + + describe( 'leftArrow key handling', () => { + let domEvtDataStub; + + beforeEach( () => { + domEvtDataStub = { + keyCode: getCode( 'arrowLeft' ), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy(), + domTarget: { + ownerDocument: { + defaultView: { + getSelection: () => ( { rangeCount: 0 } ) + } + } + } + }; + } ); + + it( 'should jump at the end of the previous node when selection is after checkmark element', () => { + setModelData( model, + '
                    foo
                    ' + + '[]bar' + ); + + viewDoc.fire( 'keydown', domEvtDataStub ); + + expect( getModelData( model ) ).to.equal( + '
                    foo[]
                    ' + + 'bar' + ); + + sinon.assert.calledOnce( domEvtDataStub.preventDefault ); + sinon.assert.calledOnce( domEvtDataStub.stopPropagation ); + } ); + + it( 'should do nothing when list item is a first block element in the root', () => { + setModelData( model, '[]bar' ); + + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.notCalled( domEvtDataStub.preventDefault ); + sinon.assert.notCalled( domEvtDataStub.stopPropagation ); + + expect( getModelData( model ) ).to.equal( '[]bar' ); + } ); + + it( 'should do nothing when selection is not collapsed', () => { + setModelData( model, '[bar]' ); + + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.notCalled( domEvtDataStub.preventDefault ); + sinon.assert.notCalled( domEvtDataStub.stopPropagation ); + } ); + + it( 'should do nothing when selection is not at the beginning list item', () => { + setModelData( model, 'b[]ar' ); + + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.notCalled( domEvtDataStub.preventDefault ); + sinon.assert.notCalled( domEvtDataStub.stopPropagation ); + } ); + } ); } ); From 4b3e6fe44314b5e8313d3f34d6f868cba2deabc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 8 Aug 2019 16:55:26 +0200 Subject: [PATCH 15/42] Introduced command for toggling checked state on todo list. --- src/todolistcheckedcommand.js | 81 +++++++++++++ tests/todolistcheckedcommand.js | 208 ++++++++++++++++++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 src/todolistcheckedcommand.js create mode 100644 tests/todolistcheckedcommand.js diff --git a/src/todolistcheckedcommand.js b/src/todolistcheckedcommand.js new file mode 100644 index 0000000..bcbeaab --- /dev/null +++ b/src/todolistcheckedcommand.js @@ -0,0 +1,81 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module list/todolistcheckedcommand + */ + +import Command from '@ckeditor/ckeditor5-core/src/command'; + +const attributeKey = 'todoListChecked'; + +/** + * @extends module:core/command~Command + */ +export default class TodoListCheckedCommand extends Command { + /** + * @param {module:core/editor/editor~Editor} editor + */ + constructor( editor ) { + super( editor ); + + this._selectedElements = []; + + /** + * Flag indicating whether the command is active. The command is active when the + * {@link module:engine/model/selection~Selection#hasAttribute selection has the attribute} which means that: + * + * * If the selection is not empty – That the attribute is set on the first node in the selection that allows this attribute. + * * If the selection is empty – That the selection has the attribute itself (which means that newly typed + * text will have this attribute, too). + * + * @observable + * @readonly + * @member {Boolean} #value + */ + } + + /** + * Updates the command's {@link #value} and {@link #isEnabled} based on the current selection. + */ + refresh() { + this._selectedElements = this._getSelectedItems(); + this.value = this._selectedElements.every( element => !!element.getAttribute( 'todoListChecked' ) ); + this.isEnabled = !!this._selectedElements.length; + } + + _getSelectedItems() { + const model = this.editor.model; + const schema = model.schema; + + const selectionRange = model.document.selection.getFirstRange(); + const startElement = selectionRange.start.parent; + const elements = []; + + if ( schema.checkAttribute( startElement, attributeKey ) ) { + elements.push( startElement ); + } + + for ( const item of selectionRange.getItems() ) { + if ( schema.checkAttribute( item, attributeKey ) && !elements.includes( item ) ) { + elements.push( item ); + } + } + + return elements; + } + + execute() { + this.editor.model.change( writer => { + for ( const element of this._selectedElements ) { + if ( !this.value ) { + writer.setAttribute( attributeKey, true, element ); + } else { + writer.removeAttribute( attributeKey, element ); + } + } + } ); + } +} diff --git a/tests/todolistcheckedcommand.js b/tests/todolistcheckedcommand.js new file mode 100644 index 0000000..72b3def --- /dev/null +++ b/tests/todolistcheckedcommand.js @@ -0,0 +1,208 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import TodoListEditing from '../src/todolistediting'; +import TodoListCheckedCommand from '../src/todolistcheckedcommand'; + +import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; +import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +describe( 'TodoListCheckedCommand', () => { + let editor, model, command; + + beforeEach( () => { + return ModelTestEditor + .create( { + plugins: [ TodoListEditing ] + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + command = new TodoListCheckedCommand( editor ); + } ); + } ); + + describe( 'value', () => { + it( 'should be false when selection is in not checked element', () => { + setModelData( model, 'ab[]c' ); + + expect( command.value ).to.equal( false ); + } ); + + it( 'should be true when selection is in checked element', () => { + setModelData( model, 'ab[]c' ); + + expect( command.value ).to.equal( true ); + } ); + + it( 'should be false when at least one selected element is not checked', () => { + setModelData( model, + 'ab[c' + + 'abc' + + 'abc' + + 'ab]c' + ); + + expect( command.value ).to.equal( false ); + } ); + + it( 'should be true when all selected elements are checked', () => { + setModelData( model, + 'ab[c' + + 'abc' + + 'abc' + + 'ab]c' + ); + + expect( command.value ).to.equal( true ); + } ); + } ); + + describe( 'isEnabled', () => { + it( 'should be enabled when selection is inside todo list item', () => { + setModelData( model, 'a[b]c' ); + + expect( command.isEnabled ).to.equal( true ); + } ); + + it( 'should be disabled when selection is not inside todo list item', () => { + setModelData( model, 'a[b]c' ); + + expect( command.isEnabled ).to.equal( false ); + } ); + + it( 'should be enabled when at least one todo list item is selected', () => { + setModelData( model, + 'a[bc' + + 'abc' + + 'ab]c' + ); + + expect( command.isEnabled ).to.equal( true ); + } ); + + it( 'should be enabled when none todo list item is selected', () => { + setModelData( model, + 'a[bc' + + 'abc' + + 'a]bc' + ); + + expect( command.isEnabled ).to.equal( false ); + } ); + } ); + + describe( 'execute()', () => { + it( 'should toggle checked state on todo list item when collapsed selection is inside this item', () => { + setModelData( model, 'b[]ar' ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'b[]ar' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'b[]ar' + ); + } ); + + it( 'should toggle checked state on todo list item when non-collapsed selection is inside this item', () => { + setModelData( model, 'b[a]r' ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'b[a]r' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'b[a]r' + ); + } ); + + it( 'should toggle state on multiple items', () => { + setModelData( model, + 'abc[' + + 'def' + + ']ghi' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'abc[' + + 'def' + + ']ghi' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'abc[' + + 'def' + + ']ghi' + ); + } ); + + it( 'should toggle state on multiple items mixed with non list item', () => { + setModelData( model, + 'a[bc' + + 'def' + + 'ghi' + + 'jkl' + + 'mn]o' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'a[bc' + + 'def' + + 'ghi' + + 'jkl' + + 'mn]o' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'a[bc' + + 'def' + + 'ghi' + + 'jkl' + + 'mn]o' + ); + } ); + + it( 'should mark all selected items as checked when at least one selected item is not checked', () => { + setModelData( model, + 'abc[' + + 'def' + + ']ghi' + ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( + 'abc[' + + 'def' + + ']ghi' + ); + } ); + + it( 'should do nothing when there is no elements to toggle attribute', () => { + setModelData( model, 'b[]ar' ); + + command.execute(); + + expect( getModelData( model ) ).to.equal( 'b[]ar' ); + } ); + } ); +} ); From 1dd136b1f537286b7fb31f99e863817bb2367ff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 8 Aug 2019 16:55:44 +0200 Subject: [PATCH 16/42] Minor improvement. --- src/todolistediting.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index 15baf2c..5cbae0c 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -113,7 +113,7 @@ function moveUIElementsAfterCheckmark( writer, uiElements ) { let hasChanged = false; for ( const uiElement of uiElements ) { - const listItem = findListItemAncestor( uiElement ); + const listItem = findViewListItemAncestor( uiElement ); const positionAtListItem = writer.createPositionAt( listItem, 0 ); const positionBeforeUiElement = writer.createPositionBefore( uiElement ); @@ -217,7 +217,7 @@ function getChangedCheckmarkElements( editingView ) { // @private // @param {module:engine/view/item~Item} item // @returns {module:engine/view/element~Element} -function findListItemAncestor( item ) { +function findViewListItemAncestor( item ) { for ( const parent of item.getAncestors( { parentFirst: true } ) ) { if ( parent.name == 'li' ) { return parent; From 7fd081abfde19d83cfc282872514bd99e9c1840b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 9 Aug 2019 00:23:05 +0200 Subject: [PATCH 17/42] Registered TodoListCheckCommand in TodoListEditing. --- ...olistcheckedcommand.js => todolistcheckcommand.js} | 2 +- src/todolistediting.js | 11 +++++++---- ...olistcheckedcommand.js => todolistcheckcommand.js} | 6 +++--- 3 files changed, 11 insertions(+), 8 deletions(-) rename src/{todolistcheckedcommand.js => todolistcheckcommand.js} (97%) rename tests/{todolistcheckedcommand.js => todolistcheckcommand.js} (97%) diff --git a/src/todolistcheckedcommand.js b/src/todolistcheckcommand.js similarity index 97% rename from src/todolistcheckedcommand.js rename to src/todolistcheckcommand.js index bcbeaab..25a550e 100644 --- a/src/todolistcheckedcommand.js +++ b/src/todolistcheckcommand.js @@ -14,7 +14,7 @@ const attributeKey = 'todoListChecked'; /** * @extends module:core/command~Command */ -export default class TodoListCheckedCommand extends Command { +export default class TodoListCheckCommand extends Command { /** * @param {module:core/editor/editor~Editor} editor */ diff --git a/src/todolistediting.js b/src/todolistediting.js index 5cbae0c..892ea28 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -9,6 +9,7 @@ import ListCommand from './listcommand'; import ListEditing from './listediting'; +import TodoListCheckCommand from './todolistcheckcommand'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; @@ -42,11 +43,16 @@ export default class TodoListEditing extends Plugin { const { editing, data, model } = editor; const viewDocument = editing.view.document; + // Extend schema. model.schema.extend( 'listItem', { allowAttributes: [ 'todoListChecked' ] } ); - // Converters. + // Register commands. + editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); + editor.commands.add( 'todoListCheck', new TodoListCheckCommand( editor ) ); + + // Define converters. editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( model ), { priority: 'high' } ); editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } ); data.downcastDispatcher.on( 'insert:listItem', dataModelViewInsertion( model ), { priority: 'high' } ); @@ -55,9 +61,6 @@ export default class TodoListEditing extends Plugin { editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) ); - // Register command for todo list. - editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); - // Move selection after a checkbox element. viewDocument.registerPostFixer( writer => moveUIElementsAfterCheckmark( writer, getChangedCheckmarkElements( editing.view ) ) ); diff --git a/tests/todolistcheckedcommand.js b/tests/todolistcheckcommand.js similarity index 97% rename from tests/todolistcheckedcommand.js rename to tests/todolistcheckcommand.js index 72b3def..bff0c5f 100644 --- a/tests/todolistcheckedcommand.js +++ b/tests/todolistcheckcommand.js @@ -4,12 +4,12 @@ */ import TodoListEditing from '../src/todolistediting'; -import TodoListCheckedCommand from '../src/todolistcheckedcommand'; +import TodoListCheckCommand from '../src/todolistcheckcommand'; import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; -describe( 'TodoListCheckedCommand', () => { +describe( 'TodoListCheckCommand', () => { let editor, model, command; beforeEach( () => { @@ -20,7 +20,7 @@ describe( 'TodoListCheckedCommand', () => { .then( newEditor => { editor = newEditor; model = editor.model; - command = new TodoListCheckedCommand( editor ); + command = new TodoListCheckCommand( editor ); } ); } ); From 0662dcbe5d6f0ce9de1a403a75060334160d65ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 9 Aug 2019 00:24:04 +0200 Subject: [PATCH 18/42] Small refactoring of todo list converters. --- src/todolistconverters.js | 44 ++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index c6059f0..e1cffd7 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -10,6 +10,7 @@ /* global document */ import { generateLiInUl, injectViewList, findInRange } from './utils'; +import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; /** * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert @@ -179,29 +180,20 @@ export function modelViewChangeChecked( model ) { const isChecked = !!data.item.getAttribute( 'todoListChecked' ); const viewItem = mapper.toViewElement( data.item ); const itemRange = viewWriter.createRangeIn( viewItem ); - const uiElement = findInRange( itemRange, item => item.is( 'uiElement' ) ? item : false ); - - viewWriter.insert( - viewWriter.createPositionAfter( uiElement ), - createCheckMarkElement( isChecked, viewWriter, isChecked => { - model.change( writer => writer.setAttribute( 'todoListChecked', isChecked, data.item ) ); - } ) - ); - viewWriter.remove( uiElement ); + const oldCheckmarkElement = findInRange( itemRange, item => item.is( 'uiElement' ) ? item : false ); + const newCheckmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, model ); + + viewWriter.insert( viewWriter.createPositionAfter( oldCheckmarkElement ), newCheckmarkElement ); + viewWriter.remove( oldCheckmarkElement ); }; } function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); + const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, model ); viewWriter.addClass( 'todo-list', viewItem.parent ); - - viewWriter.insert( - viewWriter.createPositionAt( viewItem, 0 ), - createCheckMarkElement( isChecked, viewWriter, isChecked => { - model.change( writer => writer.setAttribute( 'todoListChecked', isChecked, modelItem ) ); - } ) - ); + viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); } function removeTodoElementsFromListItem( modelItem, viewItem, viewWriter, model ) { @@ -210,7 +202,7 @@ function removeTodoElementsFromListItem( modelItem, viewItem, viewWriter, model model.change( writer => writer.removeAttribute( 'todoListChecked', modelItem ) ); } -function createCheckMarkElement( isChecked, viewWriter, onChange ) { +function createCheckmarkElement( modelItem, viewWriter, isChecked, model ) { const uiElement = viewWriter.createUIElement( 'label', { @@ -218,12 +210,22 @@ function createCheckMarkElement( isChecked, viewWriter, onChange ) { contenteditable: false }, function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - const checkbox = document.createElement( 'input' ); + const checkbox = createElement( document, 'input', { type: 'checkbox', } ); - checkbox.type = 'checkbox'; checkbox.checked = isChecked; - checkbox.addEventListener( 'change', evt => onChange( evt.target.checked ) ); + + checkbox.addEventListener( 'change', evt => { + model.change( writer => { + if ( evt.target.checked ) { + writer.setAttribute( 'todoListChecked', true, modelItem ); + } else { + writer.removeAttribute( 'todoListChecked', modelItem ); + } + } ); + } ); + + const domElement = this.toDomElement( domDocument ); + domElement.appendChild( checkbox ); return domElement; From 8d45c1f40d5f4838098cd18e5cb3badd3da2cbec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 9 Aug 2019 00:24:30 +0200 Subject: [PATCH 19/42] Added very basic manual test for todo lists. --- tests/manual/todo-list.html | 22 ++++++++++++++++++++++ tests/manual/todo-list.js | 28 ++++++++++++++++++++++++++++ tests/manual/todo-list.md | 0 3 files changed, 50 insertions(+) create mode 100644 tests/manual/todo-list.html create mode 100644 tests/manual/todo-list.js create mode 100644 tests/manual/todo-list.md diff --git a/tests/manual/todo-list.html b/tests/manual/todo-list.html new file mode 100644 index 0000000..bc4914d --- /dev/null +++ b/tests/manual/todo-list.html @@ -0,0 +1,22 @@ +
                    +

                    This is a test for list feature.

                    +

                    Some more text for testing.

                    +
                      +
                    • Bullet list item 1
                      • +
                    • Bullet list item 2
                      • +
                    • Bullet list item 3
                      • +
                    • Bullet list item 4
                      • +
                    • Bullet list item 5
                      • +
                    • Bullet list item 6
                      • +
                    • Bullet list item 7
                      • +
                    • Bullet list item 8
                      • +
                    +

                    Paragraph.

                    +

                    Another testing paragraph.

                    +
                      +
                    1. Numbered list item 1
                      2. +
                    +
                      +
                    • Another bullet list
                      • +
                    +
                    diff --git a/tests/manual/todo-list.js b/tests/manual/todo-list.js new file mode 100644 index 0000000..fa94cdc --- /dev/null +++ b/tests/manual/todo-list.js @@ -0,0 +1,28 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard'; +import List from '../../src/list'; +import TodoList from '../../src/todolist'; + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ Enter, Typing, Heading, Paragraph, Undo, List, TodoList, Clipboard ], + toolbar: [ 'heading', '|', 'bulletedList', 'numberedList', 'todoList', '|', 'undo', 'redo' ] + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/todo-list.md b/tests/manual/todo-list.md new file mode 100644 index 0000000..e69de29 From 49c9dbdbf5dcf009d7e0a14a61e59e295c7bad8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 9 Aug 2019 00:28:27 +0200 Subject: [PATCH 20/42] Added toggling check state of selected todo list items on keystroke. --- src/todolistediting.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/todolistediting.js b/src/todolistediting.js index 892ea28..b5d1539 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -78,6 +78,9 @@ export default class TodoListEditing extends Plugin { //
                    • Bar
                    editor.keystrokes.set( 'arrowleft', ( evt, stop ) => jumpOverCheckmarkOnLeftArrowKeyPress( stop, model ) ); + // Toggle check state of selected todo list items on keystroke. + editor.keystrokes.set( 'Ctrl+space', () => editor.execute( 'todoListCheck' ) ); + // Remove `todoListChecked` attribute when a host element is no longer a todo list item. const listItemsToFix = new Set(); From 9ae9e5a86e137f9d19382f22869eae2ea73fac32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Sun, 11 Aug 2019 17:08:19 +0200 Subject: [PATCH 21/42] Improved schema validation of todoListChecked attribute. --- src/todolistediting.js | 9 +++++++++ tests/todolistcheckcommand.js | 8 ++++---- tests/todolistediting.js | 11 ++++++++++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index b5d1539..40a8193 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -48,6 +48,15 @@ export default class TodoListEditing extends Plugin { allowAttributes: [ 'todoListChecked' ] } ); + // Disallow todoListChecked attribute on other nodes than listItem with todo listType. + model.schema.addAttributeCheck( ( context, attributeName ) => { + const item = context.last; + + if ( attributeName == 'todoListChecked' && item.name == 'listItem' && item.getAttribute( 'listType' ) != 'todo' ) { + return false; + } + } ); + // Register commands. editor.commands.add( 'todoList', new ListCommand( editor, 'todo' ) ); editor.commands.add( 'todoListCheck', new TodoListCheckCommand( editor ) ); diff --git a/tests/todolistcheckcommand.js b/tests/todolistcheckcommand.js index bff0c5f..fa0f937 100644 --- a/tests/todolistcheckcommand.js +++ b/tests/todolistcheckcommand.js @@ -151,11 +151,11 @@ describe( 'TodoListCheckCommand', () => { ); } ); - it( 'should toggle state on multiple items mixed with non list item', () => { + it( 'should toggle state on multiple items mixed with none todo list items', () => { setModelData( model, 'a[bc' + 'def' + - 'ghi' + + 'ghi' + 'jkl' + 'mn]o' ); @@ -165,7 +165,7 @@ describe( 'TodoListCheckCommand', () => { expect( getModelData( model ) ).to.equal( 'a[bc' + 'def' + - 'ghi' + + 'ghi' + 'jkl' + 'mn]o' ); @@ -175,7 +175,7 @@ describe( 'TodoListCheckCommand', () => { expect( getModelData( model ) ).to.equal( 'a[bc' + 'def' + - 'ghi' + + 'ghi' + 'jkl' + 'mn]o' ); diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 0591ab4..d0f63e6 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -9,6 +9,7 @@ import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting'; import Typing from '@ckeditor/ckeditor5-typing/src/typing'; import ListCommand from '../src/listcommand'; +import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; @@ -48,7 +49,15 @@ describe( 'TodoListEditing', () => { } ); it( 'should set proper schema rules', () => { - expect( model.schema.checkAttribute( [ '$root', 'listItem' ], 'todoListChecked' ) ).to.be.true; + const todoListItem = new ModelElement( 'listItem', { listType: 'todo' } ); + const bulletedListItem = new ModelElement( 'listItem', { listType: 'bulleted' } ); + const numberedListItem = new ModelElement( 'listItem', { listType: 'numbered' } ); + const paragraph = new ModelElement( 'paragraph' ); + + expect( model.schema.checkAttribute( [ '$root', todoListItem ], 'todoListChecked' ) ).to.be.true; + expect( model.schema.checkAttribute( [ '$root', bulletedListItem ], 'todoListChecked' ) ).to.be.false; + expect( model.schema.checkAttribute( [ '$root', numberedListItem ], 'todoListChecked' ) ).to.be.false; + expect( model.schema.checkAttribute( [ '$root', paragraph ], 'todoListChecked' ) ).to.be.false; } ); describe( 'command', () => { From 1fcf0071d1dde9d7b41b0b175d858287a2aaf7c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Sun, 11 Aug 2019 22:12:52 +0200 Subject: [PATCH 22/42] Added v -> m conversion for data pipeline. --- src/todolistconverters.js | 30 +++++++++ src/todolistediting.js | 3 + tests/todolistediting.js | 132 +++++++++++++++++++++++++++++++++++++- 3 files changed, 164 insertions(+), 1 deletion(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index e1cffd7..e69cdd2 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -146,6 +146,36 @@ export function dataModelViewTextInsertion( evt, data, conversionApi ) { viewWriter.wrap( viewWriter.createRangeOn( viewText.parent ), label ); } +/** + * @see module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element + * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. + * @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values. + * @param {module:engine/conversion/upcastdispatcher~UpcastConversionApi} conversionApi Conversion interface to be used by the callback. + */ +export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) { + const modelCursor = data.modelCursor; + const modelItem = modelCursor.parent; + const viewItem = data.viewItem; + + if ( viewItem.getAttribute( 'type' ) != 'checkbox' || modelItem.name != 'listItem' || !modelCursor.isAtStart ) { + return; + } + + if ( !conversionApi.consumable.consume( viewItem, { name: true } ) ) { + return; + } + + const writer = conversionApi.writer; + + writer.setAttribute( 'listType', 'todo', modelItem ); + + if ( data.viewItem.getAttribute( 'checked' ) == 'checked' ) { + writer.setAttribute( 'todoListChecked', true, modelItem ); + } + + data.modelRange = writer.createRange( modelCursor ); +} + /** * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute * @param {module:engine/model/model~Model} model Model instance. diff --git a/src/todolistediting.js b/src/todolistediting.js index 40a8193..7b4c751 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -18,6 +18,7 @@ import { modelViewTextInsertion, dataModelViewInsertion, dataModelViewTextInsertion, + dataViewModelCheckmarkInsertion, modelViewChangeChecked, modelViewChangeType } from './todolistconverters'; @@ -70,6 +71,8 @@ export default class TodoListEditing extends Plugin { editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) ); + data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } ); + // Move selection after a checkbox element. viewDocument.registerPostFixer( writer => moveUIElementsAfterCheckmark( writer, getChangedCheckmarkElements( editing.view ) ) ); diff --git a/tests/todolistediting.js b/tests/todolistediting.js index d0f63e6..1ef5e83 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -358,7 +358,7 @@ describe( 'TodoListEditing', () => { } ); } ); - describe( 'data pipeline', () => { + describe( 'data pipeline m -> v', () => { it( 'should convert todo list item', () => { setModelData( model, '1' + @@ -526,6 +526,136 @@ describe( 'TodoListEditing', () => { } ); } ); + describe( 'data pipeline v -> m', () => { + it( 'should convert li with checkbox inside before the first text node as todo list item', () => { + editor.setData( '
                    • Foo
                    ' ); + + expect( getModelData( model ) ).to.equal( '[]Foo' ); + } ); + + it( 'should convert li with checked checkbox inside as checked todo list item', () => { + editor.setData( '
                    • Foo
                    ' ); + + expect( getModelData( model ) ).to.equal( + '[]Foo' + ); + } ); + + it( 'should not convert li with checkbox in the middle of the text', () => { + editor.setData( '
                    • FooBar
                    ' ); + + expect( getModelData( model ) ).to.equal( '[]FooBar' ); + } ); + + it( 'should convert li with checkbox wrapped by inline elements when checkbox is before the first text node', () => { + editor.setData( '
                    ' ); + + expect( getModelData( model ) ).to.equal( '[]Foo' ); + } ); + + it( 'should split items with checkboxes - bulleted list', () => { + editor.setData( + '
                      ' + + '
                    • foo
                      • ' + + '
                    • bar
                      • ' + + '
                    • biz
                      • ' + + '
                    ' + ); + + expect( getModelData( model ) ).to.equal( + '[]foo' + + 'bar' + + 'biz' + ); + } ); + + it( 'should split items with checkboxes - numbered list', () => { + editor.setData( + '
                      ' + + '
                    1. foo
                      2. ' + + '
                    2. bar
                      3. ' + + '
                    3. biz
                      4. ' + + '
                    ' + ); + + expect( getModelData( model ) ).to.equal( + '[]foo' + + 'bar' + + 'biz' + ); + } ); + + it( 'should convert checkbox in nested lists', () => { + editor.setData( + '
                      ' + + '
                    • 1.1' + + '
                        ' + + '
                      • 2.2
                        • ' + + '
                      • 3.2
                        • ' + + '
                      ' + + '
                      • ' + + '
                    • 4.1' + + '
                        ' + + '
                      1. 5.2
                        2. ' + + '
                      2. 6.2
                        3. ' + + '
                      ' + + '
                      • ' + + '
                    • 7.1
                      • ' + + '
                    ' + ); + + expect( getModelData( model ) ).to.equal( + '[]1.1' + + '2.2' + + '3.2' + + '4.1' + + '5.2' + + '6.2' + + '7.1' + ); + } ); + + it( 'should convert todo list returned by m -> v data pipeline conversion', () => { + editor.setData( + '
                      ' + + '
                    • ' + + '' + + '
                        ' + + '
                      • ' + + '' + + '
                        • ' + + '
                      • ' + + '' + + '
                        • ' + + '
                      ' + + '
                      • ' + + '
                    • ' + + '' + + '
                      • ' + + '
                    ' + ); + + expect( getModelData( model ) ).to.equal( + '[]1.1' + + '2.2' + + '3.2' + + '4.1' + ); + } ); + } ); + describe( 'selection view post-fixer', () => { it( 'should move selection after checkmark element to the first text node', () => { setModelData( model, 'Foo' ); From 526b0f497e2da70cdbe566fd3f9ddb51169b87a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Sun, 11 Aug 2019 22:40:15 +0200 Subject: [PATCH 23/42] Added test for Ctrl+space keystroke handler. --- tests/todolistediting.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 1ef5e83..b9b3365 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -836,4 +836,31 @@ describe( 'TodoListEditing', () => { sinon.assert.notCalled( domEvtDataStub.stopPropagation ); } ); } ); + + describe( 'Ctrl+space keystroke handling', () => { + let domEvtDataStub; + + beforeEach( () => { + domEvtDataStub = { + keyCode: getCode( 'space' ), + ctrlKey: true, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + } ); + + it( 'should execute TodoListCheckCommand', () => { + const command = editor.commands.get( 'todoListCheck' ); + + sinon.spy( command, 'execute' ); + + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.calledOnce( command.execute ); + + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.calledTwice( command.execute ); + } ); + } ); } ); From 8038f00d5ab283f6c17cc2b92c6ecbdf40148c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 00:27:01 +0200 Subject: [PATCH 24/42] Added missing API docs. --- src/converters.js | 3 ++ src/todolist.js | 3 ++ src/todolistcheckcommand.js | 29 ++++++++++----- src/todolistconverters.js | 71 +++++++++++++++++++++++++++++++++++++ src/todolistediting.js | 12 +++++++ src/todolistui.js | 3 ++ 6 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/converters.js b/src/converters.js index e17e48d..490161c 100644 --- a/src/converters.js +++ b/src/converters.js @@ -98,6 +98,9 @@ export function modelViewRemove( model ) { * by breaking view elements and changing their name. Next {@link module:list/converters~modelViewMergeAfterChangeType} * converter will try to merge split nodes. * + * Splitting this conversion into 2 steps makes it possible to add an additional conversion in the middle. + * Check {@link module:list/todolistconverters~modelViewChangeType} to see an example of it. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. * @param {Object} data Additional information about the change. diff --git a/src/todolist.js b/src/todolist.js index 913cc53..9a94604 100644 --- a/src/todolist.js +++ b/src/todolist.js @@ -15,6 +15,9 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; /** * The todo list feature. * + * This is a "glue" plugin which loads the {@link module:list/todolistediting~TodoListEditing todo list editing feature} + * and {@link module:list/todolistui~TodoListUI list UI feature}. + * * @extends module:core/plugin~Plugin */ export default class List extends Plugin { diff --git a/src/todolistcheckcommand.js b/src/todolistcheckcommand.js index 25a550e..1f0bd0a 100644 --- a/src/todolistcheckcommand.js +++ b/src/todolistcheckcommand.js @@ -16,25 +16,27 @@ const attributeKey = 'todoListChecked'; */ export default class TodoListCheckCommand extends Command { /** - * @param {module:core/editor/editor~Editor} editor + * @inheritDoc */ constructor( editor ) { super( editor ); - this._selectedElements = []; - /** - * Flag indicating whether the command is active. The command is active when the - * {@link module:engine/model/selection~Selection#hasAttribute selection has the attribute} which means that: - * - * * If the selection is not empty – That the attribute is set on the first node in the selection that allows this attribute. - * * If the selection is empty – That the selection has the attribute itself (which means that newly typed - * text will have this attribute, too). + * Flag indicating whether the command is active. The command is active when at least one of + * {@link module:engine/model/selection~Selection selected} elements is a todo list item. * * @observable * @readonly * @member {Boolean} #value */ + + /** + * List of todo list items selected by the {@link module:engine/model/selection~Selection}. + * + * @type {Array.} + * @private + */ + this._selectedElements = []; } /** @@ -46,6 +48,12 @@ export default class TodoListCheckCommand extends Command { this.isEnabled = !!this._selectedElements.length; } + /** + * Gets all todo list items selected by the {@link module:engine/model/selection~Selection}. + * + * @private + * @returns {Array.} + */ _getSelectedItems() { const model = this.editor.model; const schema = model.schema; @@ -67,6 +75,9 @@ export default class TodoListCheckCommand extends Command { return elements; } + /** + * @inheritDoc + */ execute() { this.editor.model.change( writer => { for ( const element of this._selectedElements ) { diff --git a/src/todolistconverters.js b/src/todolistconverters.js index e69cdd2..0ca603a 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -13,6 +13,13 @@ import { generateLiInUl, injectViewList, findInRange } from './utils'; import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; /** + * A model-to-view converter for `listItem` model element insertion. + * + * It converts `listItem` model element to an unordered list with {@link module:engine/view/uielement~UIElement checkbox element} + * at the beginning of each list item. It also merges the list with surrounding lists (if available). + * + * It is used by {@link module:engine/controller/editingcontroller~EditingController}. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert * @param {module:engine/model/model~Model} model Model instance. * @returns {Function} Returns a conversion callback. @@ -47,6 +54,12 @@ export function modelViewInsertion( model ) { } /** + * A model-to-view converter for model `$text` element inside a todo list item. + * + * It takes care of creating text after the {@link module:engine/view/uielement~UIElement checkbox UI element}. + * + * It is used by {@link module:engine/controller/editingcontroller~EditingController}. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. * @param {Object} data Additional information about the change. @@ -67,10 +80,19 @@ export function modelViewTextInsertion( evt, data, conversionApi ) { const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); const viewText = viewWriter.createText( data.item.data ); + // Be sure text is created after the UIElement, so if it is a first text node inside a `listItem` element + // it has to be moved after the first node in the view list item. + // + // model: [foo] + // view:
                  • ^
                    • ->
                  • foo
                    • viewWriter.insert( viewPosition.offset ? viewPosition : viewPosition.getShiftedBy( 1 ), viewText ); } /** + * A model-to-view converter for `listItem` model element insertion. + * + * It is used by {@link module:engine/controller/datacontroller~DataController}. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert * @param {module:engine/model/model~Model} model Model instance. * @returns {Function} Returns a conversion callback. @@ -119,6 +141,10 @@ export function dataModelViewInsertion( model ) { } /** + * A model-to-view converter for model `$text` element inside a todo list item. + * + * It is used by {@link module:engine/controller/datacontroller~DataController}. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:insert * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. * @param {Object} data Additional information about the change. @@ -147,6 +173,13 @@ export function dataModelViewTextInsertion( evt, data, conversionApi ) { } /** + * A view-to-model converter for checkbox element inside a view list item. + * + * It changes `listType` of model `listItem` to a `todo` value. + * When view checkbox is marked as checked the additional `todoListChecked="true"` attribute is added to model item. + * + * It is used by {@link module:engine/controller/datacontroller~DataController}. + * * @see module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element * @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event. * @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values. @@ -177,6 +210,16 @@ export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) { } /** + * A model-to-view converter for `listType` attribute change on `listItem` model element. + * + * This change means that `
                  • ` elements parent changes to `
                      ` and + * {@link module:engine/view/uielement~UIElement checkbox UI element} is added at the beginning of the list item element. + * + * This converter is preceded by {@link module:list/converters~modelViewChangeType} and followed by + * {@link module:list/converters~modelViewMergeAfterChangeType} to handle splitting and merging surrounding lists of the same type. + * + * It is used by {@link module:engine/controller/editingcontroller~EditingController}. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute * @param {module:engine/model/model~Model} model Model instance. * @returns {Function} Returns a conversion callback. @@ -196,6 +239,12 @@ export function modelViewChangeType( model ) { } /** + * A model-to-view converter for `todoListChecked` attribute change on `listItem` model element. + * + * It marks {@link module:engine/view/uielement~UIElement checkbox UI element} as checked. + * + * It is used by {@link module:engine/controller/editingcontroller~EditingController}. + * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute * @param {module:engine/model/model~Model} model Model instance. * @returns {Function} Returns a conversion callback. @@ -218,6 +267,13 @@ export function modelViewChangeChecked( model ) { }; } +// Injects checkbox element inside a view list item and adds `todo-list` class to the parent list element. +// +// @private +// @param {module:engine/model/item~Item} modelItem +// @param {module:engine/view/item~Item} ViewItem +// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter +// @param {module:engine/model/model~Model} model function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, model ); @@ -226,12 +282,27 @@ function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); } +// Removes checkbox element from a view list item and removes `todo-list` class from the parent list element. +// +// @private +// @param {module:engine/model/item~Item} modelItem +// @param {module:engine/view/item~Item} ViewItem +// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter +// @param {module:engine/model/model~Model} model function removeTodoElementsFromListItem( modelItem, viewItem, viewWriter, model ) { viewWriter.removeClass( 'todo-list', viewItem.parent ); viewWriter.remove( viewItem.getChild( 0 ) ); model.change( writer => writer.removeAttribute( 'todoListChecked', modelItem ) ); } +// Creates checkbox UI element. +// +// @private +// @param {module:engine/model/item~Item} modelItem +// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter +// @param {Boolean} isChecked +// @param {module:engine/model/model~Model} model +// @returns {module:view/uielement~UIElement} function createCheckmarkElement( modelItem, viewWriter, isChecked, model ) { const uiElement = viewWriter.createUIElement( 'label', diff --git a/src/todolistediting.js b/src/todolistediting.js index 7b4c751..676338e 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -26,6 +26,11 @@ import { import { findInRange } from './utils'; /** + * The engine of the todo list feature. It handles creating, editing and removing todo lists and its items. + * + * It registers all functionalities of {@link module:list/listediting~ListEditing list editing plugin} and extends + * it by `'todoList'` command. + * * @extends module:core/plugin~Plugin */ export default class TodoListEditing extends Plugin { @@ -123,6 +128,8 @@ export default class TodoListEditing extends Plugin { } } +// Moves all uiElements in the todo list item after the checkmark element. +// // @private // @param {module:engine/view/downcastwriter~DowncastWriter} writer // @param {Array.} uiElements @@ -152,6 +159,8 @@ function moveUIElementsAfterCheckmark( writer, uiElements ) { return hasChanged; } +// Moves selection in the todo list item after the checkmark element. +// // @private // @param {module:engine/view/downcastwriter~DowncastWriter} writer // @param {module:engine/view/documentselection~DocumentSelection} selection @@ -188,6 +197,9 @@ function moveSelectionAfterCheckmark( writer, selection ) { return false; } +// Handles left arrow key and move selection at the end of the previous block element if the selection is just after +// the checkmark element. In other words, it jumps over the checkmark element when moving the selection to the left. +// // @private // @param {Function} stopKeyEvent // @param {module:engine/model/model~Model} model diff --git a/src/todolistui.js b/src/todolistui.js index b02549f..8a8dd53 100644 --- a/src/todolistui.js +++ b/src/todolistui.js @@ -15,6 +15,9 @@ import '../theme/list.css'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; /** + * The todo list UI feature. It introduces the `'todoList'` button that + * allow to convert elements to and from list items and indent or outdent them. + * * @extends module:core/plugin~Plugin */ export default class TodoListUI extends Plugin { From 378bf5b31ef4791f074f2010c146c1cc6b2947db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 00:30:33 +0200 Subject: [PATCH 25/42] Removed redundant comma. --- src/listediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/listediting.js b/src/listediting.js index cdc4e5f..83186d9 100644 --- a/src/listediting.js +++ b/src/listediting.js @@ -27,7 +27,7 @@ import { modelIndentPasteFixer, viewModelConverter, modelToViewPosition, - viewToModelPosition, + viewToModelPosition } from './converters'; /** From edbb988d12bd627c1da9307f3db856d436035e05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 00:47:51 +0200 Subject: [PATCH 26/42] Remove todoListChecked attribute in a postfixer instead of converter. --- src/todolistconverters.js | 59 ++++++++++++++------------------------- src/todolistediting.js | 6 ++++ 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index 0ca603a..ea6ce4e 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -39,16 +39,22 @@ export function modelViewInsertion( model ) { return; } - consumable.consume( data.item, 'insert' ); - consumable.consume( data.item, 'attribute:listType' ); - consumable.consume( data.item, 'attribute:listIndent' ); - consumable.consume( data.item, 'attribute:todoListChecked' ); + const modelItem = data.item; + + consumable.consume( modelItem, 'insert' ); + consumable.consume( modelItem, 'attribute:listType' ); + consumable.consume( modelItem, 'attribute:listIndent' ); + consumable.consume( modelItem, 'attribute:todoListChecked' ); const viewWriter = conversionApi.writer; - const modelItem = data.item; const viewItem = generateLiInUl( modelItem, conversionApi ); - addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ); + const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); + const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, model ); + + viewWriter.addClass( 'todo-list', viewItem.parent ); + viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); + injectViewList( modelItem, viewItem, conversionApi, model ); }; } @@ -213,7 +219,8 @@ export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) { * A model-to-view converter for `listType` attribute change on `listItem` model element. * * This change means that `
                    • ` elements parent changes to `
                        ` and - * {@link module:engine/view/uielement~UIElement checkbox UI element} is added at the beginning of the list item element. + * {@link module:engine/view/uielement~UIElement checkbox UI element} is added at the beginning + * of the list item element (or vice versa). * * This converter is preceded by {@link module:list/converters~modelViewChangeType} and followed by * {@link module:list/converters~modelViewMergeAfterChangeType} to handle splitting and merging surrounding lists of the same type. @@ -229,11 +236,15 @@ export function modelViewChangeType( model ) { const viewItem = conversionApi.mapper.toViewElement( data.item ); const viewWriter = conversionApi.writer; - // Add or remove checkbox for toto list. if ( data.attributeNewValue == 'todo' ) { - addTodoElementsToListItem( data.item, viewItem, viewWriter, model ); + const isChecked = !!data.item.getAttribute( 'todoListChecked' ); + const checkmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, model ); + + viewWriter.addClass( 'todo-list', viewItem.parent ); + viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); } else if ( data.attributeOldValue == 'todo' ) { - removeTodoElementsFromListItem( data.item, viewItem, viewWriter, model ); + viewWriter.removeClass( 'todo-list', viewItem.parent ); + viewWriter.remove( viewItem.getChild( 0 ) ); } }; } @@ -267,34 +278,6 @@ export function modelViewChangeChecked( model ) { }; } -// Injects checkbox element inside a view list item and adds `todo-list` class to the parent list element. -// -// @private -// @param {module:engine/model/item~Item} modelItem -// @param {module:engine/view/item~Item} ViewItem -// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter -// @param {module:engine/model/model~Model} model -function addTodoElementsToListItem( modelItem, viewItem, viewWriter, model ) { - const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); - const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, model ); - - viewWriter.addClass( 'todo-list', viewItem.parent ); - viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); -} - -// Removes checkbox element from a view list item and removes `todo-list` class from the parent list element. -// -// @private -// @param {module:engine/model/item~Item} modelItem -// @param {module:engine/view/item~Item} ViewItem -// @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter -// @param {module:engine/model/model~Model} model -function removeTodoElementsFromListItem( modelItem, viewItem, viewWriter, model ) { - viewWriter.removeClass( 'todo-list', viewItem.parent ); - viewWriter.remove( viewItem.getChild( 0 ) ); - model.change( writer => writer.removeAttribute( 'todoListChecked', modelItem ) ); -} - // Creates checkbox UI element. // // @private diff --git a/src/todolistediting.js b/src/todolistediting.js index 676338e..11b90a4 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -110,6 +110,12 @@ export default class TodoListEditing extends Plugin { if ( item.hasAttribute( 'todoListChecked' ) ) { listItemsToFix.add( item ); } + } else if ( operation.type == 'changeAttribute' && operation.key == 'listType' && operation.oldValue === 'todo' ) { + for ( const item of operation.range.getItems() ) { + if ( item.hasAttribute( 'todoListChecked' ) && item.getAttribute( 'listType' ) !== 'todo' ) { + listItemsToFix.add( item ); + } + } } } ); From 5800bee34ebfe7f62ddb55359669553fa4b56dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 14:31:58 +0200 Subject: [PATCH 27/42] Change `todoListChecked` state using command when clicking on checkbox element. --- src/todolistcheckcommand.js | 6 ++ src/todolistconverters.js | 31 ++++----- src/todolistediting.js | 39 ++++++++++- tests/listediting.js | 4 ++ tests/todolistcheckcommand.js | 12 ++++ tests/todolistediting.js | 119 ++++++++++++++++++++++++++++++++++ 6 files changed, 190 insertions(+), 21 deletions(-) diff --git a/src/todolistcheckcommand.js b/src/todolistcheckcommand.js index 1f0bd0a..c2d835d 100644 --- a/src/todolistcheckcommand.js +++ b/src/todolistcheckcommand.js @@ -37,6 +37,12 @@ export default class TodoListCheckCommand extends Command { * @private */ this._selectedElements = []; + + // Refresh command before executing to be sure all values are up to date. + // It is needed when selection has changed before command execution, in the same change block. + this.on( 'execute', () => { + this.refresh(); + }, { priority: 'highest' } ); } /** diff --git a/src/todolistconverters.js b/src/todolistconverters.js index ea6ce4e..fe015d8 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -24,7 +24,7 @@ import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; * @param {module:engine/model/model~Model} model Model instance. * @returns {Function} Returns a conversion callback. */ -export function modelViewInsertion( model ) { +export function modelViewInsertion( model, onCheckboxChecked ) { return ( evt, data, conversionApi ) => { const consumable = conversionApi.consumable; @@ -50,7 +50,7 @@ export function modelViewInsertion( model ) { const viewItem = generateLiInUl( modelItem, conversionApi ); const isChecked = !!modelItem.getAttribute( 'todoListChecked' ); - const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, model ); + const checkmarkElement = createCheckmarkElement( modelItem, viewWriter, isChecked, onCheckboxChecked ); viewWriter.addClass( 'todo-list', viewItem.parent ); viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); @@ -228,17 +228,17 @@ export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) { * It is used by {@link module:engine/controller/editingcontroller~EditingController}. * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute - * @param {module:engine/model/model~Model} model Model instance. + * @param {Function} onCheckedChange Callback fired after clicking on checkmark element. * @returns {Function} Returns a conversion callback. */ -export function modelViewChangeType( model ) { +export function modelViewChangeType( onCheckedChange ) { return ( evt, data, conversionApi ) => { const viewItem = conversionApi.mapper.toViewElement( data.item ); const viewWriter = conversionApi.writer; if ( data.attributeNewValue == 'todo' ) { const isChecked = !!data.item.getAttribute( 'todoListChecked' ); - const checkmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, model ); + const checkmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, onCheckedChange ); viewWriter.addClass( 'todo-list', viewItem.parent ); viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement ); @@ -257,10 +257,10 @@ export function modelViewChangeType( model ) { * It is used by {@link module:engine/controller/editingcontroller~EditingController}. * * @see module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute - * @param {module:engine/model/model~Model} model Model instance. + * @param {Function} onCheckedChange Callback fired after clicking on checkmark element. * @returns {Function} Returns a conversion callback. */ -export function modelViewChangeChecked( model ) { +export function modelViewChangeChecked( onCheckedChange ) { return ( evt, data, conversionApi ) => { if ( !conversionApi.consumable.consume( data.item, 'attribute:todoListChecked' ) ) { return; @@ -271,7 +271,7 @@ export function modelViewChangeChecked( model ) { const viewItem = mapper.toViewElement( data.item ); const itemRange = viewWriter.createRangeIn( viewItem ); const oldCheckmarkElement = findInRange( itemRange, item => item.is( 'uiElement' ) ? item : false ); - const newCheckmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, model ); + const newCheckmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, onCheckedChange ); viewWriter.insert( viewWriter.createPositionAfter( oldCheckmarkElement ), newCheckmarkElement ); viewWriter.remove( oldCheckmarkElement ); @@ -284,9 +284,9 @@ export function modelViewChangeChecked( model ) { // @param {module:engine/model/item~Item} modelItem // @param {module:engine/view/downcastwriter~DowncastWriter} viewWriter // @param {Boolean} isChecked -// @param {module:engine/model/model~Model} model +// @param {Function} onChange // @returns {module:view/uielement~UIElement} -function createCheckmarkElement( modelItem, viewWriter, isChecked, model ) { +function createCheckmarkElement( modelItem, viewWriter, isChecked, onChange ) { const uiElement = viewWriter.createUIElement( 'label', { @@ -298,14 +298,9 @@ function createCheckmarkElement( modelItem, viewWriter, isChecked, model ) { checkbox.checked = isChecked; - checkbox.addEventListener( 'change', evt => { - model.change( writer => { - if ( evt.target.checked ) { - writer.setAttribute( 'todoListChecked', true, modelItem ); - } else { - writer.removeAttribute( 'todoListChecked', modelItem ); - } - } ); + checkbox.addEventListener( 'mousedown', evt => { + evt.stopPropagation(); + onChange( modelItem ); } ); const domElement = this.toDomElement( domDocument ); diff --git a/src/todolistediting.js b/src/todolistediting.js index 11b90a4..e18ad2d 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -68,13 +68,23 @@ export default class TodoListEditing extends Plugin { editor.commands.add( 'todoListCheck', new TodoListCheckCommand( editor ) ); // Define converters. - editing.downcastDispatcher.on( 'insert:listItem', modelViewInsertion( model ), { priority: 'high' } ); + editing.downcastDispatcher.on( + 'insert:listItem', + modelViewInsertion( model, listItem => this._handleCheckmarkChange( listItem ) ), + { priority: 'high' } + ); editing.downcastDispatcher.on( 'insert:$text', modelViewTextInsertion, { priority: 'high' } ); data.downcastDispatcher.on( 'insert:listItem', dataModelViewInsertion( model ), { priority: 'high' } ); data.downcastDispatcher.on( 'insert:$text', dataModelViewTextInsertion, { priority: 'high' } ); - editing.downcastDispatcher.on( 'attribute:listType:listItem', modelViewChangeType( model ) ); - editing.downcastDispatcher.on( 'attribute:todoListChecked:listItem', modelViewChangeChecked( model ) ); + editing.downcastDispatcher.on( + 'attribute:listType:listItem', + modelViewChangeType( listItem => this._handleCheckmarkChange( listItem ) ) + ); + editing.downcastDispatcher.on( + 'attribute:todoListChecked:listItem', + modelViewChangeChecked( listItem => this._handleCheckmarkChange( listItem ) ) + ); data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } ); @@ -132,6 +142,29 @@ export default class TodoListEditing extends Plugin { return hasChanged; } ); } + + /** + * Handles checkmar element change, moves selection to the corresponding model item to makes possible + * to toggle `todoListChecked` attribute using command and restore the selection position. + * + * Some say it's a hack :) Moving selection only for executing the command on a certain node and restoring it after, + * it's not a clear solution. We need to design an API for using commands beyond the selection range. + * See https://github.com/ckeditor/ckeditor5/issues/1954. + * + * @private + * @param {module:engine/model/element~Element} listItem + */ + _handleCheckmarkChange( listItem ) { + const editor = this.editor; + const model = editor.model; + const previousSelectionRanges = Array.from( model.document.selection.getRanges() ); + + model.change( writer => { + writer.setSelection( listItem, 'end' ); + editor.execute( 'todoListCheck' ); + writer.setSelection( previousSelectionRanges ); + } ); + } } // Moves all uiElements in the todo list item after the checkmark element. diff --git a/tests/listediting.js b/tests/listediting.js index 4c6523f..073efbe 100644 --- a/tests/listediting.js +++ b/tests/listediting.js @@ -50,6 +50,10 @@ describe( 'ListEditing', () => { } ); } ); + afterEach( () => { + return editor.destroy(); + } ); + it( 'should be loaded', () => { expect( editor.plugins.get( ListEditing ) ).to.be.instanceOf( ListEditing ); } ); diff --git a/tests/todolistcheckcommand.js b/tests/todolistcheckcommand.js index fa0f937..9ef4d30 100644 --- a/tests/todolistcheckcommand.js +++ b/tests/todolistcheckcommand.js @@ -204,5 +204,17 @@ describe( 'TodoListCheckCommand', () => { expect( getModelData( model ) ).to.equal( 'b[]ar' ); } ); + + it( 'should be up to date just before execution', () => { + setModelData( model, + 'f[]oo' + + 'bar' + ); + + model.change( writer => { + writer.setSelection( model.document.getRoot().getChild( 1 ), 'end' ); + command.execute(); + } ); + } ); } ); } ); diff --git a/tests/todolistediting.js b/tests/todolistediting.js index b9b3365..477f71b 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -12,10 +12,13 @@ import ListCommand from '../src/listcommand'; import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; +/* global Event, document */ + describe( 'TodoListEditing', () => { let editor, model, modelDoc, modelRoot, view, viewDoc, viewRoot; @@ -44,6 +47,10 @@ describe( 'TodoListEditing', () => { } ); } ); + afterEach( () => { + return editor.destroy(); + } ); + it( 'should load ListEditing', () => { expect( TodoListEditing.requires ).to.have.members( [ ListEditing ] ); } ); @@ -654,6 +661,22 @@ describe( 'TodoListEditing', () => { '4.1' ); } ); + + it( 'should be overwritable', () => { + editor.data.upcastDispatcher.on( 'element:input', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.viewItem, { name: true } ); + conversionApi.writer.setAttribute( 'listType', 'numbered', data.modelCursor.parent ); + data.modelRange = conversionApi.writer.createRange( data.modelCursor ); + }, { priority: 'highest' } ); + + editor.setData( + '
                          ' + + '
                        • foo
                          • ' + + '
                        ' + ); + + expect( getModelData( model ) ).to.equal( '[]foo' ); + } ); } ); describe( 'selection view post-fixer', () => { @@ -864,3 +887,99 @@ describe( 'TodoListEditing', () => { } ); } ); } ); + +describe( 'TodoListEditing', () => { + let editorElement, editor, model, view, viewDoc, viewRoot; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ TodoListEditing ] + } ) + .then( newEditor => { + editor = newEditor; + + model = editor.model; + + view = editor.editing.view; + viewDoc = view.document; + viewRoot = viewDoc.getRoot(); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should render checkbox inside a checkmark UIElement', () => { + setModelData( model, 'foo' ); + + const checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + + expect( checkmarkViewElement.is( 'uiElement' ) ).to.equal( true ); + + const checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + const checkboxElement = checkmarkDomElement.children[ 0 ]; + + expect( checkboxElement.tagName ).to.equal( 'INPUT' ); + expect( checkboxElement.checked ).to.equal( false ); + } ); + + it( 'should render checked checkbox inside a checkmark UIElement', () => { + setModelData( model, 'foo' ); + + const checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + const checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + const checkboxElement = checkmarkDomElement.children[ 0 ]; + + expect( checkboxElement.checked ).to.equal( true ); + } ); + + it( 'should toggle `todoListChecked` state using command when click on checkbox element', () => { + setModelData( model, + 'foo' + + 'b[a]r' + ); + + const command = editor.commands.get( 'todoListCheck' ); + + sinon.spy( command, 'execute' ); + + let checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + let checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + let checkboxElement = checkmarkDomElement.children[ 0 ]; + + expect( checkboxElement.checked ).to.equal( false ); + + checkboxElement.dispatchEvent( new Event( 'mousedown' ) ); + + checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + checkboxElement = checkmarkDomElement.children[ 0 ]; + + sinon.assert.calledOnce( command.execute ); + expect( checkboxElement.checked ).to.equal( true ); + expect( getModelData( model ) ).to.equal( + 'foo' + + 'b[a]r' + ); + + checkboxElement.dispatchEvent( new Event( 'mousedown' ) ); + + checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + checkboxElement = checkmarkDomElement.children[ 0 ]; + + sinon.assert.calledTwice( command.execute ); + expect( checkboxElement.checked ).to.equal( false ); + expect( getModelData( model ) ).to.equal( + 'foo' + + 'b[a]r' + ); + } ); +} ); From 31275b683bb96a9bc5e0cfec56b0d0d72cfd8d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 14:33:31 +0200 Subject: [PATCH 28/42] Added missing destroy in tests. --- tests/listui.js | 6 ++++-- tests/todolistui.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/listui.js b/tests/listui.js index 263aa82..4618b41 100644 --- a/tests/listui.js +++ b/tests/listui.js @@ -15,10 +15,10 @@ import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; describe( 'ListUI', () => { - let editor, model, bulletedListButton, numberedListButton; + let editorElement, editor, model, bulletedListButton, numberedListButton; beforeEach( () => { - const editorElement = document.createElement( 'div' ); + editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); return ClassicTestEditor.create( editorElement, { plugins: [ Paragraph, BlockQuote, ListEditing, ListUI ] } ) @@ -32,6 +32,8 @@ describe( 'ListUI', () => { } ); afterEach( () => { + editorElement.remove(); + return editor.destroy(); } ); diff --git a/tests/todolistui.js b/tests/todolistui.js index c67ac2d..663de19 100644 --- a/tests/todolistui.js +++ b/tests/todolistui.js @@ -14,10 +14,10 @@ import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; describe( 'TodoListUI', () => { - let editor, model, button; + let editorElement, editor, model, button; beforeEach( () => { - const editorElement = document.createElement( 'div' ); + editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); return ClassicTestEditor.create( editorElement, { plugins: [ Paragraph, TodoListEditing, TodoListUI ] } ) @@ -30,6 +30,8 @@ describe( 'TodoListUI', () => { } ); afterEach( () => { + editorElement.remove(); + return editor.destroy(); } ); From 492a56a17c1b2ca0808e70b297a4d89ad5166da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 14:33:40 +0200 Subject: [PATCH 29/42] Visual improvements. --- theme/list.css | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/theme/list.css b/theme/list.css index 598aa7e..a1f0d02 100644 --- a/theme/list.css +++ b/theme/list.css @@ -1,5 +1,5 @@ :root { - --ck-todo-list-checkmark-size: 14px; + --ck-todo-list-checkmark-size: 16px; --ck-color-todo-list-checkmark-background: hsl(120, 100%, 42%); --ck-color-todo-list-checkmark-border: hsl( 120, 100%, 27%); } @@ -17,12 +17,18 @@ margin-top: 5px; } -/* Let's hide native checkbox and make a fancy one. */ +/* + * Let's hide native checkbox and make a fancy one. + * + * Note: Checkbox element cannot be hidden using `display: block` or `visibility: hidden`. + * It has to be clickable to not steal the focus after clicking on it. + */ .ck .todo-list__checkmark input[type='checkbox'] { display: block; width: 100%; height: 100%; opacity: 0; + margin: 0; } .ck .todo-list__checkmark { @@ -31,8 +37,9 @@ height: var(--ck-todo-list-checkmark-size); position: relative; display: inline-block; - left: -20px; - margin-right: -10px; + left: -23px; + margin-right: -16px; + top: 2px; } .ck .todo-list__checkmark::before { @@ -44,6 +51,7 @@ border: 1px solid var(--ck-color-base-text); border-radius: var(--ck-border-radius); transition: 250ms ease-in-out box-shadow, 250ms ease-in-out background, 250ms ease-in-out border; + box-sizing: border-box; } .ck .todo-list__checkmark::after { From 7020e678561158f40e4fb7d5d2f206095b42726b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 12 Aug 2019 15:02:50 +0200 Subject: [PATCH 30/42] Do not use renderer for getting changed UIElements. --- src/todolistediting.js | 22 +++++++++++++++++++--- tests/todolistediting.js | 10 +--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index e18ad2d..86bbbe5 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -88,8 +88,22 @@ export default class TodoListEditing extends Plugin { data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } ); + // Collect all view nodes that have changed and use it to check if checkmark UI element is going to + // be re-rendered. If yes than view post-fixer should verify view structure. + const changedViewNodes = new Set(); + + for ( const viewRoot of viewDocument.roots ) { + this.listenTo( viewRoot, 'change:children', ( evt, node ) => changedViewNodes.add( node ) ); + } + // Move selection after a checkbox element. - viewDocument.registerPostFixer( writer => moveUIElementsAfterCheckmark( writer, getChangedCheckmarkElements( editing.view ) ) ); + viewDocument.registerPostFixer( writer => { + const changedCheckmarkElements = getChangedCheckmarkElements( writer, changedViewNodes ); + + changedViewNodes.clear(); + + return moveUIElementsAfterCheckmark( writer, changedCheckmarkElements ); + } ); // Move all uiElements after a checkbox element. viewDocument.registerPostFixer( writer => moveSelectionAfterCheckmark( writer, viewDocument.selection ) ); @@ -266,11 +280,13 @@ function jumpOverCheckmarkOnLeftArrowKeyPress( stopKeyEvent, model ) { // Gets list of all checkmark elements that are going to be rendered. // // @private +// @param {module:engine/view/view~View>} editingView +// @param {Set.} changedViewNodes // @returns {Array.} -function getChangedCheckmarkElements( editingView ) { +function getChangedCheckmarkElements( editingView, changedViewNodes ) { const elements = []; - for ( const element of Array.from( editingView._renderer.markedChildren ) ) { + for ( const element of changedViewNodes ) { for ( const item of editingView.createRangeIn( element ).getItems() ) { if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) && !elements.includes( item ) ) { elements.push( item ); diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 477f71b..54ef7d1 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -20,7 +20,7 @@ import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; /* global Event, document */ describe( 'TodoListEditing', () => { - let editor, model, modelDoc, modelRoot, view, viewDoc, viewRoot; + let editor, model, modelDoc, modelRoot, view, viewDoc; beforeEach( () => { return VirtualTestEditor @@ -36,7 +36,6 @@ describe( 'TodoListEditing', () => { view = editor.editing.view; viewDoc = view.document; - viewRoot = viewDoc.getRoot(); model.schema.register( 'foo', { allowWhere: '$block', @@ -758,13 +757,6 @@ describe( 'TodoListEditing', () => { range: writer.createRangeIn( modelRoot.getChild( 0 ) ), usingOperation: false } ); - - // VirtualTestEeditor does not render V to DOM, so we need to mock element market to be rendered - // because view post-fixer uses it. - view._renderer.markedChildren = new Set( [ - viewRoot.getChild( 0 ).getChild( 0 ), - viewRoot.getChild( 0 ).getChild( 1 ) - ] ); } ); expect( getViewData( view ) ).to.equal( From a54f67b976f6fd89602fa231fa258177b066999c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 08:50:49 +0200 Subject: [PATCH 31/42] Improved CC. --- src/todolistediting.js | 2 +- tests/todolistediting.js | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index 86bbbe5..df2a259 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -288,7 +288,7 @@ function getChangedCheckmarkElements( editingView, changedViewNodes ) { for ( const element of changedViewNodes ) { for ( const item of editingView.createRangeIn( element ).getItems() ) { - if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) && !elements.includes( item ) ) { + if ( item.is( 'uiElement' ) && item.hasClass( 'todo-list__checkmark' ) && !elements.includes( item ) && element.document ) { elements.push( item ); } } diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 54ef7d1..4b71efb 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -553,7 +553,7 @@ describe( 'TodoListEditing', () => { expect( getModelData( model ) ).to.equal( '[]FooBar' ); } ); - it( 'should convert li with checkbox wrapped by inline elements when checkbox is before the first text node', () => { + it( 'should convert li with checkbox wrapped by inline elements when checkbox is before the first text node', () => { editor.setData( '
                        ' ); expect( getModelData( model ) ).to.equal( '[]Foo' ); @@ -723,7 +723,7 @@ describe( 'TodoListEditing', () => { describe( 'uiElements view post-fixer', () => { it( 'should move all UIElements from before a checkmark after the checkmark element', () => { setModelData( model, - 'foo' + + '[]foo' + 'bar' ); @@ -774,6 +774,9 @@ describe( 'TodoListEditing', () => { '' + '
                      ' ); + + // CC. + editor.execute( 'todoListCheck' ); } ); } ); @@ -974,4 +977,31 @@ describe( 'TodoListEditing', () => { 'b[a]r' ); } ); + + it( 'should toggle `todoListChecked` state using command when checkmark was created as a result of changing list type', () => { + setModelData( model, 'f[]oo' ); + editor.execute( 'todoList' ); + + const command = editor.commands.get( 'todoListCheck' ); + + sinon.spy( command, 'execute' ); + + let checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + let checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + let checkboxElement = checkmarkDomElement.children[ 0 ]; + + expect( checkboxElement.checked ).to.equal( false ); + + checkboxElement.dispatchEvent( new Event( 'mousedown' ) ); + + checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); + checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + checkboxElement = checkmarkDomElement.children[ 0 ]; + + sinon.assert.calledOnce( command.execute ); + expect( checkboxElement.checked ).to.equal( true ); + expect( getModelData( model ) ).to.equal( + 'f[]oo' + ); + } ); } ); From 69cd4e16b5196fa95800268a4264c74fe2b0490c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 09:37:46 +0200 Subject: [PATCH 32/42] Selection improvements. --- src/todolistconverters.js | 6 +----- src/todolistediting.js | 14 +++++--------- tests/todolistediting.js | 26 +++----------------------- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index fe015d8..8449cd8 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -297,11 +297,7 @@ function createCheckmarkElement( modelItem, viewWriter, isChecked, onChange ) { const checkbox = createElement( document, 'input', { type: 'checkbox', } ); checkbox.checked = isChecked; - - checkbox.addEventListener( 'mousedown', evt => { - evt.stopPropagation(); - onChange( modelItem ); - } ); + checkbox.addEventListener( 'change', () => onChange( modelItem ) ); const domElement = this.toDomElement( domDocument ); diff --git a/src/todolistediting.js b/src/todolistediting.js index df2a259..c396513 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -218,6 +218,10 @@ function moveUIElementsAfterCheckmark( writer, uiElements ) { // @param {module:engine/view/downcastwriter~DowncastWriter} writer // @param {module:engine/view/documentselection~DocumentSelection} selection function moveSelectionAfterCheckmark( writer, selection ) { + if ( !selection.isCollapsed ) { + return false; + } + const positionToChange = selection.getFirstPosition(); if ( positionToChange.parent.name != 'li' || !positionToChange.parent.parent.hasClass( 'todo-list' ) ) { @@ -234,15 +238,7 @@ function moveSelectionAfterCheckmark( writer, selection ) { const text = findInRange( boundaries, item => item.is( 'textProxy' ) ? item.textNode : false ); const nextPosition = text ? writer.createPositionAt( text, 0 ) : parentEndPosition; - let range; - - if ( selection.isCollapsed ) { - range = writer.createRange( nextPosition ); - } else { - range = writer.createRange( nextPosition, selection.getLastPosition() ); - } - - writer.setSelection( range, { isBackward: selection.isBackward } ); + writer.setSelection( writer.createRange( nextPosition ), { isBackward: selection.isBackward } ); return true; } diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 4b71efb..1039cc8 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -698,26 +698,6 @@ describe( 'TodoListEditing', () => { '
                    ' ); } ); - - it( 'should move start of none-collapsed selection after checkmark element to the first text node', () => { - setModelData( model, '[Foo]' ); - - expect( getViewData( view ) ).to.equal( - '
                      ' + - '
                    • {Foo}
                      • ' + - '
                    ' - ); - } ); - - it( 'should move start of none-collapsed, backward selection after checkmark element to the first text node', () => { - setModelData( model, '[Foo]', { lastRangeBackward: true } ); - - expect( getViewData( view ) ).to.equal( - '
                      ' + - '
                    • {Foo}
                      • ' + - '
                    ' - ); - } ); } ); describe( 'uiElements view post-fixer', () => { @@ -951,7 +931,7 @@ describe( 'TodoListEditing', () => { expect( checkboxElement.checked ).to.equal( false ); - checkboxElement.dispatchEvent( new Event( 'mousedown' ) ); + checkboxElement.dispatchEvent( new Event( 'change' ) ); checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); @@ -964,7 +944,7 @@ describe( 'TodoListEditing', () => { 'b[a]r' ); - checkboxElement.dispatchEvent( new Event( 'mousedown' ) ); + checkboxElement.dispatchEvent( new Event( 'change' ) ); checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); @@ -992,7 +972,7 @@ describe( 'TodoListEditing', () => { expect( checkboxElement.checked ).to.equal( false ); - checkboxElement.dispatchEvent( new Event( 'mousedown' ) ); + checkboxElement.dispatchEvent( new Event( 'change' ) ); checkmarkViewElement = viewRoot.getChild( 0 ).getChild( 0 ).getChild( 0 ); checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); From c48cb3f8ae6052c88e576a1c87c85b831c318589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 09:58:37 +0200 Subject: [PATCH 33/42] Do not convert `todoListChecked` attribute on non-todo list item. --- src/todolistconverters.js | 6 ++++++ tests/todolistediting.js | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/todolistconverters.js b/src/todolistconverters.js index 8449cd8..e946b0c 100644 --- a/src/todolistconverters.js +++ b/src/todolistconverters.js @@ -262,6 +262,12 @@ export function modelViewChangeType( onCheckedChange ) { */ export function modelViewChangeChecked( onCheckedChange ) { return ( evt, data, conversionApi ) => { + // Do not convert `todoListChecked` attribute when todo list item has changed to other list item. + // This attribute will be removed by the model post fixer. + if ( data.item.getAttribute( 'listType' ) != 'todo' ) { + return; + } + if ( !conversionApi.consumable.consume( data.item, 'attribute:todoListChecked' ) ) { return; } diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 1039cc8..2b7875b 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -312,11 +312,17 @@ describe( 'TodoListEditing', () => { } ); it( 'should remove todoListChecked attribute when checked todoListItem is changed to regular list item', () => { - setModelData( model, 'fo[]o' ); + setModelData( model, + 'f[oo' + + 'fo]o' + ); editor.execute( 'bulletedList' ); - expect( getModelData( model ) ).to.equal( 'fo[]o' ); + expect( getModelData( model ) ).to.equal( + 'f[oo' + + 'fo]o' + ); } ); it( 'should be overwritable', () => { From e125d83478ee432b90dfaabe6a56f0e3aad65c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 10:27:31 +0200 Subject: [PATCH 34/42] Added more plugins to todo list manual test. --- tests/manual/todo-list.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/manual/todo-list.js b/tests/manual/todo-list.js index fa94cdc..b78cc9d 100644 --- a/tests/manual/todo-list.js +++ b/tests/manual/todo-list.js @@ -9,6 +9,9 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor' import Enter from '@ckeditor/ckeditor5-enter/src/enter'; import Typing from '@ckeditor/ckeditor5-typing/src/typing'; import Heading from '@ckeditor/ckeditor5-heading/src/heading'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; +import Highlight from '@ckeditor/ckeditor5-highlight/src/highlight'; +import Table from '@ckeditor/ckeditor5-table/src/table'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Undo from '@ckeditor/ckeditor5-undo/src/undo'; import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard'; @@ -17,8 +20,17 @@ import TodoList from '../../src/todolist'; ClassicEditor .create( document.querySelector( '#editor' ), { - plugins: [ Enter, Typing, Heading, Paragraph, Undo, List, TodoList, Clipboard ], - toolbar: [ 'heading', '|', 'bulletedList', 'numberedList', 'todoList', '|', 'undo', 'redo' ] + plugins: [ Enter, Typing, Heading, Highlight, Table, Bold, Paragraph, Undo, List, TodoList, Clipboard ], + toolbar: [ + 'heading', '|', 'bulletedList', 'numberedList', 'todoList', '|', 'bold', 'highlight', 'insertTable', '|', 'undo', 'redo' + ], + table: { + contentToolbar: [ + 'tableColumn', + 'tableRow', + 'mergeTableCells' + ] + } } ) .then( editor => { window.editor = editor; From 4a792cdceeb3a4356610568f73c98c4f10de6fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 11:03:57 +0200 Subject: [PATCH 35/42] Improved manual test. --- tests/manual/todo-list.html | 23 +++++----- tests/manual/todo-list.md | 86 +++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/tests/manual/todo-list.html b/tests/manual/todo-list.html index bc4914d..6149d19 100644 --- a/tests/manual/todo-list.html +++ b/tests/manual/todo-list.html @@ -2,21 +2,24 @@

                    This is a test for list feature.

                    Some more text for testing.

                      -
                    • Bullet list item 1
                      • -
                    • Bullet list item 2
                      • -
                    • Bullet list item 3
                      • -
                    • Bullet list item 4
                      • -
                    • Bullet list item 5
                      • -
                    • Bullet list item 6
                      • -
                    • Bullet list item 7
                      • -
                    • Bullet list item 8
                      • +
                    • Todo list item 1
                      • +
                    • Todo list item 2
                      • +
                    • Todo list item 3
                      • +
                    • Todo list item 4
                      • +
                    • Todo list item 5
                      • +
                    • Todo list item 6
                      • +
                    • Todo list item 7
                      • +
                    • Todo list item 8

                    Paragraph.

                    Another testing paragraph.

                      -
                    1. Numbered list item 1
                      2. +
                    2. Numbered list item
                      -
                    • Another bullet list
                      • +
                    • Todo list item
                      • +
                    +
                      +
                    • Bullet list
                    diff --git a/tests/manual/todo-list.md b/tests/manual/todo-list.md index e69de29..d5b06ea 100644 --- a/tests/manual/todo-list.md +++ b/tests/manual/todo-list.md @@ -0,0 +1,86 @@ +## Loading + +1. The data should be loaded with: + * two paragraphs, + * todo list with eight items, where 2,4 and 7 are checked + * two paragraphs, + * numbered list with one item, + * todo list with one unchecked item, + * bullet list with one item. +2. Toolbar should have two buttons: for bullet and for numbered list. + +## Testing + +### Creating: + +1. Convert first paragraph to todo list item +2. Create empty paragraph and convert to todo list item +3. Press `Enter` in the middle of item +4. Press `Enter` at the start of item +5. Press `Enter` at the end of item + +### Removing: + +1. Delete all contents from list item and then the list item +2. Press enter in empty list item +3. Click on highlighted button ("turn off" list feature) +4. Do it for first, second and last list item + +### Changing type: + +1. Change type from todo to numbered for checked and unchecked list item +3. Do it for multiple items at once + +### Merging: + +1. Convert paragraph before todo list to same type of list +2. Convert paragraph after todo list to same type of list +3. Convert paragraph before todo list to different type of list +4. Convert paragraph after todo list to different type of list +5. Convert first paragraph to todo list, then convert second paragraph to todo list +6. Convert multiple items and paragraphs at once + +### Toggling check state: + +1. Put selection in the middle of unchecked the todo list item +2. Check list item (selection should not move) + +--- + +1. Select multiple todo list items +2. Check or uncheck todo list item (selection should not move) + +--- + +1. Check todo list item +2. Convert checked list item to other list item +3. Convert this list item once again to todo list item ()should be unchecked) + +--- + +1. Put collapsed selection to todo list item +2. Press `Ctrl+Space` (check state should toggle) + +### Toggling check state for multiple items: + +1. Select two unchecked list items +2. Press `Ctrl+Space` (both should be checked) +3. Press `Ctrl+Space` once again (both should be unchecked) + +--- + +1. Select checked and unchecked list item +2. Press `Ctrl+Space` (both should be checked) + +--- + +1. Select the entire content +2. Press `Ctrl+Space` (all todo list items should be checked) +3. Press `Ctrl+Space` once again (all todo list items should be unchecked) + +### Integration with attribute elements: + +1. Select multiple todo list items +2. Highlight selected text +3. Check or uncheck highlighted todo list item +4. Type inside highlighted todo list item From 890cd8af0ce97237bbe52dc1685d1d1a0e914ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 11:21:02 +0200 Subject: [PATCH 36/42] Docs. --- src/todolistcheckcommand.js | 10 +++++++++- src/todolistediting.js | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/todolistcheckcommand.js b/src/todolistcheckcommand.js index c2d835d..cf78a2a 100644 --- a/src/todolistcheckcommand.js +++ b/src/todolistcheckcommand.js @@ -27,7 +27,15 @@ export default class TodoListCheckCommand extends Command { * * @observable * @readonly - * @member {Boolean} #value + * @member {Boolean} #isEnabled + */ + + /** + * A List of todo list item selected by the {@link module:engine/model/selection~Selection}. + * + * @observable + * @readonly + * @member {Array.} #value */ /** diff --git a/src/todolistediting.js b/src/todolistediting.js index c396513..2758113 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -96,7 +96,7 @@ export default class TodoListEditing extends Plugin { this.listenTo( viewRoot, 'change:children', ( evt, node ) => changedViewNodes.add( node ) ); } - // Move selection after a checkbox element. + // Move all uiElements after a checkmark element. viewDocument.registerPostFixer( writer => { const changedCheckmarkElements = getChangedCheckmarkElements( writer, changedViewNodes ); @@ -105,7 +105,7 @@ export default class TodoListEditing extends Plugin { return moveUIElementsAfterCheckmark( writer, changedCheckmarkElements ); } ); - // Move all uiElements after a checkbox element. + // Move selection after a checkmark element. viewDocument.registerPostFixer( writer => moveSelectionAfterCheckmark( writer, viewDocument.selection ) ); // Jump at the end of the previous node on left arrow key press, when selection is after the checkbox. @@ -158,7 +158,7 @@ export default class TodoListEditing extends Plugin { } /** - * Handles checkmar element change, moves selection to the corresponding model item to makes possible + * Handles checkmark element change, moves selection to the corresponding model item to makes it possible * to toggle `todoListChecked` attribute using command and restore the selection position. * * Some say it's a hack :) Moving selection only for executing the command on a certain node and restoring it after, From 597c479af234ab3471aaecdeb750a770dbfcd3e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 12:46:31 +0200 Subject: [PATCH 37/42] Improved tests. --- tests/todolistediting.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 2b7875b..54a022a 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -9,6 +9,7 @@ import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting'; import Typing from '@ckeditor/ckeditor5-typing/src/typing'; import ListCommand from '../src/listcommand'; +import TodoListCheckCommand from '../src/todolistcheckcommand'; import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; @@ -66,7 +67,7 @@ describe( 'TodoListEditing', () => { expect( model.schema.checkAttribute( [ '$root', paragraph ], 'todoListChecked' ) ).to.be.false; } ); - describe( 'command', () => { + describe( 'commands', () => { it( 'should register todoList list command', () => { const command = editor.commands.get( 'todoList' ); @@ -110,6 +111,10 @@ describe( 'TodoListEditing', () => { expect( getModelData( model ) ).to.equal( 'ab[]' ); expect( getViewData( view ) ).to.equal( '

                    ab{}

                    ' ); } ); + + it( 'should register todoListCheck command', () => { + expect( editor.commands.get( 'todoListCheck' ) ).to.be.instanceOf( TodoListCheckCommand ); + } ); } ); describe( 'editing pipeline', () => { From 42035d439578e15b6635fdac59ca550b7673960a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 13:12:48 +0200 Subject: [PATCH 38/42] Changed view post fixers to work properly with dynamic roots. --- src/todolistediting.js | 7 +++++-- tests/todolistediting.js | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/todolistediting.js b/src/todolistediting.js index 2758113..d547a7c 100644 --- a/src/todolistediting.js +++ b/src/todolistediting.js @@ -92,8 +92,11 @@ export default class TodoListEditing extends Plugin { // be re-rendered. If yes than view post-fixer should verify view structure. const changedViewNodes = new Set(); - for ( const viewRoot of viewDocument.roots ) { - this.listenTo( viewRoot, 'change:children', ( evt, node ) => changedViewNodes.add( node ) ); + Array.from( viewDocument.roots ).forEach( watchRootForViewChildChanges ); + this.listenTo( viewDocument.roots, 'add', ( evt, root ) => watchRootForViewChildChanges( root ) ); + + function watchRootForViewChildChanges( viewRoot ) { + viewRoot.on( 'change:children', ( evt, node ) => changedViewNodes.add( node ) ); } // Move all uiElements after a checkmark element. diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 54a022a..5034ff4 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -11,6 +11,7 @@ import Typing from '@ckeditor/ckeditor5-typing/src/typing'; import ListCommand from '../src/listcommand'; import TodoListCheckCommand from '../src/todolistcheckcommand'; import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element'; +import InlineEditableUIView from '@ckeditor/ckeditor5-ui/src/editableui/inline/inlineeditableuiview'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; @@ -875,7 +876,7 @@ describe( 'TodoListEditing', () => { } ); describe( 'TodoListEditing', () => { - let editorElement, editor, model, view, viewDoc, viewRoot; + let editorElement, editor, model, modelDoc, view, viewDoc, viewRoot; beforeEach( () => { editorElement = document.createElement( 'div' ); @@ -889,6 +890,7 @@ describe( 'TodoListEditing', () => { editor = newEditor; model = editor.model; + modelDoc = model.document; view = editor.editing.view; viewDoc = view.document; @@ -995,4 +997,39 @@ describe( 'TodoListEditing', () => { 'f[]oo' ); } ); + + it( 'should toggle `todoListChecked` state using command in root created in a runtime', () => { + const dynamicRootElement = document.createElement( 'div' ); + const dynamicRootEditable = new InlineEditableUIView( editor.locale, view, dynamicRootElement ); + + document.body.appendChild( dynamicRootElement ); + + modelDoc.createRoot( '$root', 'dynamicRoot' ); + dynamicRootEditable.name = 'dynamicRoot'; + view.attachDomRoot( dynamicRootElement, 'dynamicRoot' ); + + const command = editor.commands.get( 'todoListCheck' ); + + sinon.spy( command, 'execute' ); + + setModelData( model, 'f[]oo', { rootName: 'dynamicRoot' } ); + + let checkmarkViewElement = viewDoc.getRoot( 'dynamicRoot' ).getChild( 0 ).getChild( 0 ).getChild( 0 ); + let checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + let checkboxElement = checkmarkDomElement.children[ 0 ]; + + expect( checkboxElement.checked ).to.equal( false ); + + checkboxElement.dispatchEvent( new Event( 'change' ) ); + + checkmarkViewElement = viewDoc.getRoot( 'dynamicRoot' ).getChild( 0 ).getChild( 0 ).getChild( 0 ); + checkmarkDomElement = view.domConverter.mapViewToDom( checkmarkViewElement ); + checkboxElement = checkmarkDomElement.children[ 0 ]; + + sinon.assert.calledOnce( command.execute ); + expect( checkboxElement.checked ).to.equal( true ); + expect( getModelData( model, { rootName: 'dynamicRoot' } ) ).to.equal( + 'f{}oo' + ); + } ); } ); From 2229454e44c5f7e0a41456b5f9e4b9567715bb21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 13:15:52 +0200 Subject: [PATCH 39/42] Added missing dependencies. --- package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package.json b/package.json index d49d125..5af58bf 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,10 @@ "@ckeditor/ckeditor5-editor-classic": "^12.1.3", "@ckeditor/ckeditor5-enter": "^11.0.4", "@ckeditor/ckeditor5-heading": "^11.0.4", + "@ckeditor/ckeditor5-highlight": "^11.0.4", "@ckeditor/ckeditor5-indent": "^10.0.1", "@ckeditor/ckeditor5-link": "^11.1.1", + "@ckeditor/ckeditor5-table": "^13.0.2", "@ckeditor/ckeditor5-typing": "^12.1.1", "@ckeditor/ckeditor5-undo": "^11.0.4", "eslint": "^5.5.0", From 7b0010908f3402b5990fdbe0c62c5b4e57d44896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 13:17:14 +0200 Subject: [PATCH 40/42] Fixed failing test. --- tests/todolistediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/todolistediting.js b/tests/todolistediting.js index 5034ff4..2044ce3 100644 --- a/tests/todolistediting.js +++ b/tests/todolistediting.js @@ -1029,7 +1029,7 @@ describe( 'TodoListEditing', () => { sinon.assert.calledOnce( command.execute ); expect( checkboxElement.checked ).to.equal( true ); expect( getModelData( model, { rootName: 'dynamicRoot' } ) ).to.equal( - 'f{}oo' + 'f[]oo' ); } ); } ); From 6e96eaefd6d46f223b069d8b519dbfdbd21cad09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 14:53:41 +0200 Subject: [PATCH 41/42] Manual test description improvement. --- tests/manual/todo-list.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/manual/todo-list.md b/tests/manual/todo-list.md index d5b06ea..f9b3cf4 100644 --- a/tests/manual/todo-list.md +++ b/tests/manual/todo-list.md @@ -2,12 +2,12 @@ 1. The data should be loaded with: * two paragraphs, - * todo list with eight items, where 2,4 and 7 are checked + * todo list with eight items, where 2,4 and 7 are checked, * two paragraphs, * numbered list with one item, * todo list with one unchecked item, * bullet list with one item. -2. Toolbar should have two buttons: for bullet and for numbered list. +2. Toolbar should have three buttons: for bullet, numbered and todo list. ## Testing From 5944925f857985f452592253d8e937ae289617f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 13 Aug 2019 15:22:43 +0200 Subject: [PATCH 42/42] Fixed invalid docs. --- src/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.js b/src/utils.js index 9430357..d8e1b62 100644 --- a/src/utils.js +++ b/src/utils.js @@ -175,7 +175,7 @@ export function mergeViewLists( viewWriter, firstList, secondList ) { * are after given position. * * For example: - * foo^bar + * `foo^bar` * For position ^, a position before "bar" will be returned. * * @param {module:engine/view/position~Position} viewPosition