Skip to content

Commit

Permalink
Merge pull request #6810 from ckeditor/i/6190
Browse files Browse the repository at this point in the history
Fix (widget): The widget toolbar should always be visible even if the widget is longer than a visible viewport (see #6190).

Fix (table): The table properties balloon should always be visible if the table is longer than a visible viewport. Closes #6190.
  • Loading branch information
niegowski authored May 15, 2020
2 parents f18f4fd + f060cce commit 75d6912
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 6 deletions.
7 changes: 6 additions & 1 deletion packages/ckeditor5-table/src/ui/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { isColor, isLength, isPercentage } from '@ckeditor/ckeditor5-engine/src/
import { getTableWidgetAncestor } from '../utils';
import { findAncestor } from '../commands/utils';
import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
import { centeredBalloonPositionForLongWidgets } from '@ckeditor/ckeditor5-widget/src/utils';

const DEFAULT_BALLOON_POSITIONS = BalloonPanelView.defaultPositions;
const BALLOON_POSITIONS = [
Expand All @@ -26,6 +27,10 @@ const BALLOON_POSITIONS = [
DEFAULT_BALLOON_POSITIONS.southArrowNorthWest,
DEFAULT_BALLOON_POSITIONS.southArrowNorthEast
];
const TABLE_PROPERTRIES_BALLOON_POSITIONS = [
...BALLOON_POSITIONS,
centeredBalloonPositionForLongWidgets
];

const isEmpty = val => val === '';

Expand Down Expand Up @@ -69,7 +74,7 @@ export function getBalloonTablePositionData( editor ) {

return {
target: editor.editing.view.domConverter.viewToDom( viewTable ),
positions: BALLOON_POSITIONS
positions: TABLE_PROPERTRIES_BALLOON_POSITIONS
};
}

Expand Down
4 changes: 3 additions & 1 deletion packages/ckeditor5-table/tests/ui/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
} from '../../src/ui/utils';
import Collection from '@ckeditor/ckeditor5-utils/src/collection';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import { centeredBalloonPositionForLongWidgets } from '@ckeditor/ckeditor5-widget/src/utils';
import { modelTable } from '../_utils/utils';

describe( 'UI Utils', () => {
Expand Down Expand Up @@ -139,7 +140,8 @@ describe( 'UI Utils', () => {
defaultPositions.northArrowSouthEast,
defaultPositions.southArrowNorth,
defaultPositions.southArrowNorthWest,
defaultPositions.southArrowNorthEast
defaultPositions.southArrowNorthEast,
centeredBalloonPositionForLongWidgets
]
} );
} );
Expand Down
55 changes: 55 additions & 0 deletions packages/ckeditor5-widget/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@

import HighlightStack from './highlightstack';
import IconView from '@ckeditor/ckeditor5-ui/src/icon/iconview';
import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';

import dragHandleIcon from '../theme/icons/drag-handle.svg';

Expand Down Expand Up @@ -339,6 +342,58 @@ export function viewToModelPositionOutsideModelElement( model, viewElementMatche
};
}

