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

Commit 6cd15de

Browse files
oleqoleq
authored
Merge pull request #545 from ckeditor/i/6319
Feature: Allowed defining initial items of `ViewCollection` and `BodyCollection` in the constructor. See ckeditor/ckeditor5#6319. The `View#createCollection()` method also started accepting an iterator of views. MAJOR BREAKING CHANGE: `ViewCollection` no longer has the `locale` property. MAJOR BREAKING CHANGE: The `ViewCollection#constructor()` no longer accepts the `locale` as a parameter.
2 parents b30bdd9 + bd93bd0 commit 6cd15de

File tree

7 files changed

+133
-88
lines changed

7 files changed

+133
-88
lines changed

‎src/editorui/bodycollection.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,24 @@ import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
3333
* @extends module:ui/viewcollection~ViewCollection
3434
*/
3535
export default class BodyCollection extends ViewCollection {
36+
/**
37+
* Creates a new instance of the {@link module:ui/editorui/bodycollection~BodyCollection}.
38+
*
39+
* @param {module:utils/locale~Locale} locale The {@link module:core/editor/editor~Editor editor's locale} instance.
40+
* @param {Iterable.<module:ui/view~View>} [initialItems] The initial items of the collection.
41+
*/
42+
constructor( locale, initialItems = [] ) {
43+
super( initialItems );
44+
45+
/**
46+
* The {@link module:core/editor/editor~Editor#locale editor's locale} instance.
47+
* See the view {@link module:ui/view~View#locale locale} property.
48+
*
49+
* @member {module:utils/locale~Locale}
50+
*/
51+
this.locale = locale;
52+
}
53+
3654
/**
3755
* Attaches the body collection to the DOM body element. You need to execute this method to render the content of
3856
* the body collection.

‎src/view.js‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,8 @@ export default class View {
253253
* constructor( locale ) {
254254
* super( locale );
255255
*
256-
* this.items = this.createCollection();
256+
* const child = new ChildView( locale );
257+
* this.items = this.createCollection( [ child ] );
257258
*
258259
* this.setTemplate( {
259260
* tag: 'p',
@@ -265,21 +266,16 @@ export default class View {
265266
* }
266267
*
267268
* const view = new SampleView( locale );
268-
* const child = new ChildView( locale );
269-
*
270269
* view.render();
271270
*
272-
* // It will append <p></p> to the <body>.
271+
* // It will append <p><child#element></p> to the <body>.
273272
* document.body.appendChild( view.element );
274273
*
275-
* // From now on the child is nested under its parent, which is also reflected in DOM.
276-
* // <p><child#element></p>
277-
* view.items.add( child );
278-
*
274+
* @param {Iterable.<module:ui/view~View>} [views] Initial views of the collection.
279275
* @returns {module:ui/viewcollection~ViewCollection} A new collection of view instances.
280276
*/
281-
createCollection() {
282-
const collection = new ViewCollection();
277+
createCollection( views ) {
278+
const collection = new ViewCollection( views );
283279

284280
this._viewCollections.add( collection );
285281

‎src/viewcollection.js‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,24 +51,18 @@ export default class ViewCollection extends Collection {
5151
/**
5252
* Creates a new instance of the {@link module:ui/viewcollection~ViewCollection}.
5353
*
54-
* @param {module:utils/locale~Locale} [locale] The {@link module:core/editor/editor~Editor editor's locale} instance.
54+
* @param {Iterable.<module:ui/view~View>} [initialItems] The initial items of the collection.
5555
*/
56-
constructor( locale ) {
57-
super( {
56+
constructor( initialItems = [] ) {
57+
super( initialItems, {
5858
// An #id Number attribute should be legal and not break the `ViewCollection` instance.
5959
// https://github.com/ckeditor/ckeditor5-ui/issues/93
6060
idProperty: 'viewUid'
6161
} );
6262

6363
// Handle {@link module:ui/view~View#element} in DOM when a new view is added to the collection.
6464
this.on( 'add', ( evt, view, index ) => {
65-
if ( !view.isRendered ) {
66-
view.render();
67-
}
68-
69-
if ( view.element && this._parentElement ) {
70-
this._parentElement.insertBefore( view.element, this._parentElement.children[ index ] );
71-
}
65+
this._renderViewIntoCollectionParent( view, index );
7266
} );
7367

7468
// 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 {
7872
}
7973
} );
8074

81-
/**
82-
* The {@link module:core/editor/editor~Editor#locale editor's locale} instance.
83-
* See the view {@link module:ui/view~View#locale locale} property.
84-
*
85-
* @member {module:utils/locale~Locale}
86-
*/
87-
this.locale = locale;
88-
8975
/**
9076
* A parent element within which child views are rendered and managed in DOM.
9177
*
@@ -112,6 +98,11 @@ export default class ViewCollection extends Collection {
11298
*/
11399
setParent( elementOrDocFragment ) {
114100
this._parentElement = elementOrDocFragment;
101+
102+
// Take care of the initial collection items passed to the constructor.
103+
for ( const view of this ) {
104+
this._renderViewIntoCollectionParent( view );
105+
}
115106
}
116107

117108
/**
@@ -194,6 +185,30 @@ export default class ViewCollection extends Collection {
194185
};
195186
}
196187

188+
/**
189+
* This method {@link module:ui/view~View#render renders} a new view added to the collection.
190+
*
191+
* If the {@link #_parentElement parent element} of the collection is set, this method also adds
192+
* the view's {@link module:ui/view~View#element} as a child of the parent in DOM at a specified index.
193+
*
194+
* **Note**: If index is not specified, the view's element is pushed as the last child
195+
* of the parent element.
196+
*
197+
* @private
198+
* @param {module:ui/view~View} view A new view added to the collection.
199+
* @param {Number} [index] An index the view holds in the collection. When not specified,
200+
* the view is added at the end.
201+
*/
202+
_renderViewIntoCollectionParent( view, index ) {
203+
if ( !view.isRendered ) {
204+
view.render();
205+
}
206+
207+
if ( view.element && this._parentElement ) {
208+
this._parentElement.insertBefore( view.element, this._parentElement.children[ index ] );
209+
}
210+
}
211+
197212
/**
198213
* Removes a child view from the collection. If the {@link #setParent parent element} of the
199214
* collection has been set, the {@link module:ui/view~View#element element} of the view is also removed

‎tests/editorui/bodycollection.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,23 @@ describe( 'BodyCollection', () => {
2828
}
2929
} );
3030

31+
describe( 'constructor', () => {
32+
it( 'assigns locale', () => {
33+
const instance = new BodyCollection( locale );
34+
35+
expect( instance.locale ).to.be.equal( locale );
36+
} );
37+
38+
it( 'stores pre-initialized collection', () => {
39+
const collectionItems = [ new View(), new View() ];
40+
const instance = new BodyCollection( locale, collectionItems );
41+
42+
expect( instance ).to.have.lengthOf( 2 );
43+
expect( instance.get( 0 ) ).to.be.equal( collectionItems[ 0 ] );
44+
expect( instance.get( 1 ) ).to.be.equal( collectionItems[ 1 ] );
45+
} );
46+
} );
47+
3148
describe( 'attachToDom', () => {
3249
it( 'should create wrapper and put the collection in that wrapper', () => {
3350
const body = new BodyCollection( locale );

‎tests/focuscycler.js‎

Lines changed: 19 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,21 @@ describe( 'FocusCycler', () => {
1717
testUtils.createSinonSandbox();
1818

1919
beforeEach( () => {
20-
focusables = new ViewCollection();
20+
testUtils.sinon.stub( global.window, 'getComputedStyle' );
21+
focusables = new ViewCollection( [
22+
nonFocusable(),
23+
focusable(),
24+
focusable(),
25+
focusable(),
26+
nonFocusable()
27+
] );
2128
focusTracker = {
2229
focusedElement: null
2330
};
2431
cycler = new FocusCycler( {
2532
focusables,
2633
focusTracker
2734
} );
28-
29-
testUtils.sinon.stub( global.window, 'getComputedStyle' );
30-
31-
focusables.add( nonFocusable() );
32-
focusables.add( focusable() );
33-
focusables.add( focusable() );
34-
focusables.add( focusable() );
35-
focusables.add( nonFocusable() );
3635
} );
3736

3837
describe( 'constructor()', () => {
@@ -60,12 +59,9 @@ describe( 'FocusCycler', () => {
6059
} );
6160

6261
it( 'returns null when no focusable items', () => {
63-
focusables = new ViewCollection();
62+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
6463
cycler = new FocusCycler( { focusables, focusTracker } );
6564

66-
focusables.add( nonFocusable() );
67-
focusables.add( nonFocusable() );
68-
6965
expect( cycler.first ).to.be.null;
7066
} );
7167

@@ -83,12 +79,9 @@ describe( 'FocusCycler', () => {
8379
} );
8480

8581
it( 'returns null when no focusable items', () => {
86-
focusables = new ViewCollection();
82+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
8783
cycler = new FocusCycler( { focusables, focusTracker } );
8884

89-
focusables.add( nonFocusable() );
90-
focusables.add( nonFocusable() );
91-
9285
expect( cycler.last ).to.be.null;
9386
} );
9487

@@ -126,23 +119,16 @@ describe( 'FocusCycler', () => {
126119
} );
127120

128121
it( 'returns null when no focusable items', () => {
129-
focusables = new ViewCollection();
122+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
130123
cycler = new FocusCycler( { focusables, focusTracker } );
131124

132-
focusables.add( nonFocusable() );
133-
focusables.add( nonFocusable() );
134-
135125
expect( cycler.next ).to.be.null;
136126
} );
137127

138128
it( 'returns null if the only focusable in focusables', () => {
139-
focusables = new ViewCollection();
129+
focusables = new ViewCollection( [ nonFocusable(), focusable(), nonFocusable() ] );
140130
cycler = new FocusCycler( { focusables, focusTracker } );
141131

142-
focusables.add( nonFocusable() );
143-
focusables.add( focusable() );
144-
focusables.add( nonFocusable() );
145-
146132
focusTracker.focusedElement = focusables.get( 1 ).element;
147133

148134
expect( cycler.first ).to.equal( focusables.get( 1 ) );
@@ -176,23 +162,16 @@ describe( 'FocusCycler', () => {
176162
} );
177163

178164
it( 'returns null when no focusable items', () => {
179-
focusables = new ViewCollection();
165+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
180166
cycler = new FocusCycler( { focusables, focusTracker } );
181167

182-
focusables.add( nonFocusable() );
183-
focusables.add( nonFocusable() );
184-
185168
expect( cycler.previous ).to.be.null;
186169
} );
187170

188171
it( 'returns null if the only focusable in focusables', () => {
189-
focusables = new ViewCollection();
172+
focusables = new ViewCollection( [ nonFocusable(), focusable(), nonFocusable() ] );
190173
cycler = new FocusCycler( { focusables, focusTracker } );
191174

192-
focusables.add( nonFocusable() );
193-
focusables.add( focusable() );
194-
focusables.add( nonFocusable() );
195-
196175
focusTracker.focusedElement = focusables.get( 1 ).element;
197176

198177
expect( cycler.first ).to.equal( focusables.get( 1 ) );
@@ -208,12 +187,9 @@ describe( 'FocusCycler', () => {
208187
} );
209188

210189
it( 'does not throw when no focusable items', () => {
211-
focusables = new ViewCollection();
190+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
212191
cycler = new FocusCycler( { focusables, focusTracker } );
213192

214-
focusables.add( nonFocusable() );
215-
focusables.add( nonFocusable() );
216-
217193
expect( () => {
218194
cycler.focusFirst();
219195
} ).to.not.throw();
@@ -231,11 +207,7 @@ describe( 'FocusCycler', () => {
231207
it( 'ignores invisible items', () => {
232208
const item = focusable();
233209

234-
focusables = new ViewCollection();
235-
focusables.add( nonFocusable() );
236-
focusables.add( focusable( true ) );
237-
focusables.add( item );
238-
210+
focusables = new ViewCollection( [ nonFocusable(), focusable( true ), item ] );
239211
cycler = new FocusCycler( { focusables, focusTracker } );
240212

241213
cycler.focusFirst();
@@ -251,12 +223,9 @@ describe( 'FocusCycler', () => {
251223
} );
252224

253225
it( 'does not throw when no focusable items', () => {
254-
focusables = new ViewCollection();
226+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
255227
cycler = new FocusCycler( { focusables, focusTracker } );
256228

257-
focusables.add( nonFocusable() );
258-
focusables.add( nonFocusable() );
259-
260229
expect( () => {
261230
cycler.focusLast();
262231
} ).to.not.throw();
@@ -281,12 +250,9 @@ describe( 'FocusCycler', () => {
281250
} );
282251

283252
it( 'does not throw when no focusable items', () => {
284-
focusables = new ViewCollection();
253+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
285254
cycler = new FocusCycler( { focusables, focusTracker } );
286255

287-
focusables.add( nonFocusable() );
288-
focusables.add( nonFocusable() );
289-
290256
expect( () => {
291257
cycler.focusNext();
292258
} ).to.not.throw();
@@ -311,12 +277,9 @@ describe( 'FocusCycler', () => {
311277
} );
312278

313279
it( 'does not throw when no focusable items', () => {
314-
focusables = new ViewCollection();
280+
focusables = new ViewCollection( [ nonFocusable(), nonFocusable() ] );
315281
cycler = new FocusCycler( { focusables, focusTracker } );
316282

317-
focusables.add( nonFocusable() );
318-
focusables.add( nonFocusable() );
319-
320283
expect( () => {
321284
cycler.focusPrevious();
322285
} ).to.not.throw();

‎tests/view.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ describe( 'View', () => {
8989
expect( view._viewCollections ).to.have.length( 2 );
9090
expect( view._viewCollections.get( 1 ) ).to.equal( collection );
9191
} );
92+
93+
it( 'accepts initial views', () => {
94+
const viewA = new View();
95+
const viewB = new View();
96+
97+
const collection = view.createCollection( [ viewA, viewB ] );
98+
99+
expect( collection ).to.have.length( 2 );
100+
expect( collection.get( 0 ) ).to.equal( viewA );
101+
expect( collection.get( 1 ) ).to.equal( viewB );
102+
} );
92103
} );
93104

94105
describe( 'registerChild()', () => {

0 commit comments

Comments
 (0)