8
0
Просмотр исходного кода

Select-all only in limit elements which accept text and paragraphs,

as suggested in reviews
https://github.com/ckeditor/ckeditor5/pull/6816#discussion_r425709607 and
https://github.com/ckeditor/ckeditor5/pull/6816#pullrequestreview-412449415 .
Closes #6621.
Tomek Wytrębowicz 5 лет назад
Родитель
Сommit
bd301c4091

+ 22 - 9
packages/ckeditor5-select-all/src/selectallcommand.js

@@ -32,19 +32,32 @@ export default class SelectAllCommand extends Command {
 	execute() {
 		const model = this.editor.model;
 		const selection = model.document.selection;
-		let limitElement = model.schema.getLimitElement( selection );
-		let place = 'in';
-		// If entire element was already selected, try selecting all in a parent limit element (if any).
-		if ( selection.containsEntireContent( limitElement ) && limitElement.root !== limitElement ) {
+		let scopeElement = model.schema.getLimitElement( selection );
+
+		// If an entire scope is selected, or the selection's ancestor is not a scope yet,
+		// browse through ancestors to find the enclosing parent scope.
+		if ( selection.containsEntireContent( scopeElement ) || !isSelectAllScope( model.schema, scopeElement ) ) {
 			do {
-				// Eventually, the $root (limitElement.root === limitElement) will be a limit.
-				limitElement = limitElement.parent;
-			} while ( !model.schema.isLimit( limitElement ) );
-			place = 'on';
+				scopeElement = scopeElement.parent;
+				// Do nothing, if the entire `root` is already selected.
+				if ( !scopeElement ) {
+					return;
+				}
+			} while ( !isSelectAllScope( model.schema, scopeElement ) );
 		}
 
 		model.change( writer => {
-			writer.setSelection( limitElement, place );
+			writer.setSelection( scopeElement, 'in' );
 		} );
 	}
 }
+
+// Checks whether the element is a valid select-all scope.
+// Returns true, it the element {@link module:engine/model/schema~Schema#isLimit `isLimit()`},
+// and can contain any text or paragraph.
+// @param {module:engine/model/schema~Schema} schema The schema to cheng against.
+// @param {module:engine/model/element~Element} element
+// @return {Boolean}
+function isSelectAllScope( schema, element ) {
+	return schema.isLimit( element ) && ( schema.checkChild( element, '$text' ) || schema.checkChild( element, 'paragraph' ) );
+}

+ 65 - 11
packages/ckeditor5-select-all/tests/selectallcommand.js

@@ -77,12 +77,50 @@ describe( 'SelectAllCommand', () => {
 			expect( getData( model ) ).to.equal( '<paragraph>foo</paragraph><image src="foo.png"><caption>[bar]</caption></image>' );
 		} );
 
-		it( 'when entire editable is selected, should select all in parent limit element', () => {
+		it( 'should select all (within limit element selected)', () => {
+			setData( model,
+				'<paragraph>foo</paragraph>' +
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>foo</paragraph>' +
+						'</tableCell>' +
+						'[<tableCell>' +
+							'<paragraph>bar</paragraph>' +
+						'</tableCell>]' +
+						'[<tableCell>' +
+							'<paragraph>baz</paragraph>' +
+						'</tableCell>]' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			editor.execute( 'selectAll' );
+
+			expect( getData( model ) ).to.equal(
+				'<paragraph>[foo</paragraph>' +
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>foo</paragraph>' +
+						'</tableCell>' +
+						'<tableCell>' +
+							'<paragraph>bar</paragraph>' +
+						'</tableCell>' +
+						'<tableCell>' +
+							'<paragraph>baz</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+				'</table>]'
+			);
+		} );
+
+		it( 'when entire editable is selected, should select all in parent select-all-limit element', () => {
 			setData( model, '<paragraph>foo</paragraph><image src="foo.png"><caption>[bar]</caption></image>' );
 
 			editor.execute( 'selectAll' );
 
-			expect( getData( model ) ).to.equal( '<paragraph>foo</paragraph>[<image src="foo.png"><caption>bar</caption></image>]' );
+			expect( getData( model ) ).to.equal( '<paragraph>[foo</paragraph><image src="foo.png"><caption>bar</caption></image>]' );
 		} );
 
 		it( 'should select all in the closest nested editable (nested editable inside another nested editable)', () => {
@@ -112,7 +150,7 @@ describe( 'SelectAllCommand', () => {
 			);
 		} );
 
-		it( 'consecutive execute() on nested editable, should select all in the parent limit element', () => {
+		it( 'consecutive execute() on nested editable, should select all in the parent sellect-all-limit element', () => {
 			setData( model,
 				'<paragraph>foo</paragraph>' +
 				'<table>' +
@@ -132,8 +170,8 @@ describe( 'SelectAllCommand', () => {
 				'<table>' +
 					'<tableRow>' +
 						'<tableCell>' +
-							'<paragraph>foo</paragraph>' +
-							'[<image src="foo.png"><caption>bar</caption></image>]' +
+							'<paragraph>[foo</paragraph>' +
+							'<image src="foo.png"><caption>bar</caption></image>]' +
 						'</tableCell>' +
 					'</tableRow>' +
 				'</table>'
@@ -141,21 +179,37 @@ describe( 'SelectAllCommand', () => {
 
 			editor.execute( 'selectAll' );
 
-			expect( getData( model ) ).to.equal( '<paragraph>foo</paragraph>' +
+			expect( getData( model ) ).to.equal( '<paragraph>[foo</paragraph>' +
 				'<table>' +
 					'<tableRow>' +
 						'<tableCell>' +
-							'<paragraph>[foo</paragraph>' +
-							'<image src="foo.png"><caption>bar</caption></image>]' +
+							'<paragraph>foo</paragraph>' +
+							'<image src="foo.png"><caption>bar</caption></image>' +
 						'</tableCell>' +
 					'</tableRow>' +
-				'</table>'
+				'</table>]'
+			);
+
+			// editor.execute( 'selectAll' );
+		} );
+
+		it( 'when entire editor is selected, should not change the selection', () => {
+			setData( model,
+				'<paragraph>[foo</paragraph>' +
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>foo</paragraph>' +
+							'<image src="foo.png"><caption>bar</caption></image>' +
+						'</tableCell>' +
+					'</tableRow>' +
+				'</table>]'
 			);
 
 			editor.execute( 'selectAll' );
 
-			expect( getData( model ) ).to.equal( '<paragraph>foo</paragraph>' +
-				'[<table>' +
+			expect( getData( model ) ).to.equal( '<paragraph>[foo</paragraph>' +
+				'<table>' +
 					'<tableRow>' +
 						'<tableCell>' +
 							'<paragraph>foo</paragraph>' +