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

Commit

Permalink
Merge de36478 into ed37ff0
Browse files Browse the repository at this point in the history
  • Loading branch information
mlewand committed Mar 11, 2020
2 parents ed37ff0 + de36478 commit 2890b6e
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 80 deletions.
18 changes: 18 additions & 0 deletions src/editorui/bodycollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.<module:ui/view~View>} [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.
Expand Down
5 changes: 3 additions & 2 deletions src/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,11 @@ export default class View {
* // <p><child#element></p>
* view.items.add( child );
*
* @param {Iterable.<module:ui/view~View>} [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 );

Expand Down
51 changes: 33 additions & 18 deletions src/viewcollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,18 @@ 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.<module:ui/view~View>} [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'
} );

// 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.
Expand All @@ -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.
*
Expand All @@ -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 );
}
}

/**
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tests/editorui/bodycollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ describe( 'BodyCollection', () => {
}
} );

describe( 'constructor', () => {
it( 'assigns locale', () => {
const instance = new BodyCollection( locale );

expect( instance.locale ).to.be.equal( locale );
} );
} );

describe( 'attachToDom', () => {
it( 'should create wrapper and put the collection in that wrapper', () => {
const body = new BodyCollection( locale );
Expand Down
75 changes: 19 additions & 56 deletions tests/focuscycler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,21 @@ 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
};
cycler = new 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()', () => {
Expand Down Expand Up @@ -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;
} );

Expand All @@ -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;
} );

Expand Down Expand Up @@ -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 ) );
Expand Down Expand Up @@ -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 ) );
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions tests/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down
33 changes: 29 additions & 4 deletions tests/viewcollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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( '<div><i></i><b></b></div>' );
sinon.assert.calledOnce( view1.render );
sinon.assert.calledOnce( view2.render );
} );
} );

describe( 'delegate()', () => {
Expand Down

0 comments on commit 2890b6e

Please sign in to comment.