/**
* A positioning function passed to the {@link module:utils/dom/position~getOptimalPosition} helper as a last resort
* when attaching {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView balloon UI} to widgets.
* It comes in handy when a widget is longer than the visual viewport of the web browser and/or upper/lower boundaries
* of a widget are off screen because of the web page scroll.
*
* ┌─┄┄┄┄┄┄┄┄┄Widget┄┄┄┄┄┄┄┄┄┐
* ┊ ┊
* ┌────────────Viewport───────────┐ ┌──╁─────────Viewport────────╁──┐
* │ ┏━━━━━━━━━━Widget━━━━━━━━━┓ │ │ ┃ ^ ┃ │
* │ ┃ ^ ┃ │ │ ┃ ╭───────/ \───────╮ ┃ │
* │ ┃ ╭───────/ \───────╮ ┃ │ │ ┃ │ Balloon │ ┃ │
* │ ┃ │ Balloon │ ┃ │ │ ┃ ╰─────────────────╯ ┃ │
* │ ┃ ╰─────────────────╯ ┃ │ │ ┃ ┃ │
* │ ┃ ┃ │ │ ┃ ┃ │
* │ ┃ ┃ │ │ ┃ ┃ │
* │ ┃ ┃ │ │ ┃ ┃ │
* │ ┃ ┃ │ │ ┃ ┃ │
* │ ┃ ┃ │ │ ┃ ┃ │
* │ ┃ ┃ │ │ ┃ ┃ │
* └──╀─────────────────────────╀──┘ └──╀─────────────────────────╀──┘
* ┊ ┊ ┊ ┊
* ┊ ┊ └┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┘
* ┊ ┊
* └┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┘
*
* **Note**: Works best if used together with
* {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions default `BalloonPanelView` positions}
* like `northArrowSouth` and `southArrowNorth`; the transition between these two and this position is smooth.
*
* @param {module:utils/dom/rect~Rect} widgetRect A rect of the widget.
* @param {module:utils/dom/rect~Rect} balloonRect A rect of the balloon.
* @returns {module:utils/dom/position~Position}
*/
export function centeredBalloonPositionForLongWidgets( widgetRect, balloonRect ) {
const viewportRect = new Rect( global.window );
const viewportWidgetInsersectionRect = viewportRect.getIntersection( widgetRect );

// Because this is a last resort positioning, to keep things simple we're not playing with positions of the arrow
// like, for instance, "south west" or whatever. Just try to keep the balloon in the middle of the visible area of
// the widget for as long as it is possible. If the widgets becomes invisible (because cropped by the viewport),
// just... place the balloon in the middle of it (because why not?).
const targetRect = viewportWidgetInsersectionRect || widgetRect;
const left = targetRect.left + targetRect.width / 2 - balloonRect.width / 2;

return {
top: Math.max( widgetRect.top, 0 ) + BalloonPanelView.arrowVerticalOffset,
left,
name: 'arrow_n'
};
}

// Default filler offset function applied to all widget elements.
//
// @returns {null}
Expand Down
8 changes: 6 additions & 2 deletions packages/ckeditor5-widget/src/widgettoolbarrepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import { isWidget } from './utils';
import {
isWidget,
centeredBalloonPositionForLongWidgets
} from './utils';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

