From 8c8ed3506d82f70404f69f6fc18602ce14fcc308 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 27 Mar 2020 19:45:34 +0100 Subject: [PATCH 1/3] Position of table cell properties balloon should be in relation to multiple selected cells. --- src/ui/utils.js | 57 +++++++++++++++++++++--- tests/ui/utils.js | 109 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 153 insertions(+), 13 deletions(-) diff --git a/src/ui/utils.js b/src/ui/utils.js index 76ae080a..f16ae38f 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -15,6 +15,7 @@ import ColorInputView from './colorinputview'; import { isColor, isLength, isPercentage } from '@ckeditor/ckeditor5-engine/src/view/styles/utils'; import { getTableWidgetAncestor } from '../utils'; import { findAncestor } from '../commands/utils'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; const DEFAULT_BALLOON_POSITIONS = BalloonPanelView.defaultPositions; const BALLOON_POSITIONS = [ @@ -81,14 +82,28 @@ export function getBalloonTablePositionData( editor ) { * @returns {module:utils/dom/position~Options} */ export function getBalloonCellPositionData( editor ) { - // This is a bit naive. See https://github.com/ckeditor/ckeditor5/issues/6357. - const modelTableCell = getTableCellAtPosition( editor.model.document.selection.getFirstPosition() ); - const viewTableCell = editor.editing.mapper.toViewElement( modelTableCell ); + const mapper = editor.editing.mapper; + const domConverter = editor.editing.view.domConverter; + const ranges = Array.from( editor.model.document.selection.getRanges() ); + + if ( ranges.length > 1 ) { + return { + target: () => createBoundingRect( ranges, modelRange => { + const modelTableCell = getTableCellAtPosition( modelRange.start ); + const viewTableCell = mapper.toViewElement( modelTableCell ); + return new Rect( domConverter.viewToDom( viewTableCell ) ); + } ), + positions: BALLOON_POSITIONS + }; + } else { + const modelTableCell = getTableCellAtPosition( ranges[ 0 ].start ); + const viewTableCell = mapper.toViewElement( modelTableCell ); - return { - target: editor.editing.view.domConverter.viewToDom( viewTableCell ), - positions: BALLOON_POSITIONS - }; + return { + target: domConverter.viewToDom( viewTableCell ), + positions: BALLOON_POSITIONS + }; + } } /** @@ -478,3 +493,31 @@ function getTableCellAtPosition( position ) { return isTableCellSelected ? position.nodeAfter : findAncestor( 'tableCell', position ); } + +// Returns bounding rect for list of rects. +// +// @param {Array.|Array.<*>} list List of `Rect`s or any list to map by `mapFn`. +// @param {Function} [mapFn] Mapping function for list elements. +// @returns {module:utils/dom/rect~Rect} +function createBoundingRect( list, mapFn = null ) { + const rectData = { + left: Number.POSITIVE_INFINITY, + top: Number.POSITIVE_INFINITY, + right: Number.NEGATIVE_INFINITY, + bottom: Number.NEGATIVE_INFINITY + }; + + for ( const item of list ) { + const rect = mapFn ? mapFn( item ) : item; + + rectData.left = Math.min( rectData.left, rect.left ); + rectData.top = Math.min( rectData.top, rect.top ); + rectData.right = Math.max( rectData.right, rect.right ); + rectData.bottom = Math.max( rectData.bottom, rect.bottom ); + } + + rectData.width = rectData.right - rectData.left; + rectData.height = rectData.bottom - rectData.top; + + return new Rect( rectData ); +} diff --git a/tests/ui/utils.js b/tests/ui/utils.js index d19baf85..4a161a94 100644 --- a/tests/ui/utils.js +++ b/tests/ui/utils.js @@ -28,10 +28,14 @@ import { fillToolbar } from '../../src/ui/utils'; import Collection from '@ckeditor/ckeditor5-utils/src/collection'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import { modelTable } from '../_utils/utils'; describe( 'UI Utils', () => { let editor, editingView, balloon, editorElement; + testUtils.createSinonSandbox(); + beforeEach( () => { editorElement = global.document.createElement( 'div' ); global.document.body.appendChild( editorElement ); @@ -152,14 +156,35 @@ describe( 'UI Utils', () => { } ); describe( 'getBalloonCellPositionData()', () => { - it( 'returns the position data', () => { - const defaultPositions = BalloonPanelView.defaultPositions; + let modelRoot; - setData( editor.model, '' + - 'foo' + - '[bar]' + - '
' ); + beforeEach( () => { + setData( editor.model, modelTable( [ + [ '11[]', '12', '13' ], + [ '21', '22', '23' ], + [ '31', '32', '33' ] + ] ) ); + + modelRoot = editor.model.document.getRoot(); + + for ( let row = 0; row < 3; row++ ) { + for ( let col = 0; col < 3; col++ ) { + const modelCell = modelRoot.getNodeByPath( [ 0, row, col ] ); + const viewCell = editor.editing.mapper.toViewElement( modelCell ); + const cellDomElement = editingView.domConverter.viewToDom( viewCell ); + + mockBoundingBox( cellDomElement, { + top: 100 + row * 10, + left: 100 + col * 10, + height: 10, + width: 10 + } ); + } + } + } ); + it( 'returns the position data', () => { + const defaultPositions = BalloonPanelView.defaultPositions; const data = getBalloonCellPositionData( editor ); const modelCell = getTableCellsContainingSelection( editor.model.document.selection )[ 0 ]; const viewCell = editor.editing.mapper.toViewElement( modelCell ); @@ -176,6 +201,78 @@ describe( 'UI Utils', () => { ] } ); } ); + + it( 'returns the position data for multiple cells selected horizontally', () => { + selectTableCells( [ + [ 0, 0 ], + [ 0, 1 ] + ] ); + + const data = getBalloonCellPositionData( editor ); + const targetData = data.target(); + + expect( targetData ).to.deep.equal( { + top: 100, + left: 100, + right: 120, + bottom: 110, + width: 20, + height: 10 + } ); + } ); + + it( 'returns the position data for multiple cells selected vertically', () => { + selectTableCells( [ + [ 0, 1 ], + [ 1, 1 ] + ] ); + + const data = getBalloonCellPositionData( editor ); + const targetData = data.target(); + + expect( targetData ).to.deep.equal( { + top: 100, + left: 110, + right: 120, + bottom: 120, + width: 10, + height: 20 + } ); + } ); + + it( 'returns the position data for multiple cells selected', () => { + selectTableCells( [ + [ 0, 1 ], + [ 1, 0 ], + [ 1, 1 ] + ] ); + + const data = getBalloonCellPositionData( editor ); + const targetData = data.target(); + + expect( targetData ).to.deep.equal( { + top: 100, + left: 100, + right: 120, + bottom: 120, + width: 20, + height: 20 + } ); + } ); + + function selectTableCells( paths ) { + editor.model.change( writer => { + writer.setSelection( paths.map( path => writer.createRangeOn( modelRoot.getNodeByPath( [ 0, ...path ] ) ) ) ); + } ); + } + + function mockBoundingBox( element, data ) { + testUtils.sinon.stub( element, 'getBoundingClientRect' ).returns( { + ...data, + right: data.left + data.width, + bottom: data.top + data.height + } ); + } } ); describe( 'getBorderStyleLabels()', () => { From d5caa2acc3e43460ee7766ed3574440336a82ac9 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 30 Mar 2020 11:40:02 +0200 Subject: [PATCH 2/3] Removed unused code branch (optional mapFn). --- src/ui/utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ui/utils.js b/src/ui/utils.js index f16ae38f..019bf4d9 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -497,9 +497,9 @@ function getTableCellAtPosition( position ) { // Returns bounding rect for list of rects. // // @param {Array.|Array.<*>} list List of `Rect`s or any list to map by `mapFn`. -// @param {Function} [mapFn] Mapping function for list elements. +// @param {Function} mapFn Mapping function for list elements. // @returns {module:utils/dom/rect~Rect} -function createBoundingRect( list, mapFn = null ) { +function createBoundingRect( list, mapFn ) { const rectData = { left: Number.POSITIVE_INFINITY, top: Number.POSITIVE_INFINITY, @@ -508,7 +508,7 @@ function createBoundingRect( list, mapFn = null ) { }; for ( const item of list ) { - const rect = mapFn ? mapFn( item ) : item; + const rect = mapFn( item ); rectData.left = Math.min( rectData.left, rect.left ); rectData.top = Math.min( rectData.top, rect.top ); From d5ddad1838beeb3bd46301eda918a018858fc3f4 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 30 Mar 2020 14:00:09 +0200 Subject: [PATCH 3/3] Code style fixes. --- src/ui/utils.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ui/utils.js b/src/ui/utils.js index 019bf4d9..b0ee0d52 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -84,26 +84,26 @@ export function getBalloonTablePositionData( editor ) { export function getBalloonCellPositionData( editor ) { const mapper = editor.editing.mapper; const domConverter = editor.editing.view.domConverter; - const ranges = Array.from( editor.model.document.selection.getRanges() ); + const selection = editor.model.document.selection; - if ( ranges.length > 1 ) { + if ( selection.rangeCount > 1 ) { return { - target: () => createBoundingRect( ranges, modelRange => { + target: () => createBoundingRect( selection.getRanges(), modelRange => { const modelTableCell = getTableCellAtPosition( modelRange.start ); const viewTableCell = mapper.toViewElement( modelTableCell ); return new Rect( domConverter.viewToDom( viewTableCell ) ); } ), positions: BALLOON_POSITIONS }; - } else { - const modelTableCell = getTableCellAtPosition( ranges[ 0 ].start ); - const viewTableCell = mapper.toViewElement( modelTableCell ); - - return { - target: domConverter.viewToDom( viewTableCell ), - positions: BALLOON_POSITIONS - }; } + + const modelTableCell = getTableCellAtPosition( selection.getFirstPosition() ); + const viewTableCell = mapper.toViewElement( modelTableCell ); + + return { + target: domConverter.viewToDom( viewTableCell ), + positions: BALLOON_POSITIONS + }; } /**