Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Added keepAttachedTo method to the BalloonPanelView. #172

Merged
merged 20 commits into from
Apr 5, 2017
Merged

Added keepAttachedTo method to the BalloonPanelView. #172

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2d9f309
Added `stickTo` method to the BalloonPanelView.
oskarwrobel Mar 20, 2017
3a6b741
Limited balloon panel updaing on scroll.
oskarwrobel Mar 31, 2017
9233160
Updated panel position on resize.
oskarwrobel Mar 31, 2017
a5c1dab
Added manual test for BalloonPanelView#keepAttachedTo.
oskarwrobel Mar 31, 2017
4d1443f
Used global namespace for global dom object.
oskarwrobel Mar 31, 2017
e078d48
Moved manual test to tickets.
oskarwrobel Mar 31, 2017
30904a7
Merge branch 'master' into t/170
oleq Apr 3, 2017
a3f0934
Added full url to the ticket in docs.
oskarwrobel Apr 3, 2017
fc6ec1a
Simplified manual test.
oskarwrobel Apr 3, 2017
724a590
Reduced similar test cases.
oskarwrobel Apr 3, 2017
f3840ae
Cleaned view listeners in destroy method.
oskarwrobel Apr 3, 2017
8692e48
Added test for default limiter.
oskarwrobel Apr 3, 2017
3941c56
Docs: Enhanced BalloonPanelView#keepAttachedTo() docs.
oleq Apr 4, 2017
af93584
Added support for Range and Rect as a target.
oskarwrobel Apr 4, 2017
4a31312
Using sinon assertions in tests.
oskarwrobel Apr 4, 2017
dd62d00
Added full ticket url.
oskarwrobel Apr 4, 2017
5fbbe27
Simplfied the test.
oskarwrobel Apr 4, 2017
b7d759e
Optimized BalloonPanelView#keepAttachedTo by extracting some logic fr…
oleq Apr 5, 2017
e369e41
Improved test.
oskarwrobel Apr 5, 2017
3c55902
Fixed failing test.
oskarwrobel Apr 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions src/panel/balloon/balloonpanelview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
* @module ui/panel/balloon/balloonpanelview
*/

/* globals document */

import View from '../../view';
import Template from '../../template';
import { getOptimalPosition } from '@ckeditor/ckeditor5-utils/src/dom/position';
import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';

const toPx = toUnit( 'px' );
const defaultLimiterElement = global.document.body;

/**
* The balloon panel view class.
Expand Down Expand Up @@ -151,14 +151,48 @@ export default class BalloonPanelView extends View {
defaultPositions.ne,
defaultPositions.nw
],
limiter: document.body,
limiter: defaultLimiterElement,
fitInViewport: true
}, options );

const { top, left, name: position } = getOptimalPosition( positionOptions );

Object.assign( this, { top, left, position } );
}

/**
* Works exactly the same as {module:ui/panel/balloon/balloonpanelview~BalloonPanelView.attachTo} with one exception.
* Position of attached panel is constantly updated when any of parents of the target or limiter elements are scrolled
* or when browser window is resized. Thanks to this panel always sticks to the target element. See #170.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full link to the issue would be safer. We had code moving between repos and such hashes became outdated more than once already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*
* @param {module:utils/dom/position~Options} options Positioning options compatible with
* {@link module:utils/dom/position~getOptimalPosition}. Default `positions` array is
* {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions}.
*/
keepAttachedTo( options ) {
// First we need to attach the balloon panel to the target element.
this.attachTo( options );

const target = options.target;
const limiter = options.limiter || defaultLimiterElement;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line lacks test coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


// Then we need to listen on scroll event of eny element in the document.
this.listenTo( global.document, 'scroll', ( evt, domEvt ) => {
// And update position if scrolled element contains related to the balloon elements.
if ( domEvt.target.contains( target ) || domEvt.target.contains( limiter ) ) {
Copy link
Member

@oleq oleq Apr 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen if options.target or options.limiter are Range or ClientRect? I suppose it's not gonna work and it may throw errors.

For the DOM Range we can do it by obtaining range ancestor Node. For ClientRect, there's nothing we can do because it's an abstract geometric entity, I guess. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

this.attachTo( options );
}
}, { useCapture: true } );

// We need to listen on window resize event and update position.
this.listenTo( global.window, 'resize', () => this.attachTo( options ) );

// After all we need to clean up the listener.
this.once( 'change:isVisible', () => {
this.stopListening( global.document, 'scroll' );
this.stopListening( global.window, 'resize' );
} );
}
}

