Merge pull request #15150 from ckeditor/ck/15040

Fix (table): Table properties should be enabled if a table is selected from the outside. Closes #15040. Closes #15041. Closes #10983.

packages/ckeditor5-table/src/tablecaption/toggletablecaptioncommand.ts

@@ -11,7 +11,8 @@ import { Command } from 'ckeditor5/src/core';
 import type { Writer } from 'ckeditor5/src/engine';
 import type TableCaptionEditing from './tablecaptionediting';
 
-import { getCaptionFromTableModelElement, getSelectionAffectedTable } from './utils';
+import { getCaptionFromTableModelElement } from './utils';
+import { getSelectionAffectedTable } from '../utils/common';
 
 /**
  * The toggle table caption command.

packages/ckeditor5-table/src/tablecaption/utils.ts

@@ -14,6 +14,8 @@ import type {
 	ViewElement
 } from 'ckeditor5/src/engine';
 
+import { getSelectionAffectedTable } from '../utils/common';
+
 /**
  * Checks if the provided model element is a `table`.
  *
@@ -75,17 +77,3 @@ export function matchTableCaptionViewElement( element: ViewElement ): { name: tr
 
 	return null;
 }
-
-/**
- * Depending on the position of the selection we either return the table under cursor or look for the table higher in the hierarchy.
- */
-export function getSelectionAffectedTable( selection: DocumentSelection ): Element {
-	const selectedElement = selection.getSelectedElement();
-
-	// Is the command triggered from the `tableToolbar`?
-	if ( selectedElement && selectedElement.is( 'element', 'table' ) ) {
-		return selectedElement;
-	}
-
-	return selection.getFirstPosition()!.findAncestor( 'table' )!;
-}

packages/ckeditor5-table/src/tableproperties/commands/tablepropertycommand.ts

@@ -9,6 +9,7 @@
 
 import type { Batch, Element } from 'ckeditor5/src/engine';
 import { Command, type Editor } from 'ckeditor5/src/core';
