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

Commit

Permalink
Merge pull request #154 from ckeditor/t/ckeditor5/2009
Browse files Browse the repository at this point in the history 
Fix: Use model-to-view position mapping in todo lists. Closes ckeditor/ckeditor5#2009. Closed ckeditor/ckeditor5#1980.
  • Loading branch information
@Reinmar
Reinmar authored Oct 3, 2019
2 parents 63deb51 + 9a881d2 commit ff460f8
Show file tree
Hide file tree
Showing 5 changed files with 259 additions and 302 deletions.
96 changes: 56 additions & 40 deletions src/todolistconverters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

/* global document */

import { generateLiInUl, injectViewList, findInRange } from './utils';
import { generateLiInUl, injectViewList } from './utils';
import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';

/**
Expand Down Expand Up @@ -59,41 +59,6 @@ export function modelViewInsertion( model, onCheckboxChecked ) {
};
}

/**
* A model-to-view converter for the model `$text` element inside a to-do 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.
* @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface.
*/
export function modelViewTextInsertion( evt, data, conversionApi ) {
const parent = data.range.start.parent;

if ( parent.name != 'listItem' || parent.getAttribute( 'listType' ) != 'todo' ) {
return;
}

if ( !conversionApi.consumable.consume( data.item, 'insert' ) ) {
return;
}

const viewWriter = conversionApi.writer;
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: <listItem listtype="todo">[foo]</listItem>
// view: <li>^<checkbox/></li> -> <li><checkbox/>foo</li>
viewWriter.insert( viewPosition.offset ? viewPosition : viewPosition.getShiftedBy( 1 ), viewText );
}

/**
* A model-to-view converter for the `listItem` model element insertion.
*
Expand Down Expand Up @@ -233,7 +198,7 @@ export function dataViewModelCheckmarkInsertion( evt, data, conversionApi ) {
* @param {Function} onCheckedChange Callback fired after clicking the checkbox UI element.
* @returns {Function} Returns a conversion callback.
*/
export function modelViewChangeType( onCheckedChange ) {
export function modelViewChangeType( onCheckedChange, view ) {
return ( evt, data, conversionApi ) => {
const viewItem = conversionApi.mapper.toViewElement( data.item );
const viewWriter = conversionApi.writer;
Expand All @@ -246,7 +211,7 @@ export function modelViewChangeType( onCheckedChange ) {
viewWriter.insert( viewWriter.createPositionAt( viewItem, 0 ), checkmarkElement );
} else if ( data.attributeOldValue == 'todo' ) {
viewWriter.removeClass( 'todo-list', viewItem.parent );
viewWriter.remove( viewItem.getChild( 0 ) );
viewWriter.remove( findLabel( viewItem, view ) );
}
};
}
Expand Down Expand Up @@ -277,15 +242,55 @@ export function modelViewChangeChecked( onCheckedChange ) {
const { mapper, writer: viewWriter } = conversionApi;
const isChecked = !!data.item.getAttribute( 'todoListChecked' );
const viewItem = mapper.toViewElement( data.item );
const itemRange = viewWriter.createRangeIn( viewItem );
const oldCheckmarkElement = findInRange( itemRange, item => item.is( 'uiElement' ) ? item : false );
// Because of m -> v position mapper we can be sure checkbox is always at the beginning.
const oldCheckmarkElement = viewItem.getChild( 0 );
const newCheckmarkElement = createCheckmarkElement( data.item, viewWriter, isChecked, onCheckedChange );

viewWriter.insert( viewWriter.createPositionAfter( oldCheckmarkElement ), newCheckmarkElement );
viewWriter.remove( oldCheckmarkElement );
};
}

/**
* A model-to-view position at zero offset mapper.
*
* This helper ensures that position inside todo-list in the view is mapped after the checkbox.
*
* It only handles the position at the beginning of a list item as other positions are properly mapped be the default mapper.
*
* @param {module:engine/view/view~View} view
* @param {module:engine/conversion/mapper~Mapper} mapper
* @return {Function}
*/
export function mapModelToViewZeroOffsetPosition( view, mapper ) {
return ( evt, data ) => {
const modelPosition = data.modelPosition;
const parent = modelPosition.parent;

// Handle only position at the beginning of a todo list item.
if ( !parent.is( 'listItem' ) || parent.getAttribute( 'listType' ) != 'todo' || modelPosition.offset !== 0 ) {
return;
}

const viewLi = mapper.toViewElement( parent );
const label = findLabel( viewLi, view );

// If there is no label then most probably the default converter was overridden.
if ( !label ) {
return;
}

// Map the position to the next sibling (if it is not a marker) - most likely it will be a text node...
if ( label.nextSibling && !label.nextSibling.is( 'uiElement' ) ) {
data.viewPosition = view.createPositionAt( label.nextSibling, 0 );
}
// ... otherwise return position after the label.
else {
data.viewPosition = view.createPositionAfter( label );
}
};
}

// Creates a checkbox UI element.
//
// @private
Expand Down Expand Up @@ -321,3 +326,14 @@ function createCheckmarkElement( modelItem, viewWriter, isChecked, onChange ) {

return uiElement;
}

// Helper method to find label element inside li.
function findLabel( viewItem, view ) {
const range = view.createRangeIn( viewItem );

for ( const value of range ) {
if ( value.item.is( 'uiElement', 'label' ) ) {
return value.item;
}
}
}
135 changes: 6 additions & 129 deletions src/todolistediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,15 @@ import TodoListCheckCommand from './todolistcheckcommand';
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';

