Balloon Panel and related components

[oskarwrobel]

Fixes #55 and #54

This PR introduces BalloonPanel component and related to LinkBalloonPanel components.

Components are moved from ckeditor5-link repository, little refactored, tested and documented.

FollowUp for missing manual tests: #61
Related to: ckeditor/ckeditor5-theme-lark#34

[Edit] Now I see that this PR could be cut into smaller pieces. There is a lot of code :/

[oskarwrobel]

We need to decide how we want to deal with keyboard events in UI components.
Right now this code is not testable, because it is difficult/not possible to dispatch Keyboard event with specified charCode on Webkit. Mostly we will need listener for Esc to close some popups, balloons etc, but I can imagine components with more advanced keyboard support (like #16).

[oleq]

It should be done by

this.listenTo( document, 'keydown', () => {}` );
this.stopListening( document, 'keydown' );

which is via DOMEmitterMixin class. See how ListDropdownView works and its tests.

[oleq]

Simulating key(Code) event should be possible via key property instead of keyCode which is marked as deprecated.

document.addEventListener( 'keydown', ( evt ) => console.log( evt ) );
document.dispatchEvent( new KeyboardEvent( 'keydown', { key: '20', keyCode: '20', code: '20' } ) );

BTW: KeyboardEvent.key uses verbal description like "Escape".

[oskarwrobel]

I think I was trying by 'key' property too but I'll try your solution.

[oleq]

And yeah, KeboardEvent.key is unsupported in Safari https://bugs.webkit.org/show_bug.cgi?id=69029#c13 :(

[oleq]

Alright. Given the above mess I see no other option than:

1. Checking if this._keyCloseCallback is called at all on document.body.dispatchEvent( new Event( 'keydown' );
2. Testing what happens if this._keyCloseCallback is called standalone with a synthetic evt object containing different keyCodes (would it call hide then or not?).

[Reinmar]

The tricky thing with keyCode is that key isn't supported in every browser yet. keyCode is (with some exceptions – namely, onscreen keyboard on Android). This means that keyCode is used by everyone anyway, so it doesn't actually change anything that it's deprecated – everyone use it.

[oleq]

This condition lacks test coverage.

[oskarwrobel]

Yep, I mentioned about it in comment below. There is a problem with dispatching Keyboard event with specified keyCode.

[oleq]

I guess we can live without LabeledInput here too.

[oleq]

A more general impression: Do we need BalloonPanelView#hide event? Would not BalloonPanelView.model#change:isVisible do the same job?

[oskarwrobel]

A more general impression: Do we need BalloonPanelView#hide event? Would not BalloonPanelView.model#change:isVisible do the same job?

I've removed it.

[oleq]

I guess there's some refactoring needed in inputs/labels/etc.

1. LabeledInput should become LabeledInputText or allow passing the class of the input in the constructor. ATM it is strictly bound to InputText but the name of the class does not reflect it.
2. InputLabel should become just Label. It can label inputs, selects, textareas. It should also be reflected in the CSS class of the component, IMO.
3. If LabeledInput becomes LabeledInputText, it should land in inputtext/labeled folder. Just like dropdown/list.

[oleq]

I was also wondering what to do with the Box component.

First of all, some binding between the model and the class attribute would not hurt.

Secondly, I wonder if the component is needed at all. ATM it exists just to wrap some elements in DOM and style them (simply align them to the right/left). It's an overkill to use JS and create yet another branch in the UI's DOM to do this. It might be a little bit easier if LinkBalloonPanelView simply extended the template of BalloonPanelView and created additional child/region with a desired CSS class to accommodate the buttons and eventually align them.

[oleq]

I checked and it's possible to extend the template with a new children

export default class LinkBalloonPanelView extends BalloonPanelView {
    /**
     * @inheritDoc
     */
    constructor( locale ) {
        super( locale );

        Template.extend( this.template, {
            attributes: {
                class: [
                    'ck-link-balloon-panel',
                ]
            }
        } );

        this.template.definition.children = [
            {
                tag: 'div',
                attributes: {
                    class: [
                        'moo'
                    ]
                }
            }
        ];
    }
}

but it does not work via Template.extend, which has been designed to extend existing children only (like their attributes) in assumption that if the number of children does not match, then it must be a different component (class). I remember talking to @Reinmar about it and we came up with this behaviour, which, TBH, I don't quite consider right anymore in the context of the problem we're facing right now.

WDYT? Is is a different component because it comes with a new container/region for the buttons? Or maybe we should allow this kind of extension for the sake of similar container–like components in the future?

[oskarwrobel]

WDYT? Is is a different component because it comes with a new container/region for the buttons? Or maybe we should allow this kind of extension for the sake of similar container–like components in the future?

I like it, I was trying to do exactly the same (I mean create a new region for buttons instead of using Box component), but i had a problem with CKEditorError: ui-template-extend-children-mismatch: The number of children in extended definition does not match.. Looks like this.template.definition.children solves the problem.

[oskarwrobel]

Here will be more types in the feature.