/**
Expand Down Expand Up @@ -272,7 +275,8 @@ function getBalloonPositionData( editor, relatedElement ) {
defaultPositions.northArrowSouthEast,
defaultPositions.southArrowNorth,
defaultPositions.southArrowNorthWest,
defaultPositions.southArrowNorthEast
defaultPositions.southArrowNorthEast,
centeredBalloonPositionForLongWidgets
]
};
}
Expand Down
105 changes: 104 additions & 1 deletion packages/ckeditor5-widget/tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import {
setHighlightHandling,
findOptimalInsertionPosition,
viewToModelPositionOutsideModelElement,
WIDGET_CLASS_NAME
WIDGET_CLASS_NAME,
centeredBalloonPositionForLongWidgets
} from '../src/utils';
import UIElement from '@ckeditor/ckeditor5-engine/src/view/uielement';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
Expand All @@ -29,6 +30,9 @@ import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import Mapper from '@ckeditor/ckeditor5-engine/src/conversion/mapper';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ModelText from '@ckeditor/ckeditor5-engine/src/model/text';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';
import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';

describe( 'widget utils', () => {
let element, writer, viewDocument;
Expand Down Expand Up @@ -489,4 +493,103 @@ describe( 'widget utils', () => {
expect( modelPosition.path ).to.deep.equal( [ 3, 1 ] );
} );
} );

describe( 'centeredBalloonPositionForLongWidgets()', () => {
const arrowVerticalOffset = BalloonPanelView.arrowVerticalOffset;

// Balloon is a 10x10 rect.
const balloonRect = new Rect( {
top: 0,
left: 0,
right: 10,
bottom: 10,
width: 10,
height: 10
} );

beforeEach( () => {
testUtils.sinon.stub( global.window, 'innerWidth' ).value( 100 );
testUtils.sinon.stub( global.window, 'innerHeight' ).value( 100 );
} );

it( 'should position the balloon inside a widget – at the top + in the middle', () => {
// Widget is a 50x150 rect, translated (25,25) from viewport's beginning (0,0).
const widgetRect = new Rect( {
top: 25,
left: 25,
right: 75,
bottom: 175,
width: 50,
height: 150
} );

const position = centeredBalloonPositionForLongWidgets( widgetRect, balloonRect );

expect( position ).to.deep.equal( {
top: 25 + arrowVerticalOffset,
left: 45,
name: 'arrow_n'
} );
} );

it( 'should stick the balloon to the top of the viewport when the top of a widget is off-screen', () => {
// Widget is a 50x150 rect, translated (25,-25) from viewport's beginning (0,0).
const widgetRect = new Rect( {
top: -25,
left: 25,
right: 75,
bottom: 150,
width: 50,
height: 150
} );

const position = centeredBalloonPositionForLongWidgets( widgetRect, balloonRect );

expect( position ).to.deep.equal( {
top: arrowVerticalOffset,
left: 45,
name: 'arrow_n'
} );
} );

it( 'should horizontally center the balloon in the visible area when the widget is cropped by the viewport', () => {
// Widget is a 50x150 rect, translated (25,-25) from viewport's beginning (0,0).
const widgetRect = new Rect( {
top: 25,
left: -25,
right: 25,
bottom: 175,
width: 50,
height: 150
} );

const position = centeredBalloonPositionForLongWidgets( widgetRect, balloonRect );

expect( position ).to.deep.equal( {
top: 25 + arrowVerticalOffset,
left: 7.5,
name: 'arrow_n'
} );
} );

it( 'should horizontally center the balloon in the widget when the widget is completely off the viewport', () => {
// Widget is a 50x150 rect, translated (0,-100) from viewport's beginning (0,0).
const widgetRect = new Rect( {
top: 0,
left: -100,
right: -50,
bottom: 150,
width: 50,
height: 150
} );

const position = centeredBalloonPositionForLongWidgets( widgetRect, balloonRect );

expect( position ).to.deep.equal( {
top: 0 + arrowVerticalOffset,
left: -80,
name: 'arrow_n'
} );
} );
} );
} );
41 changes: 40 additions & 1 deletion packages/ckeditor5-widget/tests/widgettoolbarrepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@

import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import BalloonEditor from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote';
import Widget from '../src/widget';
import WidgetToolbarRepository from '../src/widgettoolbarrepository';
import { isWidget, toWidget } from '../src/utils';
import {
isWidget,
toWidget,
centeredBalloonPositionForLongWidgets
} from '../src/utils';
import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';
import View from '@ckeditor/ckeditor5-ui/src/view';

Expand Down Expand Up @@ -470,6 +475,40 @@ describe( 'WidgetToolbarRepository', () => {

expect( balloon.view.pin.lastCall.args[ 0 ].target ).to.equal( newFakeDomElement );
} );

it( 'toolbar should use one of pre-defined positions when attaching to a widget', () => {
const editingView = editor.editing.view;
const balloonAddSpy = sinon.spy( balloon, 'add' );
const defaultPositions = BalloonPanelView.defaultPositions;

widgetToolbarRepository.register( 'fake', {
items: editor.config.get( 'fake.toolbar' ),
getRelatedElement: getSelectedFakeWidget
} );

setData( model, '<paragraph>foo</paragraph>[<fake-widget></fake-widget>]' );

const fakeWidgetToolbarView = widgetToolbarRepository._toolbarDefinitions.get( 'fake' ).view;
const widgetViewElement = editingView.document.getRoot().getChild( 1 );

sinon.assert.calledOnce( balloonAddSpy );
sinon.assert.calledWithExactly( balloonAddSpy, {
view: fakeWidgetToolbarView,
position: {
target: editingView.domConverter.mapViewToDom( widgetViewElement ),
positions: [
defaultPositions.northArrowSouth,
defaultPositions.northArrowSouthWest,
defaultPositions.northArrowSouthEast,
defaultPositions.southArrowNorth,
defaultPositions.southArrowNorthWest,
defaultPositions.southArrowNorthEast,
centeredBalloonPositionForLongWidgets
]
},
balloonClassName: 'ck-toolbar-container'
} );
} );
} );
} );

Expand Down

0 comments on commit 75d6912

Please sign in to comment.