import {
modelViewInsertion,
modelViewTextInsertion,
dataModelViewInsertion,
dataModelViewTextInsertion,
dataViewModelCheckmarkInsertion,
mapModelToViewZeroOffsetPosition,
modelViewChangeChecked,
modelViewChangeType
modelViewChangeType,
modelViewInsertion
} from './todolistconverters';

import { findInRange } from './utils';

/**
* The engine of the to-do list feature. It handles creating, editing and removing to-do lists and their items.
*
Expand All @@ -47,7 +45,6 @@ export default class TodoListEditing extends Plugin {
init() {
const editor = this.editor;
const { editing, data, model } = editor;
const viewDocument = editing.view.document;

// Extend schema.
model.schema.extend( 'listItem', {
Expand All @@ -73,43 +70,21 @@ export default class TodoListEditing extends Plugin {
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( listItem => this._handleCheckmarkChange( listItem ) )
modelViewChangeType( listItem => this._handleCheckmarkChange( listItem ), editing.view )
);
editing.downcastDispatcher.on(
'attribute:todoListChecked:listItem',
modelViewChangeChecked( listItem => this._handleCheckmarkChange( listItem ) )
);

data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } );

// Collect all view nodes that have changed and use it to check if the checkbox UI element is going to
// be re-rendered. If yes than view post-fixer should verify view structure.
const changedViewNodes = new Set();

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 checkbox element.
viewDocument.registerPostFixer( writer => {
const changedCheckmarkElements = getChangedCheckmarkElements( writer, changedViewNodes );
editing.mapper.on( 'modelToViewPosition', mapModelToViewZeroOffsetPosition( editing.view, editing.mapper ) );

changedViewNodes.clear();

return moveUIElementsAfterCheckmark( writer, changedCheckmarkElements );
} );

// Move selection after a checkbox element.
viewDocument.registerPostFixer( writer => moveSelectionAfterCheckmark( writer, viewDocument.selection ) );
data.upcastDispatcher.on( 'element:input', dataViewModelCheckmarkInsertion, { priority: 'high' } );

// Jump at the end of the previous node on left arrow key press, when selection is after the checkbox.
//
Expand Down Expand Up @@ -188,71 +163,6 @@ export default class TodoListEditing extends Plugin {
}
}

// Moves all UI elements in the to-do list item after the checkbox element.
//
// @private
// @param {module:engine/view/downcastwriter~DowncastWriter} writer
// @param {Array.<module:engine/view/uielement~UIElement>} uiElements
// @returns {Boolean}
function moveUIElementsAfterCheckmark( writer, uiElements ) {
let hasChanged = false;

for ( const uiElement of uiElements ) {
const listItem = findViewListItemAncestor( 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;
}

// Moves the selection in the to-do list item after the checkbox element.
//
// @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 positionToChange = selection.getFirstPosition();

if ( positionToChange.parent.name != 'li' || !positionToChange.parent.parent.hasClass( 'todo-list' ) ) {
return false;
}

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 && !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( writer.createRange( nextPosition ), { isBackward: selection.isBackward } );

return true;
}

return false;
}

// Handles the left/right (LTR/RTL content) arrow key and moves the selection at the end of the previous block element
// if the selection is just after the checkbox element. In other words, it jumps over the checkbox element when
// moving the selection to the left/right (LTR/RTL).
Expand Down Expand Up @@ -280,36 +190,3 @@ function jumpOverCheckmarkOnSideArrowKeyPress( stopKeyEvent, model ) {
}
}
}

// Gets the list of all checkbox elements that are going to be rendered.
//
// @private
// @param {module:engine/view/view~View>} editingView
// @param {Set.<module:engine/view/element~Element>} changedViewNodes
// @returns {Array.<module:engine/view/uielement~UIElement>}
function getChangedCheckmarkElements( editingView, changedViewNodes ) {
const elements = [];

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 ) && element.document ) {
elements.push( item );
}
}
}

return elements;
}

// Returns the list item ancestor of a given element.
//
// @private
// @param {module:engine/view/item~Item} item
// @returns {module:engine/view/element~Element}
function findViewListItemAncestor( item ) {
for ( const parent of item.getAncestors( { parentFirst: true } ) ) {
if ( parent.name == 'li' ) {
return parent;
}
}
}
10 changes: 0 additions & 10 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,6 @@ 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 a UI button and linking it with an appropriate command.
*
Expand Down
5 changes: 3 additions & 2 deletions tests/manual/todo-list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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 Link from '@ckeditor/ckeditor5-link/src/link';
import Table from '@ckeditor/ckeditor5-table/src/table';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Undo from '@ckeditor/ckeditor5-undo/src/undo';
Expand All @@ -21,13 +22,13 @@ import TodoList from '../../src/todolist';

ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ Enter, Typing, Heading, Highlight, Table, Bold, Paragraph, Undo, List, TodoList, Clipboard, FontSize ],
plugins: [ Enter, Typing, Heading, Highlight, Table, Bold, Paragraph, Undo, List, TodoList, Clipboard, Link, FontSize ],
toolbar: [
'heading',
'|',
'bulletedList', 'numberedList', 'todoList',
'|',
'bold', 'highlight', 'insertTable', 'fontSize',
'bold', 'link', 'highlight', 'insertTable', 'fontSize',
'|',
'undo', 'redo'
],
Expand Down
Loading

0 comments on commit ff460f8

Please sign in to comment.