Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

To-do lists in document lists #14700

Merged
merged 62 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
3fe0d1c
Initial PoC of to-do lists in document lists.
niegowski Jul 31, 2023
21448e3
Extend to-do list upcast.
niegowski Aug 4, 2023
d3d8e8c
The to-do list item should be properly upcasted even if wrapped with …
niegowski Aug 10, 2023
aa63392
Merge branch 'master' into ck/14663-to-do-lists-poc
niegowski Aug 17, 2023
4705bf3
Renamed Todo plugin to TodoDocumentList.
niegowski Aug 17, 2023
56b2251
Added command to toggle to-do list items.
niegowski Aug 17, 2023
76a59fb
Allow converting any block type to a to-do list item.
niegowski Aug 18, 2023
8205c7f
WiP.
niegowski Aug 18, 2023
f8799ab
Merge branch 'master' into ck/14663-to-do-lists-poc
niegowski Aug 25, 2023
3df3875
Introducing a generic list item marker downcast strategy (PoC).
niegowski Aug 25, 2023
6386b7c
Review fixes.
niegowski Aug 28, 2023
2045bea
Code cleanup and refactoring.
niegowski Aug 28, 2023
e71aeff
Generic strategies for downcast item marker.
niegowski Aug 28, 2023
b735dab
Reverted changes in GHS coupled attributes.
niegowski Aug 28, 2023
b5fb187
Code cleanup.
niegowski Aug 28, 2023
9c3eb52
Cleaning manual test.
niegowski Aug 28, 2023
3a0ffeb
Fixed Blink glitch.
niegowski Aug 28, 2023
f4a7309
Adding tests.
niegowski Aug 28, 2023
717e5bb
First block of to-do list item should be wrapped in description span …
niegowski Aug 29, 2023
2621462
Review fixes.
niegowski Aug 30, 2023
a1d530f
Checkbox should not be allowed between blocks of a single list item.
niegowski Aug 31, 2023
8e83b68
Code cleaning.
niegowski Sep 1, 2023
97bec24
Handle arrow keys before a checkbox.
niegowski Sep 1, 2023
e89e4c6
Added code comments and doclets.
niegowski Sep 1, 2023
05289c2
Updated manual tests.
niegowski Sep 4, 2023
d9ea416
Merge branch 'master' into ck/14663-to-do-lists-poc
niegowski Sep 4, 2023
0cb7df6
WiP css.
niegowski Sep 5, 2023
2fabd69
Unified to-do list styles.
niegowski Sep 6, 2023
aa9238e
Merge branch 'master' into ck/14663-to-do-lists-poc
niegowski Sep 7, 2023
75f0c75
The label element can't be rendered in the editing view because it in…
niegowski Sep 7, 2023
1be5de4
Fixed RTL styles.
niegowski Sep 8, 2023
b993cc7
Handling view positions inside the checkbox wrappers.
niegowski Sep 12, 2023
b9cfab7
Merge remote-tracking branch 'origin/ck/14663-to-do-lists-poc' into o…
mmotyczynska Sep 12, 2023
60c913d
Tests: todo document list conversion (wip).
mmotyczynska Sep 12, 2023
a62a386
Merge branch 'master' into ck/14663-to-do-lists-poc
niegowski Sep 12, 2023
b313a69
Merge branch 'origin/ck/14663-to-do-lists-poc-tests' into ck/14663-to…
mmotyczynska Sep 12, 2023
9fcd90e
Adding tests.
niegowski Sep 13, 2023
f163b70
Tests: todo document list command (wip).
mmotyczynska Sep 14, 2023
0ec1f70
Adding tests.
niegowski Sep 14, 2023
d641ec9
Adding tests.
niegowski Sep 15, 2023
b44ed26
Tests: check todo document list command.
mmotyczynska Sep 15, 2023
153877a
Tests: todo checkbox change observer.
mmotyczynska Sep 15, 2023
40c7511
Adding tests.
niegowski Sep 18, 2023
da290f5
Added tests.
niegowski Sep 18, 2023
2419eb2
Adding tests.
niegowski Sep 18, 2023
7b20945
Tests: todo list item postfixers and user interactions.
mmotyczynska Sep 18, 2023
00d0db9
Fixed checkbox toggling.
niegowski Sep 19, 2023
f13f328
Tests: arrow keys in todo list items.
mmotyczynska Sep 19, 2023
02a8e61
Added test.
niegowski Sep 19, 2023
636ec2c
Adding tests.
niegowski Sep 20, 2023
17c31ed
Adding tests.
niegowski Sep 20, 2023
f10cda7
Adding tests.
niegowski Sep 20, 2023
3795143
Added missing comments.
niegowski Sep 20, 2023
f00e6fc
Fixed test.
niegowski Sep 20, 2023
9724e17
Fixed white-list of allowed attributes on code-block.
niegowski Sep 20, 2023
23434fc
Fixed to-do list item alignment.
niegowski Sep 20, 2023
7b183f3
Adding tests.
niegowski Sep 20, 2023
0374893
Removed the checkbox toggling animation to avoid glitches while reusi…
niegowski Sep 20, 2023
aa4b2bf
Fixed tests.
niegowski Sep 20, 2023
960220d
The inline html elements should not be formatted in source editing (i…
niegowski Sep 20, 2023
4bb162e
Merge branch 'master' into ck/14663-to-do-lists-poc
niegowski Sep 20, 2023
7e4b6f6
Apply suggestions from code review.
arkflpc Sep 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 58 additions & 30 deletions packages/ckeditor5-html-support/src/datafilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export default class DataFilter extends Plugin {
/**
* Allowed element definitions by {@link module:html-support/datafilter~DataFilter#allowElement} method.
*/
private readonly _allowedElements: Set<DataSchemaDefinition & { coupledAttribute?: string }>;
private readonly _allowedElements: Set<DataSchemaBlockElementDefinition | DataSchemaInlineElementDefinition>;

/**
* Disallowed element names by {@link module:html-support/datafilter~DataFilter#disallowElement} method.
Expand All @@ -128,22 +128,22 @@ export default class DataFilter extends Plugin {
*/
private _coupledAttributes: Map<string, Array<string>> | null;

/**
* TODO
*/
private _coupledGHSAttributes: Map<string, string> | null;

constructor( editor: Editor ) {
super( editor );

this._dataSchema = editor.plugins.get( 'DataSchema' );

this._allowedAttributes = new Matcher();

this._disallowedAttributes = new Matcher();

this._allowedElements = new Set();

this._disallowedElements = new Set();

this._dataInitialized = false;

this._coupledAttributes = null;
this._coupledGHSAttributes = null;

this._registerElementsAfterInit();
this._registerElementHandlers();
Expand Down Expand Up @@ -449,27 +449,44 @@ export default class DataFilter extends Plugin {
const changes = model.document.differ.getChanges();
let changed = false;

const coupledAttributes = this._getCoupledAttributesMap();
const [ coupledAttributes, coupledGHSAttributes ] = this._getCoupledAttributesMaps();

for ( const change of changes ) {
// Handle only attribute removals.
if ( change.type != 'attribute' || change.attributeNewValue !== null ) {
continue;
}
// Handle attribute removals.
if ( change.type == 'attribute' && change.attributeNewValue === null ) {
// Find a list of coupled GHS attributes.
const attributeKeys = coupledAttributes.get( change.attributeKey );

// Find a list of coupled GHS attributes.
const attributeKeys = coupledAttributes.get( change.attributeKey );
if ( !attributeKeys ) {
continue;
}

if ( !attributeKeys ) {
continue;
// Remove the coupled GHS attributes on the same range as the feature attribute was removed.
for ( const { item } of change.range.getWalker( { shallow: true } ) ) {
for ( const attributeKey of attributeKeys ) {
if ( item.hasAttribute( attributeKey ) ) {
writer.removeAttribute( attributeKey, item );
changed = true;
}
}
}
}
// Handle element rename (and attribute remove on it).
// Example: <paragraph htmlLiAttributes listItemId> -> <heading1>
// The GHS element attributes can't exist without coupled feature attribute (for example in lists).
else if ( change.type == 'insert' && change.name != '$text' ) {
for ( const { item } of writer.createRangeOn( change.position.nodeAfter! ) ) {
if ( !item.is( 'element' ) ) {
continue;
}

for ( const [ key ] of item.getAttributes() ) {
const coupledAttribute = coupledGHSAttributes.get( key );

// Remove the coupled GHS attributes on the same range as the feature attribute was removed.
for ( const { item } of change.range.getWalker( { shallow: true } ) ) {
for ( const attributeKey of attributeKeys ) {
if ( item.hasAttribute( attributeKey ) ) {
writer.removeAttribute( attributeKey, item );
changed = true;
if ( coupledAttribute && !item.hasAttribute( coupledAttribute ) ) {
writer.removeAttribute( key, item );
changed = true;
}
}
}
}
Expand Down Expand Up @@ -540,26 +557,37 @@ export default class DataFilter extends Plugin {
* Collects the map of coupled attributes. The returned map is keyed by the feature attribute name
* and coupled GHS attribute names are stored in the value array.
*/
private _getCoupledAttributesMap(): Map<string, Array<string>> {
if ( this._coupledAttributes ) {
return this._coupledAttributes;
private _getCoupledAttributesMaps(): [ Map<string, Array<string>>, Map<string, string> ] {
if ( this._coupledAttributes && this._coupledGHSAttributes ) {
return [ this._coupledAttributes, this._coupledGHSAttributes ];
}

this._coupledAttributes = new Map<string, Array<string>>();
this._coupledGHSAttributes = new Map<string, string>();

for ( const definition of this._allowedElements ) {
if ( definition.coupledAttribute && definition.model ) {
const attributeNames = this._coupledAttributes.get( definition.coupledAttribute );
if ( !definition.isInline ) {
continue;
}

const inlineDefinition = definition as DataSchemaInlineElementDefinition;

if ( inlineDefinition.coupledAttribute && inlineDefinition.model ) {
const attributeNames = this._coupledAttributes.get( inlineDefinition.coupledAttribute );

if ( attributeNames ) {
attributeNames.push( definition.model );
attributeNames.push( inlineDefinition.model );
} else {
this._coupledAttributes.set( definition.coupledAttribute, [ definition.model ] );
this._coupledAttributes.set( inlineDefinition.coupledAttribute, [ inlineDefinition.model ] );
}

if ( inlineDefinition.appliesToBlock ) {
this._coupledGHSAttributes.set( inlineDefinition.model, inlineDefinition.coupledAttribute );
}
}
}

return this._coupledAttributes;
return [ this._coupledAttributes, this._coupledGHSAttributes ];
}

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/ckeditor5-html-support/src/integrations/documentlist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export default class DocumentListElementSupport extends Plugin {
for ( const { node } of listNodes ) {
const listType = node.getAttribute( 'listType' );

if ( listType === 'bulleted' && node.getAttribute( 'htmlOlAttributes' ) ) {
if ( listType !== 'numbered' && node.getAttribute( 'htmlOlAttributes' ) ) {
writer.removeAttribute( 'htmlOlAttributes', node );
evt.return = true;
}
Expand Down Expand Up @@ -256,8 +256,8 @@ function viewToModelListAttributeConverter( attributeName: string, dataFilter: D
/**
* Returns HTML attribute name based on provided list type.
*/
function getAttributeFromListType( listType: 'bulleted' | 'numbered' ) {
return listType === 'bulleted' ?
'htmlUlAttributes' :
'htmlOlAttributes';
function getAttributeFromListType( listType: string ) {
return listType === 'numbered' ?
'htmlOlAttributes' :
'htmlUlAttributes';
}
11 changes: 8 additions & 3 deletions packages/ckeditor5-list/src/augmentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import type {
TodoList,
TodoListEditing,
TodoListUI,
TodoDocumentList,
TodoDocumentListEditing,

ListCommand,
DocumentListCommand,
Expand All @@ -36,7 +38,8 @@ import type {
DocumentListStartCommand,
ListReversedCommand,
DocumentListReversedCommand,
CheckTodoListCommand
CheckTodoListCommand,
CheckTodoDocumentListCommand
} from '.';

declare module '@ckeditor/ckeditor5-core' {
Expand Down Expand Up @@ -69,6 +72,8 @@ declare module '@ckeditor/ckeditor5-core' {
[ TodoList.pluginName ]: TodoList;
[ TodoListEditing.pluginName ]: TodoListEditing;
[ TodoListUI.pluginName ]: TodoListUI;
[ TodoDocumentList.pluginName ]: TodoDocumentList;
[ TodoDocumentListEditing.pluginName ]: TodoDocumentListEditing;
}

interface CommandsMap {
Expand All @@ -83,7 +88,7 @@ declare module '@ckeditor/ckeditor5-core' {
listStyle: ListStyleCommand | DocumentListStyleCommand;
listStart: ListStartCommand | DocumentListStartCommand;
listReversed: ListReversedCommand | DocumentListReversedCommand;
todoList: ListCommand;
checkTodoList: CheckTodoListCommand;
todoList: ListCommand | DocumentListCommand;
checkTodoList: CheckTodoListCommand | CheckTodoDocumentListCommand;
}
}
9 changes: 7 additions & 2 deletions packages/ckeditor5-list/src/documentlist/converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,13 @@ export function listItemUpcastConverter(): GetCallback<UpcastElementEvent> {

for ( const item of items ) {
// Set list attributes only on same level items, those nested deeper are already handled by the recursive conversion.
if ( !isListItemBlock( item ) ) {
writer.setAttributes( attributes, item );
if ( !item.hasAttribute( 'listItemId' ) ) {
// Preserve list type if was already set (for example by to-do list feature).
if ( item.getAttribute( 'listType' ) ) {
writer.setAttributes( { ...attributes, listType: item.getAttribute( 'listType' ) }, item );
} else {
writer.setAttributes( attributes, item );
}
}
}

Expand Down
30 changes: 25 additions & 5 deletions packages/ckeditor5-list/src/documentlist/documentlistcommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import {
ListItemUid,
sortBlocks,
getSelectedBlockObject,
isListItemBlock
isListItemBlock,
checkCanBeRenamed
} from './utils/model';

/**
Expand All @@ -29,7 +30,7 @@ export default class DocumentListCommand extends Command {
/**
* The type of the list created by the command.
*/
public readonly type: 'numbered' | 'bulleted';
public readonly type: 'numbered' | 'bulleted' | 'todo';

/**
* A flag indicating whether the command is active, which means that the selection starts in a list of the same type.
Expand All @@ -39,16 +40,22 @@ export default class DocumentListCommand extends Command {
*/
public declare value: boolean;

/**
* TODO
*/
private _requiredElementName?: string;

/**
* Creates an instance of the command.
*
* @param editor The editor instance.
* @param type List type that will be handled by this command.
*/
constructor( editor: Editor, type: 'numbered' | 'bulleted' ) {
constructor( editor: Editor, type: 'numbered' | 'bulleted' | 'todo', requiredElementName?: string ) {
super( editor );

this.type = type;
this._requiredElementName = requiredElementName;
}

/**
Expand All @@ -75,7 +82,10 @@ export default class DocumentListCommand extends Command {
const selectedBlockObject = getSelectedBlockObject( model );

const blocks = Array.from( document.selection.getSelectedBlocks() )
.filter( block => model.schema.checkAttribute( block, 'listType' ) );
.filter( block => (
model.schema.checkAttribute( block, 'listType' ) ||
this._requiredElementName && checkCanBeRenamed( block, model.schema, this._requiredElementName )
) );

// Whether we are turning off some items.
const turnOff = options.forceValue !== undefined ? !options.forceValue : this.value;
Expand All @@ -100,7 +110,7 @@ export default class DocumentListCommand extends Command {

this._fireAfterExecute( changedBlocks );
}
// Turning on the list items for a collapsed selection inside a list item.
// Changing type of list items for a collapsed selection inside a list item.
else if ( ( selectedBlockObject || document.selection.isCollapsed ) && isListItemBlock( blocks[ 0 ] ) ) {
const changedBlocks = getListItems( selectedBlockObject || blocks[ 0 ] );

Expand All @@ -115,6 +125,16 @@ export default class DocumentListCommand extends Command {
const changedBlocks = [];

for ( const block of blocks ) {
// Rename block to a required element name if type of the list requires it.
if (
this._requiredElementName &&
!block.is( 'element', this._requiredElementName ) &&
checkCanBeRenamed( block, model.schema, this._requiredElementName )
) {
writer.rename( block, this._requiredElementName );
changedBlocks.push( block );
}

// Promote the given block to the list item.
if ( !block.hasAttribute( 'listType' ) ) {
writer.setAttributes( {
Expand Down
25 changes: 23 additions & 2 deletions packages/ckeditor5-list/src/documentlist/documentlistediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const LIST_BASE_ATTRIBUTES = [ 'listType', 'listIndent', 'listItemId' ];
* Map of model attributes applicable to list blocks.
*/
export interface ListItemAttributesMap {
listType?: 'numbered' | 'bulleted';
listType?: 'numbered' | 'bulleted' | string;
listIndent?: number;
listItemId?: string;
}
Expand Down Expand Up @@ -391,7 +391,28 @@ export default class DocumentListEditing extends Plugin {
const attributeNames = this._getListAttributeNames();

editor.conversion.for( 'upcast' )
.elementToElement( { view: 'li', model: 'paragraph' } )
// Convert <li> to a generic paragraph so the content of <li> is always inside a block.
// Setting the listType attribute to let other features (to-do list) know that this is part of a list item.
.elementToElement( {
view: 'li',
model: ( viewElement, { writer } ) => writer.createElement( 'paragraph', { listType: '' } )
} )
// Convert paragraph to the list block (without list type defined yet).
// This is important to properly handle bogus paragraph and to-do lists.
// Most of the time the bogus paragraph should not appear in the data of to-do list,
// but if there is any marker or an attribute on the paragraph then the bogus paragraph
// is preserved in the data, and we need to be able to detect this case.
.elementToElement( {
view: 'p',
model: ( viewElement, { writer } ) => {
if ( viewElement.parent && viewElement.parent.is( 'element', 'li' ) ) {
return writer.createElement( 'paragraph', { listType: '' } );
}

return null;
},
converterPriority: 'high'
} )
.add( dispatcher => {
dispatcher.on<UpcastElementEvent>( 'element:li', listItemUpcastConverter() );
dispatcher.on<UpcastElementEvent>( 'element:ul', listUpcastCleanList(), { priority: 'high' } );
Expand Down
15 changes: 13 additions & 2 deletions packages/ckeditor5-list/src/documentlist/utils/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import type {
Model,
Node,
Writer,
Item
Item,
Schema
} from 'ckeditor5/src/engine';

import { uid, toArray, type ArrayOrItem } from 'ckeditor5/src/utils';
Expand Down Expand Up @@ -45,7 +46,7 @@ export class ListItemUid {
export interface ListElement extends Element {
getAttribute( key: 'listItemId' ): string;
getAttribute( key: 'listIndent' ): number;
getAttribute( key: 'listType' ): 'numbered' | 'bulleted';
getAttribute( key: 'listType' ): 'numbered' | 'bulleted' | string;
getAttribute( key: string ): unknown;
}

Expand Down Expand Up @@ -545,6 +546,16 @@ export function getSelectedBlockObject( model: Model ): Element | null {
return null;
}

/**
* Checks whether the given block can be replaced by a paragraph.
*
* @param block A block to be tested.
* @param schema The schema of the document.
*/
export function checkCanBeRenamed( block: Element, schema: Schema, elementName: string ): boolean {
return schema.checkChild( block.parent as Element, elementName ) && !schema.isObject( block );
}

// Merges a given block to the given parent block if parent is a list item and there is no more blocks in the same item.
function mergeListItemIfNotLast(
block: ListElement,
Expand Down
11 changes: 11 additions & 0 deletions packages/ckeditor5-list/src/documentlist/utils/postfixers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,17 @@ export function fixListItemIds(

seenIds.add( listItemId );

// Make sure that all items in a to-do list have unique IDs.
if ( listType == 'todo' ) {
if ( node.getAttribute( 'listItemId' ) != listItemId ) {
writer.setAttribute( 'listItemId', listItemId, node );

applied = true;
}

continue;
}

for ( const block of getListItemBlocks( node, { direction: 'forward' } ) ) {
visited.add( block );

Expand Down
Loading