Merge pull request #155 from ckeditor/t/99

Feature: Introduce images in tables. Closes #99.
ckeditor · Dec 20, 2018 · 39c09e6 · 39c09e6
2 parents 3ac19ea + 57e9554
commit 39c09e6
Show file tree

Hide file tree

Showing 10 changed files with 113 additions and 22 deletions.
diff --git a/src/tableediting.js b/src/tableediting.js
@@ -81,13 +81,13 @@ export default class TableEditing extends Plugin {
 			}
 		} );
 
-		// Disallow image and media in table cell.
+		// Disallow media in table cell.
 		schema.addChildCheck( ( context, childDefinition ) => {
 			if ( !Array.from( context.getNames() ).includes( 'table' ) ) {
 				return;
 			}
 
-			if ( childDefinition.name == 'image' || childDefinition.name == 'media' ) {
+			if ( childDefinition.name == 'media' ) {
 				return false;
 			}
 		} );

diff --git a/src/utils.js b/src/utils.js
@@ -57,7 +57,10 @@ export function isTableWidgetSelected( selection ) {
  * @returns {Boolean}
  */
 export function isTableContentSelected( selection ) {
+	const selectedElement = selection.getSelectedElement();
+	const isInnerWidgetSelected = selectedElement && isWidget( selectedElement );
+
 	const parentTable = findAncestor( 'table', selection.getFirstPosition() );
 
-	return !!( parentTable && isTableWidget( parentTable.parent ) );
+	return !isInnerWidgetSelected && !!( parentTable && isTableWidget( parentTable.parent ) );
 }
diff --git a/tests/_utils/utils.js b/tests/_utils/utils.js
@@ -187,13 +187,6 @@ export function defaultSchema( schema, registerParagraph = true ) {
 		}
 	} );
 
-	// Disallow image in table.
-	schema.addChildCheck( ( context, childDefinition ) => {
-		if ( childDefinition.name == 'image' && Array.from( context.getNames() ).includes( 'table' ) ) {
-			return false;
-		}
-	} );
-
 	if ( registerParagraph ) {
 		schema.register( 'paragraph', { inheritAllFrom: '$block' } );
 	}

diff --git a/tests/converters/upcasttable.js b/tests/converters/upcasttable.js
@@ -457,7 +457,7 @@ describe( 'upcastTable()', () => {
 			] ) );
 		} );
 