+import { getSelectionAffectedTable } from '../../utils/common';
 
 export interface TablePropertyCommandExecuteOptions {
 	batch?: Batch;
@@ -55,7 +56,7 @@ export default class TablePropertyCommand extends Command {
 		const editor = this.editor;
 		const selection = editor.model.document.selection;
 
-		const table = selection.getFirstPosition()!.findAncestor( 'table' )!;
+		const table = getSelectionAffectedTable( selection );
 
 		this.isEnabled = !!table;
 		this.value = this._getValue( table );
@@ -76,7 +77,7 @@ export default class TablePropertyCommand extends Command {
 
 		const { value, batch } = options;
 
-		const table = selection.getFirstPosition()!.findAncestor( 'table' )!;
+		const table = getSelectionAffectedTable( selection );
 		const valueToSet = this._getValueToSet( value );
 
 		model.enqueueChange( batch, writer => {

packages/ckeditor5-table/src/tableproperties/tablepropertiesui.ts

@@ -29,7 +29,7 @@ import {
 	lineWidthFieldValidator,
 	defaultColors
 } from '../utils/ui/table-properties';
-import { getTableWidgetAncestor } from '../utils/ui/widget';
+import { getSelectionAffectedTableWidget } from '../utils/ui/widget';
 import { getBalloonTablePositionData, repositionContextualBalloon } from '../utils/ui/contextualballoon';
 import { getNormalizedDefaultProperties, type NormalizedDefaultProperties } from '../utils/table-properties';
 import type { Batch } from 'ckeditor5/src/engine';
@@ -357,7 +357,7 @@ export default class TablePropertiesUI extends Plugin {
 		const editor = this.editor;
 		const viewDocument = editor.editing.view.document;
 
-		if ( !getTableWidgetAncestor( viewDocument.selection ) ) {
+		if ( !getSelectionAffectedTableWidget( viewDocument.selection ) ) {
 			this._hideView();
 		} else if ( this._isViewVisible ) {
 			repositionContextualBalloon( editor, 'table' );

packages/ckeditor5-table/src/utils/common.ts

@@ -13,7 +13,8 @@ import type {
 	Item,
 	Position,
 	Schema,
-	Writer
+	Writer,
+	DocumentSelection
 } from 'ckeditor5/src/engine';
 
 import { downcastAttributeToStyle, upcastStyleToAttribute } from './../converters/tableproperties';
@@ -87,3 +88,17 @@ export function enableProperty(
 	upcastStyleToAttribute( conversion, { viewElement: /^(td|th)$/, ...options } );
 	downcastAttributeToStyle( conversion, { modelElement: 'tableCell', ...options } );
 }
+
+/**
+ * Depending on the position of the selection we either return the table under cursor or look for the table higher in the hierarchy.
+ */
+export function getSelectionAffectedTable( selection: DocumentSelection ): Element {
+	const selectedElement = selection.getSelectedElement();
+
+	// Is the command triggered from the `tableToolbar`?
+	if ( selectedElement && selectedElement.is( 'element', 'table' ) ) {
+		return selectedElement;
+	}
+
+	return selection.getFirstPosition()!.findAncestor( 'table' )!;
+}

packages/ckeditor5-table/src/utils/ui/contextualballoon.ts

@@ -9,11 +9,12 @@
 
 import { Rect, type PositionOptions } from 'ckeditor5/src/utils';
 import { BalloonPanelView, type ContextualBalloon } from 'ckeditor5/src/ui';
-
-import { getTableWidgetAncestor } from './widget';
 import type { Editor } from 'ckeditor5/src/core';
 import type { Element, Position, Range } from 'ckeditor5/src/engine';
 
+import { getSelectionAffectedTableWidget, getTableWidgetAncestor } from './widget';
+import { getSelectionAffectedTable } from '../common';
+
 const DEFAULT_BALLOON_POSITIONS = BalloonPanelView.defaultPositions;
 
 const BALLOON_POSITIONS = [
@@ -36,16 +37,19 @@ const BALLOON_POSITIONS = [
  */
 export function repositionContextualBalloon( editor: Editor, target: string ): void {
 	const balloon: ContextualBalloon = editor.plugins.get( 'ContextualBalloon' );
+	const selection = editor.editing.view.document.selection;
+	let position;
 
-	if ( getTableWidgetAncestor( editor.editing.view.document.selection ) ) {
-		let position;
-
-		if ( target === 'cell' ) {
+	if ( target === 'cell' ) {
+		if ( getTableWidgetAncestor( selection ) ) {
 			position = getBalloonCellPositionData( editor );
-		} else {
-			position = getBalloonTablePositionData( editor );
 		}
+	}
+	else if ( getSelectionAffectedTableWidget( selection ) ) {
+		position = getBalloonTablePositionData( editor );
+	}
 
+	if ( position ) {
 		balloon.updatePosition( position );
 	}
 }
@@ -58,8 +62,8 @@ export function repositionContextualBalloon( editor: Editor, target: string ): v
  * @param editor The editor instance.
  */
 export function getBalloonTablePositionData( editor: Editor ): Partial<PositionOptions> {
-	const firstPosition = editor.model.document.selection.getFirstPosition()!;
-	const modelTable = firstPosition.findAncestor( 'table' )!;
+	const selection = editor.model.document.selection;
+	const modelTable = getSelectionAffectedTable( selection );
 	const viewTable = editor.editing.mapper.toViewElement( modelTable )!;
 
 	return {

packages/ckeditor5-table/src/utils/ui/widget.ts

@@ -11,6 +11,19 @@ import type { ViewDocumentFragment, ViewDocumentSelection, ViewElement, ViewNode
 
 import { isWidget } from 'ckeditor5/src/widget';
 
+/**
+ * Depending on the position of the selection either return the selected table or the table higher in the hierarchy.
+ */
+export function getSelectionAffectedTableWidget( selection: ViewDocumentSelection ): ViewElement | null {
+	const selectedTable = getSelectedTableWidget( selection );
+
+	if ( selectedTable ) {
+		return selectedTable;
+	}
+
+	return getTableWidgetAncestor( selection );
+}
+
 /**
  * Returns a table widget editing view element if one is selected.
  */

packages/ckeditor5-table/tests/tablecaption/utils.js

@@ -4,7 +4,6 @@
  */
 
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
-import Selection from '@ckeditor/ckeditor5-engine/src/model/selection';
 import View from '@ckeditor/ckeditor5-engine/src/view/view';
 import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
@@ -15,7 +14,6 @@ import TableEditing from '../../src/tableediting';
 import {
 	getCaptionFromModelSelection,
 	getCaptionFromTableModelElement,
-	getSelectionAffectedTable,
 	isTable,
 	matchTableCaptionViewElement
 } from '../../src/tablecaption/utils';
@@ -212,23 +210,4 @@ describe( 'table caption utils', () => {
 			} );
 		} );
 	} );
-
-	describe( 'getSelectionAffectedTable', () => {
-		it( 'should return null if table is not present', () => {
-			setModelData( model, '<paragraph>Foo[]</paragraph>' );
-			const selection = new Selection( model.createPositionFromPath( modelRoot, [ 0 ] ) );
-
-			const tableElement = getSelectionAffectedTable( selection );
-
-			expect( tableElement ).to.be.null;
-		} );
-
-		it( 'should return table if present higher in the model tree', () => {
-			const selection = new Selection( model.createPositionFromPath( modelRoot, [ 0, 0, 0 ] ) );
-
-			const tableElement = getSelectionAffectedTable( selection );
-
-			expect( tableElement ).to.equal( modelRoot.getNodeByPath( [ 0 ] ) );
-		} );
-	} );
 } );

packages/ckeditor5-table/tests/tableproperties/commands/tablealignmentcommand.js

@@ -49,10 +49,15 @@ describe( 'table properties', () => {
 						expect( command.isEnabled ).to.be.false;
 					} );
 
-					it( 'should be true is selection has table', () => {
+					it( 'should be true if selection is in a table', () => {
 						setData( model, modelTable( [ [ 'f[o]o' ] ] ) );
 						expect( command.isEnabled ).to.be.true;
 					} );
+
+					it( 'should be true if table is selected', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ] ) + ']' );
+						expect( command.isEnabled ).to.be.true;
+					} );
 				} );
 			} );
 
@@ -78,23 +83,35 @@ describe( 'table properties', () => {
 				} );
 
 				describe( 'non-collapsed selection', () => {
-					it( 'should be false if selection does not have table', () => {
+					it( 'should be undefined if selection does not have table', () => {
 						setData( model, '<paragraph>f[oo]</paragraph>' );
 
 						expect( command.value ).to.be.undefined;
 					} );
 
-					it( 'should be undefined if selected table has set the default value', () => {
+					it( 'should be undefined if selected table has set the default value (selection inside table)', () => {
 						setData( model, modelTable( [ [ 'f[o]o' ] ], { tableAlignment: 'center' } ) );
 
 						expect( command.value ).to.be.undefined;
 					} );
 
-					it( 'should be true is selection has table', () => {
+					it( 'should be set if selection has table (selection inside table)', () => {
 						setData( model, modelTable( [ [ 'f[o]o' ] ], { tableAlignment: 'left' } ) );
 
 						expect( command.value ).to.equal( 'left' );
 					} );
+
+					it( 'should be undefined if selected table has set the default value (selected table)', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ], { tableAlignment: 'center' } ) + ']' );
+
+						expect( command.value ).to.be.undefined;
+					} );
+
+					it( 'should be set if selection has table (selected table)', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ], { tableAlignment: 'left' } ) + ']' );
+
+						expect( command.value ).to.equal( 'left' );
+					} );
 				} );
 			} );
 
