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

Commit a78efec

Browse files
ReinmarReinmar
authoredNov 19, 2018
Merge pull request #58 from ckeditor/t/57
Fix: Selection converter will mark only the topmost widget in case of selecting a widget with another widget nested inside it. Closes #57.
·
v19.0.0v10.3.1
2 parents 49afa6e + cf06c9f commit a78efec

File tree

7 files changed

+275
-6
lines changed

7 files changed

+275
-6
lines changed
 

‎src/utils.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,17 @@ export const WIDGET_CLASS_NAME = 'ck-widget';
3131
export const WIDGET_SELECTED_CLASS_NAME = 'ck-widget_selected';
3232

3333
/**
34-
* Returns `true` if given {@link module:engine/view/element~Element} is a widget.
34+
* Returns `true` if given {@link module:engine/view/node~Node} is an {@link module:engine/view/element~Element} and a widget.
3535
*
36-
* @param {module:engine/view/element~Element} element
36+
* @param {module:engine/view/node~Node} node
3737
* @returns {Boolean}
3838
*/
39-
export function isWidget( element ) {
40-
return !!element.getCustomProperty( widgetSymbol );
39+
export function isWidget( node ) {
40+
if ( !node.is( 'element' ) ) {
41+
return false;
42+
}
43+
44+
return !!node.getCustomProperty( widgetSymbol );
4145
}
4246

4347
/**
@@ -86,7 +90,7 @@ export function isWidget( element ) {
8690
* @returns {module:engine/view/element~Element} Returns the same element.
8791
*/
8892
export function toWidget( element, writer, options = {} ) {
89-
// The selection on Edge behaves better when the whole editor contents is in a single contentedible element.
93+
// The selection on Edge behaves better when the whole editor contents is in a single contenteditable element.
9094
// https://github.com/ckeditor/ckeditor5/issues/1079
9195
if ( !env.isEdge ) {
9296
writer.setAttribute( 'contenteditable', 'false', element );

‎src/widget.js

+18-1
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,18 @@ export default class Widget extends Plugin {
6363
const viewWriter = conversionApi.writer;
6464
const viewSelection = viewWriter.document.selection;
6565
const selectedElement = viewSelection.getSelectedElement();
66+
let lastMarked = null;
6667

6768
for ( const range of viewSelection.getRanges() ) {
6869
for ( const value of range ) {
6970
const node = value.item;
7071

71-
if ( node.is( 'element' ) && isWidget( node ) ) {
72+
// Do not mark nested widgets in selected one. See: #57.
73+
if ( isWidget( node ) && !isChild( node, lastMarked ) ) {
7274
viewWriter.addClass( WIDGET_SELECTED_CLASS_NAME, node );
75+
7376
this._previouslySelected.add( node );
77+
lastMarked = node;
7478

7579
// Check if widget is a single element selected.
7680
if ( node == selectedElement ) {
@@ -420,3 +424,16 @@ function isInsideNestedEditable( element ) {
420424

421425
return false;
422426
}
427+
428+
// Checks whether the specified `element` is a child of the `parent` element.
429+
//
430+
// @param {module:engine/view/element~Element} element An element to check.
431+
// @param {module:engine/view/element~Element|null} parent A parent for the element.
432+
// @returns {Boolean}
433+
function isChild( element, parent ) {
434+
if ( !parent ) {
435+
return false;
436+
}
437+
438+
return Array.from( element.getAncestors() ).includes( parent );
439+
}

‎tests/manual/nested-widgets.html

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<head>
2+
<style>
3+
body {
4+
max-width: 800px;
5+
margin: 20px auto;
6+
}
7+
8+
.ck-content .widget {
9+
background: rgba( 0, 0, 0, 0.1 );
10+
padding: 20px !important;
11+
min-height: 50px;
12+
}
13+
</style>
14+
</head>
15+
<div id="editor">
16+
<h2>Heading 1</h2>
17+
<p>Paragraph</p>
18+
<div class="widget">
19+
<div class="widget"></div>
20+
<div class="widget"></div>
21+
</div>
22+
<p>Paragraph</p>
23+
</div>

‎tests/manual/nested-widgets.js

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
3+
* For licensing, see LICENSE.md.
4+
*/
5+
6+
/* globals console, window, document, setInterval */
7+
8+
import Widget from '../../src/widget';
9+
import { toWidget } from '../../src/utils';
10+
import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
11+
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
12+
import { downcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters';
13+
import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
14+
import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
15+
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
16+
17+
function MyPlugin( editor ) {
18+
editor.model.schema.register( 'div', {
19+
allowIn: [ '$root', 'div' ],
20+
isObject: true
21+
} );
22+
23+
editor.model.schema.extend( '$text', {
24+
allowIn: 'div'
25+
} );
26+
27+
editor.conversion.for( 'downcast' ).add( downcastElementToElement( {
28+
model: 'div',
29+
view: ( modelElement, writer ) => {
30+
return toWidget( writer.createContainerElement( 'div', { class: 'widget' } ), writer );
31+
}
32+
} ) );
33+
34+
editor.conversion.for( 'upcast' ).add( upcastElementToElement( {
35+
model: 'div',
36+
view: 'div'
37+
} ) );
38+
}
39+
40+
ClassicEditor
41+
.create( document.querySelector( '#editor' ), {
42+
plugins: [ ArticlePluginSet, Widget, MyPlugin ],
43+
toolbar: [
44+
'heading',
45+
'|',
46+
'bold',
47+
'italic',
48+
'link',
49+
'bulletedList',
50+
'numberedList',
51+
'blockQuote',
52+
'insertTable',
53+
'mediaEmbed',
54+
'undo',
55+
'redo'
56+
],
57+
image: {
58+
toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]
59+
},
60+
table: {
61+
contentToolbar: [
62+
'tableColumn',
63+
'tableRow',
64+
'mergeTableCells'
65+
]
66+
}
67+
} )
68+
.then( editor => {
69+
window.editor = editor;
70+
71+
setInterval( () => {
72+
console.log( getModelData( editor.model ) );
73+
console.log( getViewData( editor.editing.view ) );
74+
}, 3000 );
75+
} )
76+
.catch( err => {
77+
console.error( err.stack );
78+
} );

‎tests/manual/nested-widgets.md

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Nested widgets
2+
3+
* When selecting the most top-outer widget, nested widgets should not be selected.

‎tests/utils.js

+5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
/* global document */
77

88
import DowncastWriter from '@ckeditor/ckeditor5-engine/src/view/downcastwriter';
9+
import Text from '@ckeditor/ckeditor5-engine/src/view/text';
910
import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
1011
import ViewEditableElement from '@ckeditor/ckeditor5-engine/src/view/editableelement';
1112
import ViewDocument from '@ckeditor/ckeditor5-engine/src/view/document';
@@ -143,6 +144,10 @@ describe( 'widget utils', () => {
143144
it( 'should return false for non-widgetized elements', () => {
144145
expect( isWidget( new ViewElement( 'p' ) ) ).to.be.false;
145146
} );
147+
148+
it( 'should return false for text node', () => {
149+
expect( isWidget( new Text( 'p' ) ) ).to.be.false;
150+
} );
146151
} );
147152

148153
describe( 'label utils', () => {

‎tests/widget.js

+139
Original file line numberDiff line numberDiff line change
@@ -1177,11 +1177,19 @@ describe( 'Widget', () => {
11771177

11781178
model.schema.register( 'widget', {
11791179
inheritAllFrom: '$block',
1180+
allowIn: 'widget',
11801181
isObject: true
11811182
} );
11821183
model.schema.register( 'paragraph', {
11831184
inheritAllFrom: '$block'
11841185
} );
1186+
model.schema.register( 'nested', {
1187+
allowIn: 'widget',
1188+
isLimit: true
1189+
} );
1190+
model.schema.extend( '$text', {
1191+
allowIn: 'nested'
1192+
} );
11851193

11861194
editor.conversion.for( 'downcast' )
11871195
.add( downcastElementToElement( { model: 'paragraph', view: 'p' } ) )
@@ -1192,6 +1200,10 @@ describe( 'Widget', () => {
11921200

11931201
return toWidget( widget, viewWriter, { hasSelectionHandler: true } );
11941202
}
1203+
} ) )
1204+
.add( downcastElementToElement( {
1205+
model: 'nested',
1206+
view: ( modelItem, viewWriter ) => viewWriter.createEditableElement( 'figcaption', { contenteditable: true } )
11951207
} ) );
11961208
} );
11971209
} );
@@ -1210,5 +1222,132 @@ describe( 'Widget', () => {
12101222

12111223
expect( getModelData( model ) ).to.equal( '<paragraph>bar</paragraph>[<widget></widget>]<paragraph>foo</paragraph>' );
12121224
} );
1225+
1226+
it( 'should select the most top-outer widget if widgets are nested', () => {
1227+
setModelData( model, '<widget><widget></widget><widget></widget></widget>' );
1228+
1229+
// The top-outer widget.
1230+
const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 0 );
1231+
1232+
const domEventDataMock = {
1233+
target: viewWidgetSelectionHandler,
1234+
preventDefault: sinon.spy()
1235+
};
1236+
1237+
viewDocument.fire( 'mousedown', domEventDataMock );
1238+
1239+
expect( getViewData( view ) ).to.equal(
1240+
'[<div class="ck-widget ck-widget_selectable ck-widget_selected" contenteditable="false">' +
1241+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1242+
'<div class="ck ck-widget__selection-handler"></div>' +
1243+
'</div>' +
1244+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1245+
'<div class="ck ck-widget__selection-handler"></div>' +
1246+
'</div>' +
1247+
'<div class="ck ck-widget__selection-handler"></div>' +
1248+
'</div>]'
1249+
);
1250+
} );
1251+
1252+
it( 'should select a proper widget if they are nested and multiplied', () => {
1253+
setModelData( model,
1254+
'<widget></widget>' +
1255+
'<widget>' +
1256+
'<widget></widget>' +
1257+
'<widget></widget>' +
1258+
'</widget>' +
1259+
'<widget></widget>'
1260+
);
1261+
1262+
const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 1 );
1263+
1264+
const domEventDataMock = {
1265+
target: viewWidgetSelectionHandler,
1266+
preventDefault: sinon.spy()
1267+
};
1268+
1269+
viewDocument.fire( 'mousedown', domEventDataMock );
1270+
1271+
expect( getViewData( view ) ).to.equal(
1272+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1273+
'<div class="ck ck-widget__selection-handler"></div>' +
1274+
'</div>' +
1275+
'[<div class="ck-widget ck-widget_selectable ck-widget_selected" contenteditable="false">' +
1276+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1277+
'<div class="ck ck-widget__selection-handler"></div>' +
1278+
'</div>' +
1279+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1280+
'<div class="ck ck-widget__selection-handler"></div>' +
1281+
'</div>' +
1282+
'<div class="ck ck-widget__selection-handler"></div>' +
1283+
'</div>]' +
1284+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1285+
'<div class="ck ck-widget__selection-handler"></div>' +
1286+
'</div>'
1287+
);
1288+
} );
1289+
1290+
it( 'works fine with a widget that contains more children', () => {
1291+
setModelData( model,
1292+
'<widget>' +
1293+
'<nested>foo bar</nested>' +
1294+
'<widget></widget>' +
1295+
'</widget>'
1296+
);
1297+
1298+
const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 0 );
1299+
1300+
const domEventDataMock = {
1301+
target: viewWidgetSelectionHandler,
1302+
preventDefault: sinon.spy()
1303+
};
1304+
1305+
viewDocument.fire( 'mousedown', domEventDataMock );
1306+
1307+
expect( getViewData( view ) ).to.equal(
1308+
'[<div class="ck-widget ck-widget_selectable ck-widget_selected" contenteditable="false">' +
1309+
'<figcaption contenteditable="true">foo bar</figcaption>' +
1310+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1311+
'<div class="ck ck-widget__selection-handler"></div>' +
1312+
'</div>' +
1313+
'<div class="ck ck-widget__selection-handler"></div>' +
1314+
'</div>]'
1315+
);
1316+
} );
1317+
1318+
it( 'should select a proper widget for more complex structures', () => {
1319+
setModelData( model,
1320+
'<widget>' +
1321+
'<widget></widget>' +
1322+
'<widget>' +
1323+
'<widget></widget>' +
1324+
'</widget>' +
1325+
'</widget>'
1326+
);
1327+
1328+
const viewWidgetSelectionHandler = viewDocument.getRoot().getChild( 0 ).getChild( 1 );
1329+
1330+
const domEventDataMock = {
1331+
target: viewWidgetSelectionHandler,
1332+
preventDefault: sinon.spy()
1333+
};
1334+
1335+
viewDocument.fire( 'mousedown', domEventDataMock );
1336+
1337+
expect( getViewData( view ) ).to.equal(
1338+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1339+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1340+
'<div class="ck ck-widget__selection-handler"></div>' +
1341+
'</div>' +
1342+
'[<div class="ck-widget ck-widget_selectable ck-widget_selected" contenteditable="false">' +
1343+
'<div class="ck-widget ck-widget_selectable" contenteditable="false">' +
1344+
'<div class="ck ck-widget__selection-handler"></div>' +
1345+
'</div>' +
1346+
'<div class="ck ck-widget__selection-handler"></div>' +
1347+
'</div>]' +
1348+
'<div class="ck ck-widget__selection-handler"></div>' +
1349+
'</div>'
1350+
);
1351+
} );
12131352
} );
12141353
} );

0 commit comments

Comments
 (0)
This repository has been archived.