diff --git a/packages/ckeditor5-ui/.travis.yml b/packages/ckeditor5-ui/.travis.yml index d839093ade1..a8cc5428bd4 100644 --- a/packages/ckeditor5-ui/.travis.yml +++ b/packages/ckeditor5-ui/.travis.yml @@ -7,7 +7,7 @@ language: node_js services: - xvfb node_js: -- '8' +- '10' cache: yarn: true branches: diff --git a/packages/ckeditor5-ui/src/dropdown/dropdownview.js b/packages/ckeditor5-ui/src/dropdown/dropdownview.js index f651e02cd82..8c836244826 100644 --- a/packages/ckeditor5-ui/src/dropdown/dropdownview.js +++ b/packages/ckeditor5-ui/src/dropdown/dropdownview.js @@ -8,7 +8,6 @@ */ import View from '../view'; -import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; import '../../theme/components/dropdown/dropdown.css'; @@ -155,14 +154,6 @@ export default class DropdownView extends View { */ this.set( 'panelPosition', 'auto' ); - /** - * Tracks information about DOM focus in the dropdown. - * - * @readonly - * @member {module:utils/focustracker~FocusTracker} - */ - this.focusTracker = new FocusTracker(); - /** * Instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}. It manages * keystrokes of the dropdown: @@ -277,9 +268,6 @@ export default class DropdownView extends View { // Listen for keystrokes coming from within #element. this.keystrokes.listenTo( this.element ); - // Register #element in the focus tracker. - this.focusTracker.add( this.element ); - const closeDropdown = ( data, cancel ) => { if ( this.isOpen ) { this.buttonView.focus(); diff --git a/packages/ckeditor5-ui/src/editorui/bodycollection.js b/packages/ckeditor5-ui/src/editorui/bodycollection.js index 8e4c218f28c..8e7654f09b3 100644 --- a/packages/ckeditor5-ui/src/editorui/bodycollection.js +++ b/packages/ckeditor5-ui/src/editorui/bodycollection.js @@ -33,6 +33,24 @@ import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; * @extends module:ui/viewcollection~ViewCollection */ export default class BodyCollection extends ViewCollection { + /** + * Creates a new instance of the {@link module:ui/editorui/bodycollection~BodyCollection}. + * + * @param {module:utils/locale~Locale} locale The {@link module:core/editor/editor~Editor editor's locale} instance. + * @param {Iterable.} [initialItems] The initial items of the collection. + */ + constructor( locale, initialItems = [] ) { + super( initialItems ); + + /** + * The {@link module:core/editor/editor~Editor#locale editor's locale} instance. + * See the view {@link module:ui/view~View#locale locale} property. + * + * @member {module:utils/locale~Locale} + */ + this.locale = locale; + } + /** * Attaches the body collection to the DOM body element. You need to execute this method to render the content of * the body collection. diff --git a/packages/ckeditor5-ui/src/view.js b/packages/ckeditor5-ui/src/view.js index ae1c3f40b0c..6a55ad8e68d 100644 --- a/packages/ckeditor5-ui/src/view.js +++ b/packages/ckeditor5-ui/src/view.js @@ -253,7 +253,8 @@ export default class View { * constructor( locale ) { * super( locale ); * - * this.items = this.createCollection(); + * const child = new ChildView( locale ); + * this.items = this.createCollection( [ child ] ); * * this.setTemplate( { * tag: 'p', @@ -265,21 +266,16 @@ export default class View { * } * * const view = new SampleView( locale ); - * const child = new ChildView( locale ); - * * view.render(); * - * // It will append

to the . + * // It will append

to the . * document.body.appendChild( view.element ); * - * // From now on the child is nested under its parent, which is also reflected in DOM. - * //

- * view.items.add( child ); - * + * @param {Iterable.} [views] Initial views of the collection. * @returns {module:ui/viewcollection~ViewCollection} A new collection of view instances. */ - createCollection() { - const collection = new ViewCollection(); + createCollection( views ) { + const collection = new ViewCollection( views ); this._viewCollections.add( collection ); diff --git a/packages/ckeditor5-ui/src/viewcollection.js b/packages/ckeditor5-ui/src/viewcollection.js index fea06ed81be..10341f8e803 100644 --- a/packages/ckeditor5-ui/src/viewcollection.js +++ b/packages/ckeditor5-ui/src/viewcollection.js @@ -51,10 +51,10 @@ export default class ViewCollection extends Collection { /** * Creates a new instance of the {@link module:ui/viewcollection~ViewCollection}. * - * @param {module:utils/locale~Locale} [locale] The {@link module:core/editor/editor~Editor editor's locale} instance. + * @param {Iterable.} [initialItems] The initial items of the collection. */ - constructor( locale ) { - super( { + constructor( initialItems = [] ) { + super( initialItems, { // An #id Number attribute should be legal and not break the `ViewCollection` instance. // https://github.com/ckeditor/ckeditor5-ui/issues/93 idProperty: 'viewUid' @@ -62,13 +62,7 @@ export default class ViewCollection extends Collection { // Handle {@link module:ui/view~View#element} in DOM when a new view is added to the collection. this.on( 'add', ( evt, view, index ) => { - if ( !view.isRendered ) { - view.render(); - } - - if ( view.element && this._parentElement ) { - this._parentElement.insertBefore( view.element, this._parentElement.children[ index ] ); - } + this._renderViewIntoCollectionParent( view, index ); } ); // Handle {@link module:ui/view~View#element} in DOM when a view is removed from the collection. @@ -78,14 +72,6 @@ export default class ViewCollection extends Collection { } } ); - /** - * The {@link module:core/editor/editor~Editor#locale editor's locale} instance. - * See the view {@link module:ui/view~View#locale locale} property. - * - * @member {module:utils/locale~Locale} - */ - this.locale = locale; - /** * A parent element within which child views are rendered and managed in DOM. * @@ -112,6 +98,11 @@ export default class ViewCollection extends Collection { */ setParent( elementOrDocFragment ) { this._parentElement = elementOrDocFragment; + + // Take care of the initial collection items passed to the constructor. + for ( const view of this ) { + this._renderViewIntoCollectionParent( view ); + } } /** @@ -194,6 +185,30 @@ export default class ViewCollection extends Collection { }; } + /** + * This method {@link module:ui/view~View#render renders} a new view added to the collection. + * + * If the {@link #_parentElement parent element} of the collection is set, this method also adds + * the view's {@link module:ui/view~View#element} as a child of the parent in DOM at a specified index. + * + * **Note**: If index is not specified, the view's element is pushed as the last child + * of the parent element. + * + * @private + * @param {module:ui/view~View} view A new view added to the collection. + * @param {Number} [index] An index the view holds in the collection. When not specified, + * the view is added at the end. + */ + _renderViewIntoCollectionParent( view, index ) { + if ( !view.isRendered ) { + view.render(); + } + + if ( view.element && this._parentElement ) { + this._parentElement.insertBefore( view.element, this._parentElement.children[ index ] ); + } + } + /** * Removes a child view from the collection. If the {@link #setParent parent element} of the * collection has been set, the {@link module:ui/view~View#element element} of the view is also removed diff --git a/packages/ckeditor5-ui/tests/dropdown/dropdownview.js b/packages/ckeditor5-ui/tests/dropdown/dropdownview.js index d06114bb6b5..15429ff62dc 100644 --- a/packages/ckeditor5-ui/tests/dropdown/dropdownview.js +++ b/packages/ckeditor5-ui/tests/dropdown/dropdownview.js @@ -5,7 +5,6 @@ import DropdownView from '../../src/dropdown/dropdownview'; import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; -import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import ButtonView from '../../src/button/buttonview'; import DropdownPanelView from '../../src/dropdown/dropdownpanelview'; @@ -72,10 +71,6 @@ describe( 'DropdownView', () => { expect( view.panelPosition ).to.equal( 'auto' ); } ); - it( 'creates #focusTracker instance', () => { - expect( view.focusTracker ).to.be.instanceOf( FocusTracker ); - } ); - it( 'creates #keystrokeHandler instance', () => { expect( view.keystrokes ).to.be.instanceOf( KeystrokeHandler ); } ); @@ -214,20 +209,6 @@ describe( 'DropdownView', () => { view.element.remove(); } ); - it( 'adds #element to #focusTracker', () => { - const view = new DropdownView( locale, - new ButtonView( locale ), - new DropdownPanelView( locale ) ); - - const spy = sinon.spy( view.focusTracker, 'add' ); - - view.render(); - sinon.assert.calledOnce( spy ); - sinon.assert.calledWithExactly( spy, view.element ); - - view.element.remove(); - } ); - describe( 'activates keyboard navigation for the dropdown', () => { it( 'so "arrowdown" opens the #panelView', () => { const keyEvtData = { diff --git a/packages/ckeditor5-ui/tests/editorui/bodycollection.js b/packages/ckeditor5-ui/tests/editorui/bodycollection.js index cca259bbca2..304042b1414 100644 --- a/packages/ckeditor5-ui/tests/editorui/bodycollection.js +++ b/packages/ckeditor5-ui/tests/editorui/bodycollection.js @@ -28,6 +28,23 @@ describe( 'BodyCollection', () => { } } ); + describe( 'constructor', () => { + it( 'assigns locale', () => { + const instance = new BodyCollection( locale ); + + expect( instance.locale ).to.be.equal( locale ); + } ); + + it( 'stores pre-initialized collection', () => { + const collectionItems = [ new View(), new View() ]; + const instance = new BodyCollection( locale, collectionItems ); + + expect( instance ).to.have.lengthOf( 2 ); + expect( instance.get( 0 ) ).to.be.equal( collectionItems[ 0 ] ); + expect( instance.get( 1 ) ).to.be.equal( collectionItems[ 1 ] ); + } ); + } ); + describe( 'attachToDom', () => { it( 'should create wrapper and put the collection in that wrapper', () => { const body = new BodyCollection( locale ); diff --git a/packages/ckeditor5-ui/tests/focuscycler.js b/packages/ckeditor5-ui/tests/focuscycler.js index c6bb50ff182..341f41e4b8d 100644 --- a/packages/ckeditor5-ui/tests/focuscycler.js +++ b/packages/ckeditor5-ui/tests/focuscycler.js @@ -17,7 +17,14 @@ describe( 'FocusCycler', () => { testUtils.createSinonSandbox(); beforeEach( () => { - focusables = new ViewCollection(); + testUtils.sinon.stub( global.window, 'getComputedStyle' ); + focusables = new ViewCollection( [ + nonFocusable(), + focusable(), + focusable(), + focusable(), + nonFocusable() + ] ); focusTracker = { focusedElement: null }; @@ -25,14 +32,6 @@ describe( 'FocusCycler', () => { focusables, focusTracker } ); - - testUtils.sinon.stub( global.window, 'getComputedStyle' ); - - focusables.add( nonFocusable() ); - focusables.add( focusable() ); - focusables.add( focusable() ); - focusables.add( focusable() ); - focusables.add( nonFocusable() ); } ); describe( 'constructor()', () => { @@ -60,12 +59,9 @@ describe( 'FocusCycler', () => { } ); it( 'returns null when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( cycler.first ).to.be.null; } ); @@ -83,12 +79,9 @@ describe( 'FocusCycler', () => { } ); it( 'returns null when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( cycler.last ).to.be.null; } ); @@ -126,23 +119,16 @@ describe( 'FocusCycler', () => { } ); it( 'returns null when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( cycler.next ).to.be.null; } ); it( 'returns null if the only focusable in focusables', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), focusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( focusable() ); - focusables.add( nonFocusable() ); - focusTracker.focusedElement = focusables.get( 1 ).element; expect( cycler.first ).to.equal( focusables.get( 1 ) ); @@ -176,23 +162,16 @@ describe( 'FocusCycler', () => { } ); it( 'returns null when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( cycler.previous ).to.be.null; } ); it( 'returns null if the only focusable in focusables', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), focusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( focusable() ); - focusables.add( nonFocusable() ); - focusTracker.focusedElement = focusables.get( 1 ).element; expect( cycler.first ).to.equal( focusables.get( 1 ) ); @@ -208,12 +187,9 @@ describe( 'FocusCycler', () => { } ); it( 'does not throw when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( () => { cycler.focusFirst(); } ).to.not.throw(); @@ -231,11 +207,7 @@ describe( 'FocusCycler', () => { it( 'ignores invisible items', () => { const item = focusable(); - focusables = new ViewCollection(); - focusables.add( nonFocusable() ); - focusables.add( focusable( true ) ); - focusables.add( item ); - + focusables = new ViewCollection( [ nonFocusable(), focusable( true ), item ] ); cycler = new FocusCycler( { focusables, focusTracker } ); cycler.focusFirst(); @@ -251,12 +223,9 @@ describe( 'FocusCycler', () => { } ); it( 'does not throw when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( () => { cycler.focusLast(); } ).to.not.throw(); @@ -281,12 +250,9 @@ describe( 'FocusCycler', () => { } ); it( 'does not throw when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( () => { cycler.focusNext(); } ).to.not.throw(); @@ -311,12 +277,9 @@ describe( 'FocusCycler', () => { } ); it( 'does not throw when no focusable items', () => { - focusables = new ViewCollection(); + focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] ); cycler = new FocusCycler( { focusables, focusTracker } ); - focusables.add( nonFocusable() ); - focusables.add( nonFocusable() ); - expect( () => { cycler.focusPrevious(); } ).to.not.throw(); diff --git a/packages/ckeditor5-ui/tests/view.js b/packages/ckeditor5-ui/tests/view.js index e99bbc50d31..226cd4c4611 100644 --- a/packages/ckeditor5-ui/tests/view.js +++ b/packages/ckeditor5-ui/tests/view.js @@ -89,6 +89,17 @@ describe( 'View', () => { expect( view._viewCollections ).to.have.length( 2 ); expect( view._viewCollections.get( 1 ) ).to.equal( collection ); } ); + + it( 'accepts initial views', () => { + const viewA = new View(); + const viewB = new View(); + + const collection = view.createCollection( [ viewA, viewB ] ); + + expect( collection ).to.have.length( 2 ); + expect( collection.get( 0 ) ).to.equal( viewA ); + expect( collection.get( 1 ) ).to.equal( viewB ); + } ); } ); describe( 'registerChild()', () => { diff --git a/packages/ckeditor5-ui/tests/viewcollection.js b/packages/ckeditor5-ui/tests/viewcollection.js index f366a90c4da..58191f2bd9a 100644 --- a/packages/ckeditor5-ui/tests/viewcollection.js +++ b/packages/ckeditor5-ui/tests/viewcollection.js @@ -20,15 +20,18 @@ describe( 'ViewCollection', () => { describe( 'constructor()', () => { it( 'sets basic properties and attributes', () => { - expect( collection.locale ).to.be.undefined; expect( collection._parentElement ).to.be.null; expect( collection._idProperty ).to.equal( 'viewUid' ); } ); - it( 'accepts locale and defines the locale property', () => { - const locale = { t() {} }; + it( 'allows setting initial collection items', () => { + const view1 = new View(); + const view2 = new View(); + const collection = new ViewCollection( [ view1, view2 ] ); - expect( new ViewCollection( locale ).locale ).to.equal( locale ); + expect( collection ).to.have.length( 2 ); + expect( collection.get( 0 ) ).to.equal( view1 ); + expect( collection.get( 1 ) ).to.equal( view2 ); } ); describe( 'child view management in DOM', () => { @@ -165,6 +168,28 @@ describe( 'ViewCollection', () => { collection.setParent( el ); expect( collection._parentElement ).to.equal( el ); } ); + + it( 'udpates initial collection items in DOM', () => { + const view1 = new View(); + view1.element = document.createElement( 'i' ); + sinon.spy( view1, 'render' ); + + const view2 = new View(); + view2.element = document.createElement( 'b' ); + sinon.spy( view2, 'render' ); + + const collection = new ViewCollection( [ view1, view2 ] ); + const parentElement = document.createElement( 'div' ); + + expect( collection ).to.have.length( 2 ); + expect( collection.get( 0 ) ).to.equal( view1 ); + expect( collection.get( 1 ) ).to.equal( view2 ); + + collection.setParent( parentElement ); + expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '
' ); + sinon.assert.calledOnce( view1.render ); + sinon.assert.calledOnce( view2.render ); + } ); } ); describe( 'delegate()', () => {