/**
Expand Down
104 changes: 104 additions & 0 deletions tests/manual/tickets/170/1.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<div class="container-a">
<div class="container-b">
<div>
<div id="editor-attach">
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p>Balloon is attached to the <strong>TARGET</strong> element.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use position: absolute or simply large margin and drop these paragraphs. It just looks weird :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
</div>
</div>

<div>
<div id="editor-stick">
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p>Balloon sticks to the <strong>TARGET</strong> element.</p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
</div>
</div>
</div>
</div>

<style>
body {
height: 100%;
}

.balloon-content {
padding: 10px;
font-size: 16px;
}

.ck-editor__editable {
max-height: 300px;
}

.container-a {
height: 500px;
width: 750px;
overflow: scroll;
}

.container-b {
padding: 500px 30px 30px;
display: flex;
width: 1000px;
height: 2000px;
background: #e1e1e1;
}

.container-b > div {
margin: 0 20px;
}
</style>
86 changes: 86 additions & 0 deletions tests/manual/tickets/170/1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals window, document, console:false */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';
import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import View from '@ckeditor/ckeditor5-ui/src/view';
import Template from '@ckeditor/ckeditor5-ui/src/template';

// Content of the balloon panel.
class BalloonContentView extends View {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KISS. Add simple styles

.ck-balloon-panel {
    padding: 2em;
}

and then balloonPanel.innerHTML = 'Balloon'. It should be about what is important in that particular test case, not about balloon content, view nesting and the such. It's just a noise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

constructor() {
super();

this.template = new Template( {
tag: 'div',
attributes: {
class: 'balloon-content'
},
children: [
{
text: 'Balloon'
}
]
} );
}
}

// Set initial scroll of one of the container element.
document.querySelector( '.container-a' ).scrollTop = 450;

// Init editor with balloon attached to the target element.
ClassicEditor.create( document.querySelector( '#editor-attach' ), {
plugins: [ ArticlePresets ],
toolbar: [ 'bold', 'italic', 'undo', 'redo' ]
} )
.then( editor => {
const panel = new BalloonPanelView();

panel.content.add( new BalloonContentView() );
editor.ui.view.body.add( panel );

editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360;

panel.init().then( () => {
panel.attachTo( {
target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ),
limiter: editor.ui.view.editableElement
} );
} );

window.attachEditor = editor;
} )
.catch( err => {
console.error( err.stack );
} );

// Init editor with balloon sticked to the target element.
ClassicEditor.create( document.querySelector( '#editor-stick' ), {
plugins: [ ArticlePresets ],
toolbar: [ 'bold', 'italic', 'undo', 'redo' ]
} )
.then( editor => {
const panel = new BalloonPanelView();

panel.content.add( new BalloonContentView() );
editor.ui.view.body.add( panel );

editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360;

panel.init().then( () => {
panel.keepAttachedTo( {
target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ),
limiter: editor.ui.view.editableElement
} );
} );

window.stickEditor = editor;
} )
.catch( err => {
console.error( err.stack );
} );
4 changes: 4 additions & 0 deletions tests/manual/tickets/170/1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## BalloonPanelView `attachTo` vs `keepAttachedTo`

Scroll editable elements and container (horizontally as well). Balloon in the left editor should float but balloon in the
right editor should stick to the target element.
Loading