Ck/10812 document list editing #11010
Conversation
packages/ckeditor5-list/src/documentlist/documentlistediting.js
Outdated
Show resolved
Hide resolved
packages/ckeditor5-list/src/documentlist/documentlistediting.js
Outdated
Show resolved
Hide resolved
| viewRange = writer.wrap( viewRange, listItemViewElement ); | ||
| viewRange = writer.wrap( viewRange, listViewElement ); |
There was a problem hiding this comment.
I think it's worth mentioning here (and/or createListElement) that list elements are AttributeElements because after each downcast, successive list elements will be joined automatically by DowncastWriter. The decision to use AttributeElement (instead of ContainerElement) was intentional to take advantage of this mechanism. It's not played by the book and we need to be clear about this.
| for ( const entry of changes ) { | ||
| if ( entry.type == 'insert' && entry.name != '$text' ) { | ||
| findAndAddListHeadToMap( entry.position, itemToListHead ); | ||
|
|
||
| // Insert of a non-list item. | ||
| if ( !entry.attributes.has( 'listItemId' ) ) { | ||
| findAndAddListHeadToMap( entry.position.getShiftedBy( entry.length ), itemToListHead ); | ||
| } else { | ||
| changedItems.add( entry.position.nodeAfter ); | ||
| } | ||
| } | ||
| // Removed list item. | ||
| else if ( entry.type == 'remove' && entry.attributes.has( 'listItemId' ) ) { | ||
| findAndAddListHeadToMap( entry.position, itemToListHead ); | ||
| } | ||
| // Changed list attribute. | ||
| else if ( entry.type == 'attribute' ) { | ||
| const item = entry.range.start.nodeAfter; | ||
|
|
||
| if ( entry.attributeKey.startsWith( 'list' ) ) { | ||
| findAndAddListHeadToMap( entry.range.start, itemToListHead ); | ||
|
|
||
| if ( entry.attributeNewValue === null ) { | ||
| findAndAddListHeadToMap( entry.range.start.getShiftedBy( 1 ), itemToListHead ); | ||
| refreshItemParagraphIfNeeded( item, [] ); | ||
| } else { | ||
| changedItems.add( item ); | ||
| } | ||
| } else if ( item.hasAttribute( 'listItemId' ) ) { | ||
| refreshItemParagraphIfNeeded( item ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Let's make a named helper out of it.
| function checkList( listHead ) { | ||
| const visited = new Set(); | ||
| const stack = []; | ||
|
|
||
| for ( | ||
| let prev = null, item = listHead; | ||
| item && item.hasAttribute( 'listItemId' ); | ||
| prev = item, item = item.nextSibling | ||
| ) { | ||
| if ( visited.has( item ) ) { | ||
| continue; | ||
| } | ||
|
|
||
| const itemIndent = item.getAttribute( 'listIndent' ); | ||
|
|
||
| if ( prev && itemIndent < prev.getAttribute( 'listIndent' ) ) { | ||
| stack.length = itemIndent + 1; | ||
| } | ||
|
|
||
| stack[ itemIndent ] = { | ||
| id: item.getAttribute( 'listItemId' ), | ||
| type: item.getAttribute( 'listType' ) | ||
| }; | ||
|
|
||
| const blocks = getListItemElements( item, 'forward' ); | ||
|
|
||
| for ( const block of blocks ) { | ||
| visited.add( block ); | ||
|
|
||
| refreshItemParagraphIfNeeded( block, blocks ); | ||
| refreshItemWrappingIfNeeded( block, stack ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function refreshItemParagraphIfNeeded( item, blocks ) { | ||
| if ( !item.is( 'element', 'paragraph' ) ) { | ||
| return; | ||
| } | ||
|
|
||
| const viewElement = editing.mapper.toViewElement( item ); | ||
|
|
||
| if ( !viewElement ) { | ||
| return; | ||
| } | ||
|
|
||
| const useBogus = shouldUseBogusParagraph( item, blocks ); | ||
|
|
||
| if ( useBogus && viewElement.is( 'element', 'p' ) ) { | ||
| itemsToRefresh.add( item ); | ||
| } else if ( !useBogus && viewElement.is( 'element', 'span' ) ) { | ||
| itemsToRefresh.add( item ); | ||
| } | ||
| } | ||
|
|
||
| function refreshItemWrappingIfNeeded( item, stack ) { | ||
| // Items directly affected by some "change" don't need a refresh, they will be converted by their own changes. | ||
| if ( changedItems.has( item ) ) { | ||
| return; | ||
| } | ||
|
|
||
| const viewElement = editing.mapper.toViewElement( item ); | ||
| let stackIdx = stack.length - 1; | ||
|
|
||
| for ( | ||
| let element = viewElement.parent; | ||
| !element.is( 'editableElement' ); | ||
| element = element.parent | ||
| ) { | ||
| if ( isListItemView( element ) ) { | ||
| if ( element.id != stack[ stackIdx ].id ) { | ||
| break; | ||
| } | ||
| } else if ( isListView( element ) ) { | ||
| const expectedElementName = getViewElementNameForListType( stack[ stackIdx ].type ); | ||
|
|
||
| if ( element.name != expectedElementName ) { | ||
| break; | ||
| } | ||
|
|
||
| stackIdx--; | ||
|
|
||
| // Don't need to iterate further if we already know that the item is wrapped appropriately. | ||
| if ( stackIdx < 0 ) { | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| itemsToRefresh.add( item ); | ||
| } |
There was a problem hiding this comment.
Do we need to create all these helpers every time change:data kicks in? I'd extract them somewhere else.
| '<!-- c3 -->' + | ||
| '<!-- c2 -->' + | ||
| '<img src="/assets/sample.png" alt="Example image">' + |
There was a problem hiding this comment.
This test didn't work on the feature branch, but we didn't notice this so far. So, it's unrelated to this PR.
…ckeditor5 into ck/10975-document-list-command
…ckeditor5 into ck/10975-document-list-command
Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
…ckeditor5 into ck/10975-document-list-command
|
Cannot remove a widget which is a list item - #11346 |
|
List items are not removed correctly with backspace - #11347 |
|
Error when removing formatting on text - #11348 |
|
Error after getting data with html comment in list item - #11350 |
|
Image is inserted incorrectly with drag&drop into paragraph in list - #11351 |
|
Removing widget breaks the document list - #11352 |
|
Cannot insert text before or after a widget inside a list item - #11353 |
|
Error after getting data with code block in list - #11354 |
|
List gets split when widget is pasted/dropped into the list item - #11355 |
|
List items misaligned and covered after aligning table to the left - #11361 |
|
I've created a single ticket for our UX feedback. We will add new comments there - #11371 |
|
Image with captions split list: #11374 |
|
Left-aligned images also cover part of the list and make list items misaligned: #11361 (comment) |
|
Blockquote does not work with document-list: #11383 |
|
Drag and dropping or pasting the table into the table cell with document list inside clears cell content: #11384 |
|
HTML snippet does not work with document-list: #11385 |
|
Media embed does not work with document-list: #11386 |
…s-on-indent Internal (list): Reset document list properties after indent. Closes #11357.
…ties Feature (list): Introducing the document list properties feature. Closes #11065. Internal (engine): Added option for the `DomConverter` to transparently render only the content of the element in the data pipeline.
|
Closing this PR as code moved to epic branch #11503 |
Suggested merge commit message (convention)
Feature (list): Introducing the document list editing feature. Closes #10812.
Other (engine): The
isAllowedInsideAttributeElementoption was removed, from now onAttrubuteElementsare allowed to wrap any view element.Fix (link): The link decorators should be converted on the block images only once (should not wrap block image with an additional link).
Other (engine): The
Schemais extended by the new generic type:$container. Container elements can contain$blockor$containerelements.Other (engine): The
DowncastHelpersare passing an additional parameter to the creator functions (thedatathat provides more context to the element creator callback.Other (engine): The
ConversionApiprovided by theUpcastDispatcheris extended with an additional methodkeepEmptyElementthat marks an element that was created during splitting some model element that should not get removed on conversion even if it's empty.Other (engine): The
Differchange entries forinsertandremovetypes are extended with a map of attributes that were set while inserting an element or that were on an element while it got removed.MINOR BREAKING CHANGE (engine): The
isAllowedInsideAttributeElementoption is removed soAttributeElements can wrap any view element (according to positions). Make sure that you are not wrapping anyContainerElementby an accident by not checking the target in the converter. Those would previously get wrapped by anAttributeElementthat immediately would be removed by theContainerElementwithin it so there would not be any visual effect.Feature (list): Introducing the document list properties feature. Closes #11065.
Internal (engine): Added option for the
DomConverterto transparently render only the content of the element in the data pipeline.Additional information