From cf06c9f5579a54c45b18f1327e28f9a5e71dc3f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 6 Nov 2018 21:12:05 +0100 Subject: [PATCH] Mark only topmost widget. --- src/utils.js | 14 ++-- src/widget.js | 19 ++++- tests/manual/nested-widgets.html | 23 +++++ tests/manual/nested-widgets.js | 78 +++++++++++++++++ tests/manual/nested-widgets.md | 3 + tests/utils.js | 5 ++ tests/widget.js | 139 +++++++++++++++++++++++++++++++ 7 files changed, 275 insertions(+), 6 deletions(-) create mode 100644 tests/manual/nested-widgets.html create mode 100644 tests/manual/nested-widgets.js create mode 100644 tests/manual/nested-widgets.md diff --git a/src/utils.js b/src/utils.js index 337076e7..7ff740a7 100644 --- a/src/utils.js +++ b/src/utils.js @@ -31,13 +31,17 @@ export const WIDGET_CLASS_NAME = 'ck-widget'; export const WIDGET_SELECTED_CLASS_NAME = 'ck-widget_selected'; /** - * Returns `true` if given {@link module:engine/view/element~Element} is a widget. + * Returns `true` if given {@link module:engine/view/node~Node} is an {@link module:engine/view/element~Element} and a widget. * - * @param {module:engine/view/element~Element} element + * @param {module:engine/view/node~Node} node * @returns {Boolean} */ -export function isWidget( element ) { - return !!element.getCustomProperty( widgetSymbol ); +export function isWidget( node ) { + if ( !node.is( 'element' ) ) { + return false; + } + + return !!node.getCustomProperty( widgetSymbol ); } /** @@ -86,7 +90,7 @@ export function isWidget( element ) { * @returns {module:engine/view/element~Element} Returns the same element. */ export function toWidget( element, writer, options = {} ) { - // The selection on Edge behaves better when the whole editor contents is in a single contentedible element. + // The selection on Edge behaves better when the whole editor contents is in a single contenteditable element. // https://github.com/ckeditor/ckeditor5/issues/1079 if ( !env.isEdge ) { writer.setAttribute( 'contenteditable', 'false', element ); diff --git a/src/widget.js b/src/widget.js index b16d93d5..c533ac30 100644 --- a/src/widget.js +++ b/src/widget.js @@ -63,14 +63,18 @@ export default class Widget extends Plugin { const viewWriter = conversionApi.writer; const viewSelection = viewWriter.document.selection; const selectedElement = viewSelection.getSelectedElement(); + let lastMarked = null; for ( const range of viewSelection.getRanges() ) { for ( const value of range ) { const node = value.item; - if ( node.is( 'element' ) && isWidget( node ) ) { + // Do not mark nested widgets in selected one. See: #57. + if ( isWidget( node ) && !isChild( node, lastMarked ) ) { viewWriter.addClass( WIDGET_SELECTED_CLASS_NAME, node ); + this._previouslySelected.add( node ); + lastMarked = node; // Check if widget is a single element selected. if ( node == selectedElement ) { @@ -420,3 +424,16 @@ function isInsideNestedEditable( element ) { return false; } + +// Checks whether the specified `element` is a child of the `parent` element. +// +// @param {module:engine/view/element~Element} element An element to check. +// @param {module:engine/view/element~Element|null} parent A parent for the element. +// @returns {Boolean} +function isChild( element, parent ) { + if ( !parent ) { + return false; + } + + return Array.from( element.getAncestors() ).includes( parent ); +} diff --git a/tests/manual/nested-widgets.html b/tests/manual/nested-widgets.html new file mode 100644 index 00000000..4fb1f618 --- /dev/null +++ b/tests/manual/nested-widgets.html @@ -0,0 +1,23 @@ + + + +
+

Heading 1

+

Paragraph

+
+
+
+
+

Paragraph

+
diff --git a/tests/manual/nested-widgets.js b/tests/manual/nested-widgets.js new file mode 100644 index 00000000..1436ae86 --- /dev/null +++ b/tests/manual/nested-widgets.js @@ -0,0 +1,78 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals console, window, document, setInterval */ + +import Widget from '../../src/widget'; +import { toWidget } from '../../src/utils'; +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; +import { downcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters'; +import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters'; +import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; + +function MyPlugin( editor ) { + editor.model.schema.register( 'div', { + allowIn: [ '$root', 'div' ], + isObject: true + } ); + + editor.model.schema.extend( '$text', { + allowIn: 'div' + } ); + + editor.conversion.for( 'downcast' ).add( downcastElementToElement( { + model: 'div', + view: ( modelElement, writer ) => { + return toWidget( writer.createContainerElement( 'div', { class: 'widget' } ), writer ); + } + } ) ); + + editor.conversion.for( 'upcast' ).add( upcastElementToElement( { + model: 'div', + view: 'div' + } ) ); +} + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ ArticlePluginSet, Widget, MyPlugin ], + toolbar: [ + 'heading', + '|', + 'bold', + 'italic', + 'link', + 'bulletedList', + 'numberedList', + 'blockQuote', + 'insertTable', + 'mediaEmbed', + 'undo', + 'redo' + ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ] + }, + table: { + contentToolbar: [ + 'tableColumn', + 'tableRow', + 'mergeTableCells' + ] + } + } ) + .then( editor => { + window.editor = editor; + + setInterval( () => { + console.log( getModelData( editor.model ) ); + console.log( getViewData( editor.editing.view ) ); + }, 3000 ); + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/nested-widgets.md b/tests/manual/nested-widgets.md new file mode 100644 index 00000000..10ec3d7e --- /dev/null +++ b/tests/manual/nested-widgets.md @@ -0,0 +1,3 @@ +# Nested widgets + +* When selecting the most top-outer widget, nested widgets should not be selected. diff --git a/tests/utils.js b/tests/utils.js index 9869c9f2..5b0fd269 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -6,6 +6,7 @@ /* global document */ import DowncastWriter from '@ckeditor/ckeditor5-engine/src/view/downcastwriter'; +import Text from '@ckeditor/ckeditor5-engine/src/view/text'; import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element'; import ViewEditableElement from '@ckeditor/ckeditor5-engine/src/view/editableelement'; import ViewDocument from '@ckeditor/ckeditor5-engine/src/view/document'; @@ -143,6 +144,10 @@ describe( 'widget utils', () => { it( 'should return false for non-widgetized elements', () => { expect( isWidget( new ViewElement( 'p' ) ) ).to.be.false; } ); + + it( 'should return false for text node', () => { + expect( isWidget( new Text( 'p' ) ) ).to.be.false; + } ); } ); describe( 'label utils', () => { diff --git a/tests/widget.js b/tests/widget.js index 3221ba4e..4dad24a0 100644 --- a/tests/widget.js +++ b/tests/widget.js @@ -1177,11 +1177,19 @@ describe( 'Widget', () => { model.schema.register( 'widget', { inheritAllFrom: '$block', + allowIn: 'widget', isObject: true } ); model.schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + model.schema.register( 'nested', { + allowIn: 'widget', + isLimit: true + } ); + model.schema.extend( '$text', { + allowIn: 'nested' + } ); editor.conversion.for( 'downcast' ) .add( downcastElementToElement( { model: 'paragraph', view: 'p' } ) ) @@ -1192,6 +1200,10 @@ describe( 'Widget', () => { return toWidget( widget, viewWriter, { hasSelectionHandler: true } ); } + } ) ) + .add( downcastElementToElement( { + model: 'nested', + view: ( modelItem, viewWriter ) => viewWriter.createEditableElement( 'figcaption', { contenteditable: true } ) } ) ); } ); } ); @@ -1210,5 +1222,132 @@ describe( 'Widget', () => { expect( getModelData( model ) ).to.equal( 'bar[]foo' ); } ); + + it( 'should select the most top-outer widget if widgets are nested', () => { + setModelData( model, '' ); + + // The top-outer widget. + const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 0 ); + + const domEventDataMock = { + target: viewWidgetSelectionHandler, + preventDefault: sinon.spy() + }; + + viewDocument.fire( 'mousedown', domEventDataMock ); + + expect( getViewData( view ) ).to.equal( + '[
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
]' + ); + } ); + + it( 'should select a proper widget if they are nested and multiplied', () => { + setModelData( model, + '' + + '' + + '' + + '' + + '' + + '' + ); + + const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 1 ); + + const domEventDataMock = { + target: viewWidgetSelectionHandler, + preventDefault: sinon.spy() + }; + + viewDocument.fire( 'mousedown', domEventDataMock ); + + expect( getViewData( view ) ).to.equal( + '
' + + '
' + + '
' + + '[
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
]' + + '
' + + '
' + + '
' + ); + } ); + + it( 'works fine with a widget that contains more children', () => { + setModelData( model, + '' + + 'foo bar' + + '' + + '' + ); + + const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 0 ); + + const domEventDataMock = { + target: viewWidgetSelectionHandler, + preventDefault: sinon.spy() + }; + + viewDocument.fire( 'mousedown', domEventDataMock ); + + expect( getViewData( view ) ).to.equal( + '[
' + + '
foo bar
' + + '
' + + '
' + + '
' + + '
' + + '
]' + ); + } ); + + it( 'should select a proper widget for more complex structures', () => { + setModelData( model, + '' + + '' + + '' + + '' + + '' + + '' + ); + + const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 0 ).getChild( 1 ); + + const domEventDataMock = { + target: viewWidgetSelectionHandler, + preventDefault: sinon.spy() + }; + + viewDocument.fire( 'mousedown', domEventDataMock ); + + expect( getViewData( view ) ).to.equal( + '
' + + '
' + + '
' + + '
' + + '[
' + + '
' + + '
' + + '
' + + '
' + + '
]' + + '
' + + '
' + ); + } ); } ); } );