From 7248774d352bd76ce1a4078b3ba0265b5df57922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 15:33:32 +0200 Subject: [PATCH 1/8] Improved focus managment test. --- tests/link.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/link.js b/tests/link.js index c3d4e78..71aca75 100644 --- a/tests/link.js +++ b/tests/link.js @@ -220,6 +220,19 @@ describe( 'Link', () => { sinon.assert.calledOnce( spy ); } ); + it( 'should keep editor ui focused when link form has focus', () => { + editor.ui.focusTracker.isFocused = false; + + // Open balloon panel with link inside. + linkButton.fire( 'execute' ); + + // Be sure that form view is focused. + formView.element.dispatchEvent( new Event( 'focus' ) ); + + // Check if editor ui is focused. + expect( editor.ui.focusTracker.isFocused ).to.true; + } ); + describe( 'close listeners', () => { describe( 'keyboard', () => { it( 'should close after Esc key press (from editor) and not focus editable', () => { From 472713e57abbbf5814aebd42147c680d5306ba17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 13 Apr 2017 09:12:43 +0200 Subject: [PATCH 2/8] Fixed issues of focus managment. --- src/link.js | 13 +++++++------ tests/link.js | 26 +++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/link.js b/src/link.js index f6eb95c..de1866a 100644 --- a/src/link.js +++ b/src/link.js @@ -240,20 +240,21 @@ export default class Link extends Plugin { * * @private * @param {Boolean} [focusInput=false] When `true`, link form will be focused on panel show. + * @return {Promise} A promise resolved when the {#formView} {@link module:ui/view~View#init} is done. */ _showPanel( focusInput ) { if ( this._balloon.hasView( this.formView ) ) { - return; + return Promise.resolve(); } - this._balloon.add( { + return this._balloon.add( { view: this.formView, position: this._getBalloonPositionData() + } ).then( () => { + if ( focusInput ) { + this.formView.urlInputView.select(); + } } ); - - if ( focusInput ) { - this.formView.urlInputView.select(); - } } /** diff --git a/tests/link.js b/tests/link.js index c6667ff..1a1571e 100644 --- a/tests/link.js +++ b/tests/link.js @@ -139,7 +139,11 @@ describe( 'Link', () => { linkButton.fire( 'execute' ); - expect( selectUrlInputSpy.calledOnce ).to.true; + // Focus of url input is called async after internal promise resolve and we are + // not able to return this promise. + return wait().then( () => { + expect( selectUrlInputSpy.calledOnce ).to.true; + } ); } ); } ); @@ -185,7 +189,12 @@ describe( 'Link', () => { editor.keystrokes.press( { keyCode: keyCodes.k, ctrlKey: true } ); expect( balloon.visibleView ).to.equal( formView ); - expect( selectUrlInputSpy.calledOnce ).to.true; + + // Focus of url input is called async after internal promise resolve and we are + // not able to return this promise. + return wait().then( () => { + expect( selectUrlInputSpy.calledOnce ).to.true; + } ); } ); it( 'should not add panel to ContextualBalloon more than once', () => { @@ -341,7 +350,12 @@ describe( 'Link', () => { observer.fire( 'click', { target: document.body } ); expect( balloon.visibleView ).to.equal( formView ); - expect( selectUrlInputSpy.notCalled ).to.true; + + // Focus of url input is called async after internal promise resolve and we are + // not able to return this promise. + return wait().then( () => { + expect( selectUrlInputSpy.notCalled ).to.true; + } ); } ); it( 'should keep open and update position until collapsed selection stay inside the same link element', () => { @@ -569,3 +583,9 @@ describe( 'Link', () => { } ); } ); } ); + +function wait( delay = 1 ) { + return new Promise( ( resolve ) => { + setTimeout( () => resolve(), delay ); + } ); +} From 2204eb9861494612954a8c964270a4ac1c1dba6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 13 Apr 2017 15:36:34 +0200 Subject: [PATCH 3/8] Made showPanel and hidePanel public and refactored tests. --- src/link.js | 38 ++-- tests/link.js | 523 ++++++++++++++++++++++++++++---------------------- 2 files changed, 318 insertions(+), 243 deletions(-) diff --git a/src/link.js b/src/link.js index de1866a..b6436e4 100644 --- a/src/link.js +++ b/src/link.js @@ -90,21 +90,21 @@ export default class Link extends Plugin { // Execute link command after clicking on formView `Save` button. this.listenTo( formView, 'submit', () => { editor.execute( 'link', formView.urlInputView.inputView.element.value ); - this._hidePanel( true ); + this.hidePanel( true ); } ); // Execute unlink command after clicking on formView `Unlink` button. this.listenTo( formView, 'unlink', () => { editor.execute( 'unlink' ); - this._hidePanel( true ); + this.hidePanel( true ); } ); // Hide the panel after clicking on formView `Cancel` button. - this.listenTo( formView, 'cancel', () => this._hidePanel( true ) ); + this.listenTo( formView, 'cancel', () => this.hidePanel( true ) ); // Close the panel on esc key press when the form has focus. formView.keystrokes.set( 'Esc', ( data, cancel ) => { - this._hidePanel( true ); + this.hidePanel( true ); cancel(); } ); @@ -123,7 +123,7 @@ export default class Link extends Plugin { const t = editor.t; // Handle `Ctrl+K` keystroke and show the panel. - editor.keystrokes.set( 'CTRL+K', () => this._showPanel( true ) ); + editor.keystrokes.set( 'CTRL+K', () => this.showPanel( true ) ); editor.ui.componentFactory.add( 'link', ( locale ) => { const button = new ButtonView( locale ); @@ -138,7 +138,7 @@ export default class Link extends Plugin { button.bind( 'isEnabled' ).to( linkCommand, 'isEnabled' ); // Show the panel on button click. - this.listenTo( button, 'execute', () => this._showPanel( true ) ); + this.listenTo( button, 'execute', () => this.showPanel( true ) ); return button; } ); @@ -191,7 +191,7 @@ export default class Link extends Plugin { // When collapsed selection is inside link element (link element is clicked). if ( viewSelection.isCollapsed && parentLink ) { // Then show panel but keep focus inside editor editable. - this._showPanel(); + this.showPanel(); // Avoid duplication of the same listener. this.stopListening( viewDocument, 'render' ); @@ -202,7 +202,7 @@ export default class Link extends Plugin { const currentParentLink = getPositionParentLink( viewSelection.getFirstPosition() ); if ( !viewSelection.isCollapsed || parentLink !== currentParentLink ) { - this._hidePanel(); + this.hidePanel(); } else { this._balloon.updatePosition(); } @@ -221,7 +221,7 @@ export default class Link extends Plugin { // Close the panel on the Esc key press when the editable has focus and the balloon is visible. this.editor.keystrokes.set( 'Esc', ( data, cancel ) => { if ( this._balloon.visibleView === this.formView ) { - this._hidePanel(); + this.hidePanel(); cancel(); } } ); @@ -231,19 +231,24 @@ export default class Link extends Plugin { emitter: this.formView, activator: () => this._balloon.hasView( this.formView ), contextElement: this._balloon.view.element, - callback: () => this._hidePanel() + callback: () => this.hidePanel() } ); } /** * Adds the {@link #formView} to the {@link #_balloon}. + * When view is already added then try to focus it `focusInput` parameter is set as true. * - * @private * @param {Boolean} [focusInput=false] When `true`, link form will be focused on panel show. * @return {Promise} A promise resolved when the {#formView} {@link module:ui/view~View#init} is done. */ - _showPanel( focusInput ) { + showPanel( focusInput ) { if ( this._balloon.hasView( this.formView ) ) { + // Check if formView should be focused and focus it if is visible. + if ( focusInput && this._balloon.visibleView === this.formView ) { + this.formView.urlInputView.select(); + } + return Promise.resolve(); } @@ -260,20 +265,19 @@ export default class Link extends Plugin { /** * Removes the {@link #formView} from the {@link #_balloon}. * - * @private * @param {Boolean} [focusEditable=false] When `true`, editable focus will be restored on panel hide. */ - _hidePanel( focusEditable ) { + hidePanel( focusEditable ) { if ( !this._balloon.hasView( this.formView ) ) { return; } - this._balloon.remove( this.formView ); - this.stopListening( this.editor.editing.view, 'render' ); - if ( focusEditable ) { this.editor.editing.view.focus(); } + + this.stopListening( this.editor.editing.view, 'render' ); + this._balloon.remove( this.formView ); } /** diff --git a/tests/link.js b/tests/link.js index 1a1571e..6d73e5b 100644 --- a/tests/link.js +++ b/tests/link.js @@ -10,6 +10,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Link from '../src/link'; import LinkEngine from '../src/linkengine'; import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon'; @@ -28,7 +29,7 @@ describe( 'Link', () => { document.body.appendChild( editorElement ); return ClassicTestEditor.create( editorElement, { - plugins: [ Link ] + plugins: [ Link, Paragraph ] } ) .then( newEditor => { newEditor.editing.view.attachDomRoot( editorElement ); @@ -69,81 +70,211 @@ describe( 'Link', () => { expect( editor.editing.view.getObserver( ClickObserver ) ).to.instanceOf( ClickObserver ); } ); - describe( 'link toolbar button', () => { - it( 'should register link button', () => { - expect( linkButton ).to.instanceOf( ButtonView ); + describe( 'showPanel()', () => { + let balloonAddSpy; + + beforeEach( () => { + balloonAddSpy = testUtils.sinon.spy( balloon, 'add' ); + editor.editing.view.isFocused = true; } ); - it( 'should bind linkButtonView to link command', () => { - const command = editor.commands.get( 'link' ); + it( 'should return promise', () => { + // @TODO: test resolved promise. + expect( linkFeature.showPanel() ).to.instanceof( Promise ); + } ); - command.isEnabled = true; - expect( linkButton.isEnabled ).to.be.true; + it( 'should add `formView` to the `ContextualBalloon` and attach panel to the selection when text fragment is selected', () => { + setModelData( editor.document, 'f[o]o' ); + const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); - command.isEnabled = false; - expect( linkButton.isEnabled ).to.be.false; + return linkFeature.showPanel() + .then( () => { + expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.calledWithExactly( balloonAddSpy, { + view: formView, + position: { + target: selectedRange, + limiter: editorElement + } + } ); + } ); } ); - it( 'should add link form to the ContextualBalloon on execute event', () => { - linkButton.fire( 'execute' ); + it( 'should add `formView` to the `ContextualBalloon` and attach panel to the selection when selection is collapsed', () => { + setModelData( editor.document, 'f[]oo' ); + const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); - expect( balloon.visibleView ).to.equal( formView ); + return linkFeature.showPanel() + .then( () => { + expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.calledWithExactly( balloonAddSpy, { + view: formView, + position: { + target: selectedRange, + limiter: editorElement + } + } ); + } ); } ); - it( 'should add link form to the ContextualBalloon and attach balloon to the link element ' + - 'when collapsed selection is inside link element', + it( 'should add `formView` to the `ContextualBalloon` and attach panel to the link element when collapsed selection is inside ' + + 'link element', () => { - const balloonAddSpy = testUtils.sinon.spy( balloon, 'add' ); + setModelData( editor.document, '<$text linkHref="url">f[]oo' ); + const linkElement = editorElement.querySelector( 'a' ); - editor.document.schema.allow( { name: '$text', inside: '$root' } ); - setModelData( editor.document, '<$text linkHref="url">some[] url' ); - editor.editing.view.isFocused = true; + return linkFeature.showPanel() + .then( () => { + expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.calledWithExactly( balloonAddSpy, { + view: formView, + position: { + target: linkElement, + limiter: editorElement + } + } ); + } ); + } ); - linkButton.fire( 'execute' ); + it( 'should not focus `formView` at default', () => { + const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - const linkElement = editorElement.querySelector( 'a' ); + return linkFeature.showPanel() + .then( () => { + sinon.assert.notCalled( spy ); + } ); + } ); - sinon.assert.calledWithExactly( balloonAddSpy, { - view: formView, - position: { - target: linkElement, - limiter: editorElement - } - } ); + it( 'should not focus `formView` when is called with `false` parameter', () => { + const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); + + return linkFeature.showPanel( false ) + .then( () => { + sinon.assert.notCalled( spy ); + } ); } ); - it( 'should add link form to the ContextualBalloon and attach balloon to the selection, when selection is non-collapsed', () => { - const balloonAddSpy = testUtils.sinon.spy( balloon, 'add' ); + it( 'should not focus `formView` when is called with `true` parameter while balloon is opened but link form is not visible', () => { + const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); + const viewMock = { + ready: true, + init: () => {}, + destroy: () => {} + }; - editor.document.schema.allow( { name: '$text', inside: '$root' } ); - setModelData( editor.document, 'so[me ur]l' ); - editor.editing.view.isFocused = true; + return linkFeature.showPanel( false ) + .then( () => balloon.add( { view: viewMock } ) ) + .then( () => linkFeature.showPanel( true ) ) + .then( () => { + sinon.assert.notCalled( spy ); + } ); + } ); - linkButton.fire( 'execute' ); + it( 'should focus `formView` when is called with `true` parameter', () => { + const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); + return linkFeature.showPanel( true ) + .then( () => { + sinon.assert.calledOnce( spy ); + } ); + } ); - sinon.assert.calledWithExactly( balloonAddSpy, { - view: formView, - position: { - target: selectedRange, - limiter: editorElement - } - } ); + it( 'should focus `formView` when is called with `true` parameter while balloon is opened and linkForm is visible', () => { + const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); + + return linkFeature.showPanel( false ) + .then( () => linkFeature.showPanel( true ) ) + .then( () => { + sinon.assert.calledOnce( spy ); + } ); } ); - it( 'should select link input value when link balloon is opened', () => { - const selectUrlInputSpy = testUtils.sinon.spy( linkFeature.formView.urlInputView, 'select' ); + it( 'should keep editor ui focused when panel is shown with selected form', () => { + editor.ui.focusTracker.isFocused = false; - editor.editing.view.isFocused = true; + // Open balloon panel with link inside. + return linkFeature.showPanel( true ) + .then( () => { + // Check if editor ui is focused. + expect( editor.ui.focusTracker.isFocused ).to.true; + } ); + } ); + } ); + + describe( 'hidePanel()', () => { + beforeEach( () => { + return balloon.add( { view: formView } ); + } ); + + it( 'should remove `formView` from the `ContextualBalloon` component', () => { + linkFeature.hidePanel(); + expect( balloon.hasView( formView ) ).to.false; + } ); + + it( 'should not focus `editable` at default', () => { + const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + linkFeature.hidePanel(); + sinon.assert.notCalled( spy ); + } ); + + it( 'should not focus `editable` when is called with `false` parameter', () => { + const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + linkFeature.hidePanel( false ); + sinon.assert.notCalled( spy ); + } ); + + it( 'should focus `editable` when is called with `true` parameter', () => { + const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + linkFeature.hidePanel( true ); + sinon.assert.calledOnce( spy ); + } ); + + it( 'should do not throw an error when `formView` is not added to the `balloon`', () => { + linkFeature.hidePanel( true ); + + expect( () => { + linkFeature.hidePanel( true ); + } ).to.not.throw(); + } ); + + it( 'should clear `render` listener from ViewDocument', () => { + const spy = sinon.spy(); + + linkFeature.listenTo( editor.editing.view, 'render', spy ); + + linkFeature.hidePanel(); + + editor.editing.view.render(); + + sinon.assert.notCalled( spy ); + } ); + } ); + + describe( 'link toolbar button', () => { + it( 'should register link button', () => { + expect( linkButton ).to.instanceOf( ButtonView ); + } ); + + it( 'should bind linkButtonView to link command', () => { + const command = editor.commands.get( 'link' ); + + command.isEnabled = true; + expect( linkButton.isEnabled ).to.be.true; + + command.isEnabled = false; + expect( linkButton.isEnabled ).to.be.false; + } ); + + it( 'should show panel on execute event with selected `formView`', () => { + // Method is stubbed because it returns internal promise which can't be returned in test. + const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); linkButton.fire( 'execute' ); - // Focus of url input is called async after internal promise resolve and we are - // not able to return this promise. - return wait().then( () => { - expect( selectUrlInputSpy.calledOnce ).to.true; - } ); + sinon.assert.calledWithExactly( spy, true ); } ); } ); @@ -172,42 +303,17 @@ describe( 'Link', () => { } ); } ); - describe( 'ContextualBalloon', () => { - let focusEditableSpy; - - beforeEach( () => { - focusEditableSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); - } ); - - it( 'should not be added to ContextualBalloon at default', () => { - expect( balloon.visibleView ).to.null; - } ); - - it( 'should be added to ContextualBalloon and form should be selected on `CTRL+K` keystroke', () => { - const selectUrlInputSpy = testUtils.sinon.spy( formView.urlInputView, 'select' ); + describe( 'keyboard support', () => { + it( 'should show panel with selected `formView` on `CTRL+K` keystroke', () => { + // Method is stubbed because it returns internal promise which can't be returned in test. + const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); editor.keystrokes.press( { keyCode: keyCodes.k, ctrlKey: true } ); - expect( balloon.visibleView ).to.equal( formView ); - - // Focus of url input is called async after internal promise resolve and we are - // not able to return this promise. - return wait().then( () => { - expect( selectUrlInputSpy.calledOnce ).to.true; - } ); - } ); - - it( 'should not add panel to ContextualBalloon more than once', () => { - // Add panel to balloon by pressing toolbar button. - linkButton.fire( 'execute' ); - - // Press button once again. - expect( () => { - linkButton.fire( 'execute' ); - } ).to.not.throw(); + sinon.assert.calledWithExactly( spy, true ); } ); - it( 'should focus the link form on Tab key press', () => { + it( 'should focus the `formView` on `Tab` key press when panel is open', () => { const keyEvtData = { keyCode: keyCodes.tab, preventDefault: sinon.spy(), @@ -225,7 +331,7 @@ describe( 'Link', () => { sinon.assert.notCalled( spy ); // Balloon is visible, form focused. - return balloon.add( { view: formView } ) + return linkFeature.showPanel( true ) .then( () => { formView.focusTracker.isFocused = true; @@ -244,122 +350,109 @@ describe( 'Link', () => { } ); } ); - it( 'should keep editor ui focused when link form has focus', () => { - editor.ui.focusTracker.isFocused = false; - - // Open balloon panel with link inside. - linkButton.fire( 'execute' ); + it( 'should hide panel after Esc key press (from editor) and not focus editable', () => { + const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const keyEvtData = { + keyCode: keyCodes.esc, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; - // Be sure that form view is focused. - formView.element.dispatchEvent( new Event( 'focus' ) ); + // Balloon is visible. + return linkFeature.showPanel( false ).then( () => { + editor.keystrokes.press( keyEvtData ); - // Check if editor ui is focused. - expect( editor.ui.focusTracker.isFocused ).to.true; + sinon.assert.calledWithExactly( spy ); + } ); } ); - describe( 'close listeners', () => { - describe( 'keyboard', () => { - it( 'should close after Esc key press (from editor) and not focus editable', () => { - const keyEvtData = { - keyCode: keyCodes.esc, - preventDefault: sinon.spy(), - stopPropagation: sinon.spy() - }; + it( 'should not hide panel after Esc key press (from editor) when panel is open but is not visible', () => { + const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const keyEvtData = { + keyCode: keyCodes.esc, + preventDefault: () => {}, + stopPropagation: () => {} + }; - // Balloon is visible. - return balloon.add( { view: formView } ).then( () => { - editor.keystrokes.press( keyEvtData ); + const viewMock = { + ready: true, + init: () => {}, + destroy: () => {} + }; - expect( balloon.visibleView ).to.null; - sinon.assert.notCalled( focusEditableSpy ); - } ); - } ); + return linkFeature.showPanel( false ) + .then( () => balloon.add( { view: viewMock } ) ) + .then( () => { + editor.keystrokes.press( keyEvtData ); - it( 'should not close after Esc key press (from editor) when panel is in stack but not visible', () => { - const keyEvtData = { - keyCode: keyCodes.esc, - preventDefault: () => {}, - stopPropagation: () => {} - }; - - const viewMock = { - init: () => {}, - destroy: () => {} - }; - - return balloon.add( { view: formView } ) - .then( () => { - return balloon.add( { view: viewMock } ); - } ) - .then( () => { - editor.keystrokes.press( keyEvtData ); - - expect( balloon.visibleView ).to.equal( viewMock ); - expect( balloon.hasView( formView ) ).to.true; - sinon.assert.notCalled( focusEditableSpy ); - } ); + sinon.assert.notCalled( spy ); } ); + } ); - it( 'should close after Esc key press (from the form) and focus editable', () => { - const keyEvtData = { - keyCode: keyCodes.esc, - preventDefault: sinon.spy(), - stopPropagation: sinon.spy() - }; + it( 'should hide panel after Esc key press (from the form) and focus editable', () => { + const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const keyEvtData = { + keyCode: keyCodes.esc, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; - return balloon.add( { view: formView } ) - .then( () => { - formView.keystrokes.press( keyEvtData ); + return linkFeature.showPanel( true ) + .then( () => { + formView.keystrokes.press( keyEvtData ); - expect( balloon.visibleView ).to.null; - sinon.assert.calledOnce( focusEditableSpy ); - } ); + sinon.assert.calledWithExactly( spy, true ); } ); - } ); + } ); + } ); + + describe( 'mouse support', () => { + it( 'should hide panel and not focus editable on click outside the panel', () => { + const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); - describe( 'mouse', () => { - it( 'should close and not focus editable on click outside the panel', () => { - return balloon.add( { view: formView } ) - .then( () => { - document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) ); + return linkFeature.showPanel( true ) + .then( () => { + document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) ); - expect( balloon.visibleView ).to.null; - expect( focusEditableSpy.notCalled ).to.true; - } ); + sinon.assert.calledWithExactly( spy ); } ); + } ); + + it( 'should not hide panel on click inside the panel', () => { + const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); - it( 'should not close on click inside the panel', () => { - return balloon.add( { view: formView } ) - .then( () => { - balloon.view.element.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) ); + return linkFeature.showPanel( true ) + .then( () => { + balloon.view.element.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) ); - expect( balloon.visibleView ).to.equal( formView ); - } ); + sinon.assert.notCalled( spy ); } ); - } ); } ); - describe( 'click on editable', () => { - it( 'should open with not selected url input when collapsed selection is inside link element', () => { - const selectUrlInputSpy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - const observer = editor.editing.view.getObserver( ClickObserver ); + describe( 'clicking on editable', () => { + let observer; + + beforeEach( () => { + observer = editor.editing.view.getObserver( ClickObserver ); + } ); + + it( 'should open with not selected formView when collapsed selection is inside link element', () => { + // Method is stubbed because it returns internal promise which can't be returned in test. + const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">fo[]o' ); observer.fire( 'click', { target: document.body } ); - expect( balloon.visibleView ).to.equal( formView ); - - // Focus of url input is called async after internal promise resolve and we are - // not able to return this promise. - return wait().then( () => { - expect( selectUrlInputSpy.notCalled ).to.true; - } ); + sinon.assert.calledWithExactly( spy ); } ); it( 'should keep open and update position until collapsed selection stay inside the same link element', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); + // Method is stubbed because it returns internal promise which can't be returned in test. + const showSpy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + const updatePositionSpy = testUtils.sinon.stub( balloon, 'updatePosition', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">b[]ar' ); @@ -369,22 +462,24 @@ describe( 'Link', () => { observer.fire( 'click', { target: document.body } ); - expect( balloon.visibleView ).to.equal( formView ); - - const updatePositionSpy = testUtils.sinon.spy( balloon, 'updatePosition' ); + // Panel is shown. + sinon.assert.calledOnce( showSpy ); // Move selection. editor.editing.view.selection.setRanges( [ Range.createFromParentsAndOffsets( text, 1, text, 1 ) ], true ); editor.editing.view.render(); - // Check if balloon is still open and position was updated. - expect( balloon.visibleView ).to.equal( formView ); - expect( updatePositionSpy.calledOnce ).to.true; + // Check if balloon is still opened (wasn't hide). + sinon.assert.notCalled( hideSpy ); + // And position was updated + sinon.assert.calledOnce( updatePositionSpy ); } ); it( 'should not duplicate `render` listener on `ViewDocument`', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); - const updatePositionSpy = testUtils.sinon.spy( balloon, 'updatePosition' ); + const updatePositionSpy = testUtils.sinon.stub( balloon, 'updatePosition', () => {} ); + + // Method is stubbed because it returns internal promise which can't be returned in test. + testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">b[]ar' ); @@ -408,7 +503,10 @@ describe( 'Link', () => { } ); it( 'should close when selection goes outside the link element', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); + const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + + // Method is stubbed because it returns internal promise which can't be returned in test. + testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, 'foo <$text linkHref="url">b[]ar' ); @@ -418,16 +516,19 @@ describe( 'Link', () => { observer.fire( 'click', { target: document.body } ); - expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.notCalled( hideSpy ); editor.editing.view.selection.setRanges( [ Range.createFromParentsAndOffsets( text, 3, text, 3 ) ], true ); editor.editing.view.render(); - expect( balloon.visibleView ).to.null; + sinon.assert.calledOnce( hideSpy ); } ); it( 'should close when selection goes to the other link element with the same href', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); + const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + + // Method is stubbed because it returns internal promise which can't be returned in test. + testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">f[]oo bar <$text linkHref="url">biz' ); @@ -437,16 +538,19 @@ describe( 'Link', () => { observer.fire( 'click', { target: document.body } ); - expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.notCalled( hideSpy ); editor.editing.view.selection.setRanges( [ Range.createFromParentsAndOffsets( text, 1, text, 1 ) ], true ); editor.editing.view.render(); - expect( balloon.visibleView ).to.null; + sinon.assert.calledOnce( hideSpy ); } ); it( 'should close when selection becomes non-collapsed', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); + const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + + // Method is stubbed because it returns internal promise which can't be returned in test. + testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">f[]oo' ); @@ -456,58 +560,31 @@ describe( 'Link', () => { observer.fire( 'click', { target: {} } ); - expect( balloon.visibleView ).to.equal( formView ); - editor.editing.view.selection.setRanges( [ Range.createFromParentsAndOffsets( text, 1, text, 2 ) ] ); editor.editing.view.render(); - expect( balloon.visibleView ).to.null; - } ); - - it( 'should stop updating position after close', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); - - editor.document.schema.allow( { name: '$text', inside: '$root' } ); - setModelData( editor.document, '<$text linkHref="url">b[]ar' ); - - const root = editor.editing.view.getRoot(); - const text = root.getChild( 0 ).getChild( 0 ); - - observer.fire( 'click', { target: {} } ); - - expect( balloon.visibleView ).to.equal( formView ); - - // Close balloon by dispatching `cancel` event on formView. - formView.fire( 'cancel' ); - - const updatePositionSpy = testUtils.sinon.spy( balloon, 'updatePosition' ); - - // Move selection inside link element. - editor.editing.view.selection.setRanges( [ Range.createFromParentsAndOffsets( text, 2, text, 2 ) ], true ); - editor.editing.view.render(); - - expect( updatePositionSpy.notCalled ).to.true; + sinon.assert.calledOnce( hideSpy ); } ); it( 'should not open when selection is not inside link element', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); + const showSpy = testUtils.sinon.stub( linkFeature, 'showPanel' ); setModelData( editor.document, '[]' ); observer.fire( 'click', { target: {} } ); - expect( balloon.visibleView ).to.null; + sinon.assert.notCalled( showSpy ); } ); it( 'should not open when selection is non-collapsed', () => { - const observer = editor.editing.view.getObserver( ClickObserver ); + const showSpy = testUtils.sinon.stub( linkFeature, 'showPanel' ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">f[o]o' ); - observer.fire( 'click', { target: document.body } ); + observer.fire( 'click', { target: {} } ); - expect( balloon.visibleView ).to.null; + sinon.assert.notCalled( showSpy ); } ); } ); } ); @@ -543,7 +620,7 @@ describe( 'Link', () => { } ); it( 'should hide and focus editable on formView#submit event', () => { - return balloon.add( { view: formView } ) + return linkFeature.showPanel() .then( () => { formView.fire( 'submit' ); @@ -562,7 +639,7 @@ describe( 'Link', () => { } ); it( 'should hide and focus editable on formView#unlink event', () => { - return balloon.add( { view: formView } ) + return linkFeature.showPanel() .then( () => { formView.fire( 'unlink' ); @@ -572,7 +649,7 @@ describe( 'Link', () => { } ); it( 'should hide and focus editable on formView#cancel event', () => { - return balloon.add( { view: formView } ) + return linkFeature.showPanel() .then( () => { formView.fire( 'cancel' ); @@ -583,9 +660,3 @@ describe( 'Link', () => { } ); } ); } ); - -function wait( delay = 1 ) { - return new Promise( ( resolve ) => { - setTimeout( () => resolve(), delay ); - } ); -} From 9b36d273b2c7c3c9f8ff165cdff8b2f676150461 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 14 Apr 2017 13:36:13 +0200 Subject: [PATCH 4/8] Tests: Refactoring in Link tests. --- tests/link.js | 58 ++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/tests/link.js b/tests/link.js index 6d73e5b..8d02396 100644 --- a/tests/link.js +++ b/tests/link.js @@ -79,17 +79,21 @@ describe( 'Link', () => { } ); it( 'should return promise', () => { - // @TODO: test resolved promise. - expect( linkFeature.showPanel() ).to.instanceof( Promise ); + const returned = linkFeature.showPanel(); + + expect( returned ).to.instanceof( Promise ); + + return returned; } ); - it( 'should add `formView` to the `ContextualBalloon` and attach panel to the selection when text fragment is selected', () => { + it( 'should add #formView to the #_balloon and attach the #_balloon to the selection when text fragment is selected', () => { setModelData( editor.document, 'f[o]o' ); const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); return linkFeature.showPanel() .then( () => { expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.calledWithExactly( balloonAddSpy, { view: formView, position: { @@ -100,13 +104,14 @@ describe( 'Link', () => { } ); } ); - it( 'should add `formView` to the `ContextualBalloon` and attach panel to the selection when selection is collapsed', () => { + it( 'should add #formView to the #_balloon and attach the #_balloon to the selection when selection is collapsed', () => { setModelData( editor.document, 'f[]oo' ); const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); return linkFeature.showPanel() .then( () => { expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.calledWithExactly( balloonAddSpy, { view: formView, position: { @@ -117,8 +122,8 @@ describe( 'Link', () => { } ); } ); - it( 'should add `formView` to the `ContextualBalloon` and attach panel to the link element when collapsed selection is inside ' + - 'link element', + it( 'should add #formView to the #_balloon and attach the #_balloon to the link element when collapsed selection is inside ' + + 'that link', () => { setModelData( editor.document, '<$text linkHref="url">f[]oo' ); const linkElement = editorElement.querySelector( 'a' ); @@ -126,6 +131,7 @@ describe( 'Link', () => { return linkFeature.showPanel() .then( () => { expect( balloon.visibleView ).to.equal( formView ); + sinon.assert.calledWithExactly( balloonAddSpy, { view: formView, position: { @@ -136,7 +142,7 @@ describe( 'Link', () => { } ); } ); - it( 'should not focus `formView` at default', () => { + it( 'should not focus the #formView at default', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); return linkFeature.showPanel() @@ -145,7 +151,7 @@ describe( 'Link', () => { } ); } ); - it( 'should not focus `formView` when is called with `false` parameter', () => { + it( 'should not focus the #formView when called with a `false` parameter', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); return linkFeature.showPanel( false ) @@ -154,7 +160,7 @@ describe( 'Link', () => { } ); } ); - it( 'should not focus `formView` when is called with `true` parameter while balloon is opened but link form is not visible', () => { + it( 'should not focus the #formView when called with a `true` parameter while the balloon is opened but link form is not visible', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); const viewMock = { ready: true, @@ -170,7 +176,7 @@ describe( 'Link', () => { } ); } ); - it( 'should focus `formView` when is called with `true` parameter', () => { + it( 'should focus the #formView when called with a `true` parameter', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); return linkFeature.showPanel( true ) @@ -179,7 +185,7 @@ describe( 'Link', () => { } ); } ); - it( 'should focus `formView` when is called with `true` parameter while balloon is opened and linkForm is visible', () => { + it( 'should focus the #formView when called with a `true` parameter while the balloon is open and the #formView is visible', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); return linkFeature.showPanel( false ) @@ -189,10 +195,10 @@ describe( 'Link', () => { } ); } ); - it( 'should keep editor ui focused when panel is shown with selected form', () => { + it( 'should keep the editor ui focused when the #_balloon is shown with the selected form', () => { editor.ui.focusTracker.isFocused = false; - // Open balloon panel with link inside. + // Open the #_balloon with the link inside. return linkFeature.showPanel( true ) .then( () => { // Check if editor ui is focused. @@ -206,33 +212,33 @@ describe( 'Link', () => { return balloon.add( { view: formView } ); } ); - it( 'should remove `formView` from the `ContextualBalloon` component', () => { + it( 'should remove #formView from the #_balloon', () => { linkFeature.hidePanel(); expect( balloon.hasView( formView ) ).to.false; } ); - it( 'should not focus `editable` at default', () => { + it( 'should not focus the `editable` by default', () => { const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); linkFeature.hidePanel(); sinon.assert.notCalled( spy ); } ); - it( 'should not focus `editable` when is called with `false` parameter', () => { + it( 'should not focus the `editable` when called with a `false` parameter', () => { const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); linkFeature.hidePanel( false ); sinon.assert.notCalled( spy ); } ); - it( 'should focus `editable` when is called with `true` parameter', () => { + it( 'should focus the `editable` when called with a `true` parameter', () => { const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); linkFeature.hidePanel( true ); sinon.assert.calledOnce( spy ); } ); - it( 'should do not throw an error when `formView` is not added to the `balloon`', () => { + it( 'should not throw an error when #formView is not added to the `balloon`', () => { linkFeature.hidePanel( true ); expect( () => { @@ -268,7 +274,7 @@ describe( 'Link', () => { expect( linkButton.isEnabled ).to.be.false; } ); - it( 'should show panel on execute event with selected `formView`', () => { + it( 'should show the #_balloon on execute event with the selected #formView', () => { // Method is stubbed because it returns internal promise which can't be returned in test. const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); @@ -304,7 +310,7 @@ describe( 'Link', () => { } ); describe( 'keyboard support', () => { - it( 'should show panel with selected `formView` on `CTRL+K` keystroke', () => { + it( 'should show the #_balloon with selected #formView on `CTRL+K` keystroke', () => { // Method is stubbed because it returns internal promise which can't be returned in test. const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); @@ -313,7 +319,7 @@ describe( 'Link', () => { sinon.assert.calledWithExactly( spy, true ); } ); - it( 'should focus the `formView` on `Tab` key press when panel is open', () => { + it( 'should focus the the #formView on `Tab` key press when the #_balloon is open', () => { const keyEvtData = { keyCode: keyCodes.tab, preventDefault: sinon.spy(), @@ -350,7 +356,7 @@ describe( 'Link', () => { } ); } ); - it( 'should hide panel after Esc key press (from editor) and not focus editable', () => { + it( 'should hide the #_balloon after Esc key press (from editor) and not focus the editable', () => { const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); const keyEvtData = { keyCode: keyCodes.esc, @@ -366,7 +372,7 @@ describe( 'Link', () => { } ); } ); - it( 'should not hide panel after Esc key press (from editor) when panel is open but is not visible', () => { + it( 'should not hide #_balloon after Esc key press (from editor) when #_balloon is open but is not visible', () => { const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); const keyEvtData = { keyCode: keyCodes.esc, @@ -389,7 +395,7 @@ describe( 'Link', () => { } ); } ); - it( 'should hide panel after Esc key press (from the form) and focus editable', () => { + it( 'should hide the #_balloon after Esc key press (from the form) and focus the editable', () => { const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); const keyEvtData = { keyCode: keyCodes.esc, @@ -407,7 +413,7 @@ describe( 'Link', () => { } ); describe( 'mouse support', () => { - it( 'should hide panel and not focus editable on click outside the panel', () => { + it( 'should hide #_balloon and not focus editable on click outside the #_balloon', () => { const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); return linkFeature.showPanel( true ) @@ -418,7 +424,7 @@ describe( 'Link', () => { } ); } ); - it( 'should not hide panel on click inside the panel', () => { + it( 'should not hide #_balloon on click inside the #_balloon', () => { const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); return linkFeature.showPanel( true ) From c8d90da4ab07db856aca7685c7daf658e0e97e65 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 14 Apr 2017 13:44:15 +0200 Subject: [PATCH 5/8] Made Link Plugin's #showPanel and #hidePanel methods @protected. --- src/link.js | 40 +++++++++++---------- tests/link.js | 98 +++++++++++++++++++++++++-------------------------- 2 files changed, 70 insertions(+), 68 deletions(-) diff --git a/src/link.js b/src/link.js index b6436e4..336d94d 100644 --- a/src/link.js +++ b/src/link.js @@ -90,21 +90,21 @@ export default class Link extends Plugin { // Execute link command after clicking on formView `Save` button. this.listenTo( formView, 'submit', () => { editor.execute( 'link', formView.urlInputView.inputView.element.value ); - this.hidePanel( true ); + this._hidePanel( true ); } ); // Execute unlink command after clicking on formView `Unlink` button. this.listenTo( formView, 'unlink', () => { editor.execute( 'unlink' ); - this.hidePanel( true ); + this._hidePanel( true ); } ); // Hide the panel after clicking on formView `Cancel` button. - this.listenTo( formView, 'cancel', () => this.hidePanel( true ) ); + this.listenTo( formView, 'cancel', () => this._hidePanel( true ) ); // Close the panel on esc key press when the form has focus. formView.keystrokes.set( 'Esc', ( data, cancel ) => { - this.hidePanel( true ); + this._hidePanel( true ); cancel(); } ); @@ -123,7 +123,7 @@ export default class Link extends Plugin { const t = editor.t; // Handle `Ctrl+K` keystroke and show the panel. - editor.keystrokes.set( 'CTRL+K', () => this.showPanel( true ) ); + editor.keystrokes.set( 'CTRL+K', () => this._showPanel( true ) ); editor.ui.componentFactory.add( 'link', ( locale ) => { const button = new ButtonView( locale ); @@ -138,7 +138,7 @@ export default class Link extends Plugin { button.bind( 'isEnabled' ).to( linkCommand, 'isEnabled' ); // Show the panel on button click. - this.listenTo( button, 'execute', () => this.showPanel( true ) ); + this.listenTo( button, 'execute', () => this._showPanel( true ) ); return button; } ); @@ -191,7 +191,7 @@ export default class Link extends Plugin { // When collapsed selection is inside link element (link element is clicked). if ( viewSelection.isCollapsed && parentLink ) { // Then show panel but keep focus inside editor editable. - this.showPanel(); + this._showPanel(); // Avoid duplication of the same listener. this.stopListening( viewDocument, 'render' ); @@ -202,7 +202,7 @@ export default class Link extends Plugin { const currentParentLink = getPositionParentLink( viewSelection.getFirstPosition() ); if ( !viewSelection.isCollapsed || parentLink !== currentParentLink ) { - this.hidePanel(); + this._hidePanel(); } else { this._balloon.updatePosition(); } @@ -221,7 +221,7 @@ export default class Link extends Plugin { // Close the panel on the Esc key press when the editable has focus and the balloon is visible. this.editor.keystrokes.set( 'Esc', ( data, cancel ) => { if ( this._balloon.visibleView === this.formView ) { - this.hidePanel(); + this._hidePanel(); cancel(); } } ); @@ -231,7 +231,7 @@ export default class Link extends Plugin { emitter: this.formView, activator: () => this._balloon.hasView( this.formView ), contextElement: this._balloon.view.element, - callback: () => this.hidePanel() + callback: () => this._hidePanel() } ); } @@ -239,10 +239,11 @@ export default class Link extends Plugin { * Adds the {@link #formView} to the {@link #_balloon}. * When view is already added then try to focus it `focusInput` parameter is set as true. * + * @protected * @param {Boolean} [focusInput=false] When `true`, link form will be focused on panel show. * @return {Promise} A promise resolved when the {#formView} {@link module:ui/view~View#init} is done. */ - showPanel( focusInput ) { + _showPanel( focusInput ) { if ( this._balloon.hasView( this.formView ) ) { // Check if formView should be focused and focus it if is visible. if ( focusInput && this._balloon.visibleView === this.formView ) { @@ -253,21 +254,22 @@ export default class Link extends Plugin { } return this._balloon.add( { - view: this.formView, - position: this._getBalloonPositionData() - } ).then( () => { - if ( focusInput ) { - this.formView.urlInputView.select(); - } - } ); + view: this.formView, + position: this._getBalloonPositionData() + } ).then( () => { + if ( focusInput ) { + this.formView.urlInputView.select(); + } + } ); } /** * Removes the {@link #formView} from the {@link #_balloon}. * + * @protected * @param {Boolean} [focusEditable=false] When `true`, editable focus will be restored on panel hide. */ - hidePanel( focusEditable ) { + _hidePanel( focusEditable ) { if ( !this._balloon.hasView( this.formView ) ) { return; } diff --git a/tests/link.js b/tests/link.js index 8d02396..b6809c0 100644 --- a/tests/link.js +++ b/tests/link.js @@ -70,7 +70,7 @@ describe( 'Link', () => { expect( editor.editing.view.getObserver( ClickObserver ) ).to.instanceOf( ClickObserver ); } ); - describe( 'showPanel()', () => { + describe( '_showPanel()', () => { let balloonAddSpy; beforeEach( () => { @@ -79,7 +79,7 @@ describe( 'Link', () => { } ); it( 'should return promise', () => { - const returned = linkFeature.showPanel(); + const returned = linkFeature._showPanel(); expect( returned ).to.instanceof( Promise ); @@ -90,7 +90,7 @@ describe( 'Link', () => { setModelData( editor.document, 'f[o]o' ); const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { expect( balloon.visibleView ).to.equal( formView ); @@ -108,7 +108,7 @@ describe( 'Link', () => { setModelData( editor.document, 'f[]oo' ); const selectedRange = editorElement.ownerDocument.getSelection().getRangeAt( 0 ); - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { expect( balloon.visibleView ).to.equal( formView ); @@ -128,7 +128,7 @@ describe( 'Link', () => { setModelData( editor.document, '<$text linkHref="url">f[]oo' ); const linkElement = editorElement.querySelector( 'a' ); - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { expect( balloon.visibleView ).to.equal( formView ); @@ -145,7 +145,7 @@ describe( 'Link', () => { it( 'should not focus the #formView at default', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { sinon.assert.notCalled( spy ); } ); @@ -154,7 +154,7 @@ describe( 'Link', () => { it( 'should not focus the #formView when called with a `false` parameter', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - return linkFeature.showPanel( false ) + return linkFeature._showPanel( false ) .then( () => { sinon.assert.notCalled( spy ); } ); @@ -168,9 +168,9 @@ describe( 'Link', () => { destroy: () => {} }; - return linkFeature.showPanel( false ) + return linkFeature._showPanel( false ) .then( () => balloon.add( { view: viewMock } ) ) - .then( () => linkFeature.showPanel( true ) ) + .then( () => linkFeature._showPanel( true ) ) .then( () => { sinon.assert.notCalled( spy ); } ); @@ -179,7 +179,7 @@ describe( 'Link', () => { it( 'should focus the #formView when called with a `true` parameter', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - return linkFeature.showPanel( true ) + return linkFeature._showPanel( true ) .then( () => { sinon.assert.calledOnce( spy ); } ); @@ -188,8 +188,8 @@ describe( 'Link', () => { it( 'should focus the #formView when called with a `true` parameter while the balloon is open and the #formView is visible', () => { const spy = testUtils.sinon.spy( formView.urlInputView, 'select' ); - return linkFeature.showPanel( false ) - .then( () => linkFeature.showPanel( true ) ) + return linkFeature._showPanel( false ) + .then( () => linkFeature._showPanel( true ) ) .then( () => { sinon.assert.calledOnce( spy ); } ); @@ -199,7 +199,7 @@ describe( 'Link', () => { editor.ui.focusTracker.isFocused = false; // Open the #_balloon with the link inside. - return linkFeature.showPanel( true ) + return linkFeature._showPanel( true ) .then( () => { // Check if editor ui is focused. expect( editor.ui.focusTracker.isFocused ).to.true; @@ -207,42 +207,42 @@ describe( 'Link', () => { } ); } ); - describe( 'hidePanel()', () => { + describe( '_hidePanel()', () => { beforeEach( () => { return balloon.add( { view: formView } ); } ); it( 'should remove #formView from the #_balloon', () => { - linkFeature.hidePanel(); + linkFeature._hidePanel(); expect( balloon.hasView( formView ) ).to.false; } ); it( 'should not focus the `editable` by default', () => { const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); - linkFeature.hidePanel(); + linkFeature._hidePanel(); sinon.assert.notCalled( spy ); } ); it( 'should not focus the `editable` when called with a `false` parameter', () => { const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); - linkFeature.hidePanel( false ); + linkFeature._hidePanel( false ); sinon.assert.notCalled( spy ); } ); it( 'should focus the `editable` when called with a `true` parameter', () => { const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); - linkFeature.hidePanel( true ); + linkFeature._hidePanel( true ); sinon.assert.calledOnce( spy ); } ); it( 'should not throw an error when #formView is not added to the `balloon`', () => { - linkFeature.hidePanel( true ); + linkFeature._hidePanel( true ); expect( () => { - linkFeature.hidePanel( true ); + linkFeature._hidePanel( true ); } ).to.not.throw(); } ); @@ -251,7 +251,7 @@ describe( 'Link', () => { linkFeature.listenTo( editor.editing.view, 'render', spy ); - linkFeature.hidePanel(); + linkFeature._hidePanel(); editor.editing.view.render(); @@ -276,7 +276,7 @@ describe( 'Link', () => { it( 'should show the #_balloon on execute event with the selected #formView', () => { // Method is stubbed because it returns internal promise which can't be returned in test. - const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + const spy = testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); linkButton.fire( 'execute' ); @@ -312,7 +312,7 @@ describe( 'Link', () => { describe( 'keyboard support', () => { it( 'should show the #_balloon with selected #formView on `CTRL+K` keystroke', () => { // Method is stubbed because it returns internal promise which can't be returned in test. - const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + const spy = testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); editor.keystrokes.press( { keyCode: keyCodes.k, ctrlKey: true } ); @@ -337,7 +337,7 @@ describe( 'Link', () => { sinon.assert.notCalled( spy ); // Balloon is visible, form focused. - return linkFeature.showPanel( true ) + return linkFeature._showPanel( true ) .then( () => { formView.focusTracker.isFocused = true; @@ -357,7 +357,7 @@ describe( 'Link', () => { } ); it( 'should hide the #_balloon after Esc key press (from editor) and not focus the editable', () => { - const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const spy = testUtils.sinon.spy( linkFeature, '_hidePanel' ); const keyEvtData = { keyCode: keyCodes.esc, preventDefault: sinon.spy(), @@ -365,7 +365,7 @@ describe( 'Link', () => { }; // Balloon is visible. - return linkFeature.showPanel( false ).then( () => { + return linkFeature._showPanel( false ).then( () => { editor.keystrokes.press( keyEvtData ); sinon.assert.calledWithExactly( spy ); @@ -373,7 +373,7 @@ describe( 'Link', () => { } ); it( 'should not hide #_balloon after Esc key press (from editor) when #_balloon is open but is not visible', () => { - const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const spy = testUtils.sinon.spy( linkFeature, '_hidePanel' ); const keyEvtData = { keyCode: keyCodes.esc, preventDefault: () => {}, @@ -386,7 +386,7 @@ describe( 'Link', () => { destroy: () => {} }; - return linkFeature.showPanel( false ) + return linkFeature._showPanel( false ) .then( () => balloon.add( { view: viewMock } ) ) .then( () => { editor.keystrokes.press( keyEvtData ); @@ -396,14 +396,14 @@ describe( 'Link', () => { } ); it( 'should hide the #_balloon after Esc key press (from the form) and focus the editable', () => { - const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const spy = testUtils.sinon.spy( linkFeature, '_hidePanel' ); const keyEvtData = { keyCode: keyCodes.esc, preventDefault: sinon.spy(), stopPropagation: sinon.spy() }; - return linkFeature.showPanel( true ) + return linkFeature._showPanel( true ) .then( () => { formView.keystrokes.press( keyEvtData ); @@ -414,9 +414,9 @@ describe( 'Link', () => { describe( 'mouse support', () => { it( 'should hide #_balloon and not focus editable on click outside the #_balloon', () => { - const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const spy = testUtils.sinon.spy( linkFeature, '_hidePanel' ); - return linkFeature.showPanel( true ) + return linkFeature._showPanel( true ) .then( () => { document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) ); @@ -425,9 +425,9 @@ describe( 'Link', () => { } ); it( 'should not hide #_balloon on click inside the #_balloon', () => { - const spy = testUtils.sinon.spy( linkFeature, 'hidePanel' ); + const spy = testUtils.sinon.spy( linkFeature, '_hidePanel' ); - return linkFeature.showPanel( true ) + return linkFeature._showPanel( true ) .then( () => { balloon.view.element.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) ); @@ -444,7 +444,7 @@ describe( 'Link', () => { it( 'should open with not selected formView when collapsed selection is inside link element', () => { // Method is stubbed because it returns internal promise which can't be returned in test. - const spy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + const spy = testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">fo[]o' ); @@ -456,8 +456,8 @@ describe( 'Link', () => { it( 'should keep open and update position until collapsed selection stay inside the same link element', () => { // Method is stubbed because it returns internal promise which can't be returned in test. - const showSpy = testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); - const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + const showSpy = testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); + const hideSpy = testUtils.sinon.stub( linkFeature, '_hidePanel' ); const updatePositionSpy = testUtils.sinon.stub( balloon, 'updatePosition', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); @@ -485,7 +485,7 @@ describe( 'Link', () => { const updatePositionSpy = testUtils.sinon.stub( balloon, 'updatePosition', () => {} ); // Method is stubbed because it returns internal promise which can't be returned in test. - testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">b[]ar' ); @@ -509,10 +509,10 @@ describe( 'Link', () => { } ); it( 'should close when selection goes outside the link element', () => { - const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + const hideSpy = testUtils.sinon.stub( linkFeature, '_hidePanel' ); // Method is stubbed because it returns internal promise which can't be returned in test. - testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, 'foo <$text linkHref="url">b[]ar' ); @@ -531,10 +531,10 @@ describe( 'Link', () => { } ); it( 'should close when selection goes to the other link element with the same href', () => { - const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + const hideSpy = testUtils.sinon.stub( linkFeature, '_hidePanel' ); // Method is stubbed because it returns internal promise which can't be returned in test. - testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">f[]oo bar <$text linkHref="url">biz' ); @@ -553,10 +553,10 @@ describe( 'Link', () => { } ); it( 'should close when selection becomes non-collapsed', () => { - const hideSpy = testUtils.sinon.stub( linkFeature, 'hidePanel' ); + const hideSpy = testUtils.sinon.stub( linkFeature, '_hidePanel' ); // Method is stubbed because it returns internal promise which can't be returned in test. - testUtils.sinon.stub( linkFeature, 'showPanel', () => {} ); + testUtils.sinon.stub( linkFeature, '_showPanel', () => {} ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">f[]oo' ); @@ -573,7 +573,7 @@ describe( 'Link', () => { } ); it( 'should not open when selection is not inside link element', () => { - const showSpy = testUtils.sinon.stub( linkFeature, 'showPanel' ); + const showSpy = testUtils.sinon.stub( linkFeature, '_showPanel' ); setModelData( editor.document, '[]' ); @@ -583,7 +583,7 @@ describe( 'Link', () => { } ); it( 'should not open when selection is non-collapsed', () => { - const showSpy = testUtils.sinon.stub( linkFeature, 'showPanel' ); + const showSpy = testUtils.sinon.stub( linkFeature, '_showPanel' ); editor.document.schema.allow( { name: '$text', inside: '$root' } ); setModelData( editor.document, '<$text linkHref="url">f[o]o' ); @@ -626,7 +626,7 @@ describe( 'Link', () => { } ); it( 'should hide and focus editable on formView#submit event', () => { - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { formView.fire( 'submit' ); @@ -645,7 +645,7 @@ describe( 'Link', () => { } ); it( 'should hide and focus editable on formView#unlink event', () => { - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { formView.fire( 'unlink' ); @@ -655,7 +655,7 @@ describe( 'Link', () => { } ); it( 'should hide and focus editable on formView#cancel event', () => { - return linkFeature.showPanel() + return linkFeature._showPanel() .then( () => { formView.fire( 'cancel' ); From fd4a0e74f34147886192603b0f3d482cc41ab806 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 14 Apr 2017 13:47:40 +0200 Subject: [PATCH 6/8] Docs: Improved docs in the Link Plugin. --- src/link.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/link.js b/src/link.js index 336d94d..9c910be 100644 --- a/src/link.js +++ b/src/link.js @@ -241,7 +241,7 @@ export default class Link extends Plugin { * * @protected * @param {Boolean} [focusInput=false] When `true`, link form will be focused on panel show. - * @return {Promise} A promise resolved when the {#formView} {@link module:ui/view~View#init} is done. + * @return {Promise} A promise resolved when the {@link #formView} {@link module:ui/view~View#init} is done. */ _showPanel( focusInput ) { if ( this._balloon.hasView( this.formView ) ) { @@ -266,6 +266,8 @@ export default class Link extends Plugin { /** * Removes the {@link #formView} from the {@link #_balloon}. * + * See {@link #_showPanel}. + * * @protected * @param {Boolean} [focusEditable=false] When `true`, editable focus will be restored on panel hide. */ From a1307b2089de0d37aa988dc3f7d366cb29b7348a Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 14 Apr 2017 13:55:55 +0200 Subject: [PATCH 7/8] Tests: Clean up DOM after test execution. --- tests/link.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/link.js b/tests/link.js index b6809c0..1de8710 100644 --- a/tests/link.js +++ b/tests/link.js @@ -51,6 +51,8 @@ describe( 'Link', () => { } ); afterEach( () => { + editorElement.remove(); + return editor.destroy(); } ); From 4097be772ee232d873ba85916d1639d04f6cf7a0 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 14 Apr 2017 14:27:19 +0200 Subject: [PATCH 8/8] Tests: Removed obsolete Link Plugin test (already tested by ContextualBalloon). --- tests/link.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/link.js b/tests/link.js index 1de8710..068d415 100644 --- a/tests/link.js +++ b/tests/link.js @@ -196,17 +196,6 @@ describe( 'Link', () => { sinon.assert.calledOnce( spy ); } ); } ); - - it( 'should keep the editor ui focused when the #_balloon is shown with the selected form', () => { - editor.ui.focusTracker.isFocused = false; - - // Open the #_balloon with the link inside. - return linkFeature._showPanel( true ) - .then( () => { - // Check if editor ui is focused. - expect( editor.ui.focusTracker.isFocused ).to.true; - } ); - } ); } ); describe( '_hidePanel()', () => {