@@ -142,7 +159,7 @@ describe( 'table properties', () => {
 					} );
 				} );
 
-				describe( 'non-collapsed selection', () => {
+				describe( 'non-collapsed selection (inside table)', () => {
 					it( 'should set selected table alignment to a passed value', () => {
 						setData( model, modelTable( [ [ '[foo]' ] ] ) );
 
@@ -175,6 +192,40 @@ describe( 'table properties', () => {
 						assertTableStyle( editor, '' );
 					} );
 				} );
+
+				describe( 'non-collapsed selection (outside table)', () => {
+					it( 'should set selected table alignment to a passed value', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ] ) + ']' );
+
+						command.execute( { value: 'right' } );
+
+						assertTableStyle( editor, null, 'float:right;' );
+					} );
+
+					it( 'should change selected table alignment to a passed value', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ] ) + ']' );
+
+						command.execute( { value: 'right' } );
+
+						assertTableStyle( editor, null, 'float:right;' );
+					} );
+
+					it( 'should remove alignment from a selected table if no value is passed', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ] ) + ']' );
+
+						command.execute();
+
+						assertTableStyle( editor, '' );
+					} );
+
+					it( 'should not set alignment in a selected table if passed the default value', () => {
+						setData( model, '[' + modelTable( [ [ 'foo' ] ] ) + ']' );
+
+						command.execute( { value: 'center' } );
+
+						assertTableStyle( editor, '' );
+					} );
+				} );
 			} );
 		} );
 	} );