-		it( 'should upcast table with <img> in table cell to empty table cell', () => {
+		it( 'should upcast table with <img> in table cell', () => {
 			editor.setData(
 				'<table>' +
 					'<tbody>' +
@@ -469,7 +469,7 @@ describe( 'upcastTable()', () => {
 			);
 
 			expectModel( modelTable( [
-				[ '' ]
+				[ '<image src="sample.png"></image>' ]
 			] ) );
 		} );
 	} );

diff --git a/tests/manual/table.js b/tests/manual/table.js
@@ -7,13 +7,10 @@
 
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
 import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
-import Table from '../../src/table';
-import TableToolbar from '../../src/tabletoolbar';
-import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote';
 
 ClassicEditor
 	.create( document.querySelector( '#editor' ), {
-		plugins: [ ArticlePluginSet, Table, TableToolbar, BlockQuote ],
+		plugins: [ ArticlePluginSet ],
 		toolbar: [
 			'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo'
 		],

diff --git a/tests/manual/tableblockcontent.html b/tests/manual/tableblockcontent.html
@@ -64,6 +64,18 @@ <h2 style="text-align: justify">An h2 with justify alignment.</h2>
 			</td>
 			<td><blockquote><p style="text-align: right">A quote with paragraph with right alignment</p></blockquote></td>
 		</tr>
+		<tr>
+			<td>image</td>
+			<td>
+				<img alt="sample image" src="sample.jpg">
+			</td>
+			<td>
+				<p>An image aligned to right:</p>
+				<figure class="image image-style-side" contenteditable="false">
+					<img src="sample.jpg" alt="sample image">
+				</figure>
+			</td>
+		</tr>
 		</tbody>
 	</table>
 </div>
diff --git a/tests/manual/tableblockcontent.js b/tests/manual/tableblockcontent.js
@@ -7,17 +7,18 @@
 
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
 import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
-import Table from '../../src/table';
-import TableToolbar from '../../src/tabletoolbar';
 import Alignment from '@ckeditor/ckeditor5-alignment/src/alignment';
 
 ClassicEditor
 	.create( document.querySelector( '#editor' ), {
-		plugins: [ ArticlePluginSet, Table, TableToolbar, Alignment ],
+		plugins: [ ArticlePluginSet, Alignment ],
 		toolbar: [
 			'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote',
 			'alignment', '|', 'undo', 'redo'
 		],
+		image: {
+			toolbar: [ 'imageStyle:full', 'imageStyle:side' ]
+		},
 		table: {
 			contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells' ]
 		}

diff --git a/tests/manual/tableblockcontent.md b/tests/manual/tableblockcontent.md
@@ -6,6 +6,7 @@
     * List
     * Heading
     * Block Quote (with inner paragraph)
+    * Image
 
 2. The third column consist blocks with text alignment.
     * Paragraph - should be rendered was `<p>` when alignment is set (apart from default) for single paragraph.

diff --git a/tests/tableediting.js b/tests/tableediting.js
@@ -74,7 +74,7 @@ describe( 'TableEditing', () => {
 		expect( model.schema.checkChild( [ '$root', 'table', 'tableRow', 'tableCell' ], '$text' ) ).to.be.false;
 		expect( model.schema.checkChild( [ '$root', 'table', 'tableRow', 'tableCell' ], '$block' ) ).to.be.true;
 		expect( model.schema.checkChild( [ '$root', 'table', 'tableRow', 'tableCell' ], 'table' ) ).to.be.false;
-		expect( model.schema.checkChild( [ '$root', 'table', 'tableRow', 'tableCell' ], 'image' ) ).to.be.false;
+		expect( model.schema.checkChild( [ '$root', 'table', 'tableRow', 'tableCell' ], 'image' ) ).to.be.true;
 	} );
 
 	it( 'adds insertTable command', () => {
@@ -183,7 +183,7 @@ describe( 'TableEditing', () => {
 				editor.setData( '<table><tbody><tr><td><img src="sample.png"></td></tr></tbody></table>' );
 
 				expect( getModelData( model, { withoutSelection: true } ) )
-					.to.equal( '<table><tableRow><tableCell><paragraph></paragraph></tableCell></tableRow></table>' );
+					.to.equal( '<table><tableRow><tableCell><image src="sample.png"></image></tableCell></tableRow></table>' );
 			} );
 
 			it( 'should convert table with media', () => {
@@ -312,6 +312,40 @@ describe( 'TableEditing', () => {
 				] ) );
 			} );
 
+			it( 'should move to next cell with an image', () => {
+				setModelData( model, modelTable( [
+					[ '11[]', '<paragraph>foo</paragraph><image></image>' ]
+				] ) );
+
+				editor.editing.view.document.fire( 'keydown', domEvtDataStub );
+
+				sinon.assert.calledOnce( domEvtDataStub.preventDefault );
+				sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+				expect( formatTable( getModelData( model ) ) ).to.equal( formattedModelTable( [
+					[ '11', '<paragraph>[foo</paragraph><image></image>]' ]
+				] ) );
+			} );
+
+			it( 'should move to next cell with an blockQuote', () => {
+				model.schema.register( 'blockQuote', {
+					allowWhere: '$block',
+					allowContentOf: '$root'
+				} );
+				editor.conversion.elementToElement( { model: 'blockQuote', view: 'blockquote' } );
+
+				setModelData( model, modelTable( [
+					[ '11[]', '<blockQuote><paragraph>foo</paragraph></blockQuote>' ]
+				] ) );
+
+				editor.editing.view.document.fire( 'keydown', domEvtDataStub );
+
+				sinon.assert.calledOnce( domEvtDataStub.preventDefault );
+				sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+				expect( formatTable( getModelData( model ) ) ).to.equal( formattedModelTable( [
+					[ '11', '<blockQuote><paragraph>[foo]</paragraph></blockQuote>' ]
+				] ) );
+			} );
+
 			it( 'should listen with lower priority then its children', () => {
 				// Cancel TAB event.
 				editor.keystrokes.set( 'Tab', ( data, cancel ) => cancel() );
@@ -461,6 +495,20 @@ describe( 'TableEditing', () => {
 					],
 				] ) );
 			} );
+
+			it( 'should move to previous cell with an image', () => {
+				setModelData( model, modelTable( [
+					[ '<paragraph>foo</paragraph><image></image>', 'bar[]' ]
+				] ) );
+
+				editor.editing.view.document.fire( 'keydown', domEvtDataStub );
+
+				sinon.assert.calledOnce( domEvtDataStub.preventDefault );
+				sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+				expect( formatTable( getModelData( model ) ) ).to.equal( formattedModelTable( [
+					[ '<paragraph>[foo</paragraph><image></image>]', 'bar' ]
+				] ) );
+			} );
 		} );
 	} );
 

diff --git a/tests/tabletoolbar.js b/tests/tabletoolbar.js
@@ -16,6 +16,9 @@ import View from '@ckeditor/ckeditor5-ui/src/view';
 import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import WidgetToolbarRepository from '@ckeditor/ckeditor5-widget/src/widgettoolbarrepository';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+import ImageToolbar from '@ckeditor/ckeditor5-image/src/imagetoolbar';
+import Image from '@ckeditor/ckeditor5-image/src/image';
+import ImageStyle from '@ckeditor/ckeditor5-image/src/imagestyle';
 
 describe( 'TableToolbar', () => {
 	testUtils.createSinonSandbox();
@@ -29,7 +32,10 @@ describe( 'TableToolbar', () => {
 
 			return ClassicTestEditor
 				.create( editorElement, {
-					plugins: [ Paragraph, Table, TableToolbar, FakeButton ],
+					plugins: [ Paragraph, Image, ImageStyle, ImageToolbar, Table, TableToolbar, FakeButton ],
+					image: {
+						toolbar: [ 'imageStyle:full', 'imageStyle:side' ]
+					},
 					table: {
 						contentToolbar: [ 'fake_button' ]
 					}
@@ -154,6 +160,36 @@ describe( 'TableToolbar', () => {
 				expect( balloon.visibleView ).to.equal( toolbar );
 			} );
 
+			it( 'should not show the toolbar on ui#update when the image inside table is selected', () => {
+				setData(
+					model,
+					'<paragraph>[foo]</paragraph>' +
+					'<table><tableRow><tableCell><paragraph>foo</paragraph><image src=""></image></tableCell></tableRow></table>'
+				);
+
+				expect( balloon.visibleView ).to.be.null;
+
+				const imageToolbar = widgetToolbarRepository._toolbars.get( 'image' ).view;
+
+				model.change( writer => {
+					// Select the <tableCell><paragraph></paragraph>[<image></image>]</tableCell>
+					const nodeByPath = doc.getRoot().getNodeByPath( [ 1, 0, 0, 1 ] );
+
+					writer.setSelection( nodeByPath, 'on' );
+				} );
+
+				expect( balloon.visibleView ).to.equal( imageToolbar );
+
+				model.change( writer => {
+					// Select the <tableCell><paragraph>[]</paragraph><image></image></tableCell>
+					writer.setSelection(
+						writer.createPositionAt( doc.getRoot().getNodeByPath( [ 1, 0, 0, 0 ] ), 0 )
+					);
+				} );
+
+				expect( balloon.visibleView ).to.equal( toolbar );
+			} );
+
 			it( 'should not engage when the toolbar is in the balloon yet invisible', () => {
 				setData( model, '<table><tableRow><tableCell><paragraph>x[y]z</paragraph></tableCell></tableRow></table>' );