Skip to content

Commit

Permalink
Table properties should be enabled if table is selected from the outs…
Browse files Browse the repository at this point in the history
…ide.
  • Loading branch information
niegowski committed Oct 6, 2023
1 parent f1a686e commit e4e6e91
Show file tree
Hide file tree
Showing 18 changed files with 476 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 2 additions & 14 deletions packages/ckeditor5-table/src/tablecaption/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*
Expand Down Expand Up @@ -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' )!;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 );
Expand All @@ -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 => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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' );
Expand Down
17 changes: 16 additions & 1 deletion packages/ckeditor5-table/src/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import type {
Item,
Position,
Schema,
Writer
Writer,
DocumentSelection
} from 'ckeditor5/src/engine';

import { downcastAttributeToStyle, upcastStyleToAttribute } from './../converters/tableproperties';
Expand Down Expand Up @@ -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' )!;
}
24 changes: 14 additions & 10 deletions packages/ckeditor5-table/src/utils/ui/contextualballoon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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 );
}
}
Expand All @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions packages/ckeditor5-table/src/utils/ui/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
21 changes: 0 additions & 21 deletions packages/ckeditor5-table/tests/tablecaption/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -15,7 +14,6 @@ import TableEditing from '../../src/tableediting';
import {
getCaptionFromModelSelection,
getCaptionFromTableModelElement,
getSelectionAffectedTable,
isTable,
matchTableCaptionViewElement
} from '../../src/tablecaption/utils';
Expand Down Expand Up @@ -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 ] ) );
} );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -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;
} );
} );
} );

Expand All @@ -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' );
} );
} );
} );

Expand Down Expand Up @@ -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]' ] ] ) );

Expand Down Expand Up @@ -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, '' );
} );
} );
} );
} );
} );
Expand Down

0 comments on commit e4e6e91

Please sign in to comment.