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

Commit

Permalink
Used the new Collection and View API to add multiple Views at a time.
Browse files Browse the repository at this point in the history
  • Loading branch information
oleq committed Oct 18, 2019
1 parent 0081982 commit 003bd92
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 178 deletions.
3 changes: 1 addition & 2 deletions src/button/buttonview.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ export default class ButtonView extends View {
this.children.add( this.iconView );
}

this.children.add( this.tooltipView );
this.children.add( this.labelView );
this.children.add( this.tooltipView, this.labelView );
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/dropdown/button/splitbuttonview.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ export default class SplitButtonView extends View {
render() {
super.render();

this.children.add( this.actionView );
this.children.add( this.arrowView );
this.children.add( this.actionView, this.arrowView );

this.focusTracker.add( this.actionView.element );
this.focusTracker.add( this.arrowView.element );
Expand Down
2 changes: 1 addition & 1 deletion src/dropdown/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export function addToolbarToDropdown( dropdownView, buttons ) {
}
} );

buttons.map( view => toolbarView.items.add( view ) );
toolbarView.items.add( ...buttons );

dropdownView.panelView.children.add( toolbarView );
toolbarView.items.delegate( 'execute' ).to( dropdownView );
Expand Down
6 changes: 2 additions & 4 deletions src/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ export default class ToolbarView extends View {
* @readonly
* @member {module:ui/viewcollection~ViewCollection}
*/
this.children = this.createCollection();
this.children.add( this.itemsView );
this.children = this.createCollection( [ this.itemsView ] );

/**
* A collection of {@link #items} that take part in the focus cycling
Expand Down Expand Up @@ -696,8 +695,7 @@ class DynamicGrouping {
*/
_groupLastItem() {
if ( !this.groupedItems.length ) {
this.viewChildren.add( new ToolbarSeparatorView() );
this.viewChildren.add( this.groupedItemsDropdown );
this.viewChildren.add( new ToolbarSeparatorView(), this.groupedItemsDropdown );
this.viewFocusTracker.add( this.groupedItemsDropdown.element );
}

Expand Down
9 changes: 3 additions & 6 deletions tests/dropdown/dropdownpanelview.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ describe( 'DropdownPanelView', () => {

firstChildView.focus = sinon.spy();

view.children.add( firstChildView );
view.children.add( new View() );
view.children.add( firstChildView, new View() );

view.focus();

Expand All @@ -104,8 +103,7 @@ describe( 'DropdownPanelView', () => {

lastChildView.focusLast = sinon.spy();

view.children.add( new View() );
view.children.add( lastChildView );
view.children.add( new View(), lastChildView );

view.focusLast();

Expand All @@ -117,8 +115,7 @@ describe( 'DropdownPanelView', () => {

lastChildView.focus = sinon.spy();

view.children.add( new View() );
view.children.add( lastChildView );
view.children.add( new View(), lastChildView );

view.focusLast();

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
25 changes: 7 additions & 18 deletions tests/list/listview.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ describe( 'ListView', () => {
const spyRemove = sinon.spy( view.focusTracker, 'remove' );

sinon.assert.notCalled( spyAdd );
view.items.add( focusable() );
view.items.add( focusable() );
view.items.add( focusable(), focusable() );

view.render();
sinon.assert.calledTwice( spyAdd );
Expand Down Expand Up @@ -94,17 +93,14 @@ describe( 'ListView', () => {
sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );

view.items.add( nonFocusable() );
view.items.add( nonFocusable() );
view.items.add( nonFocusable(), nonFocusable() );

// No focusable children.
view.keystrokes.press( keyEvtData );
sinon.assert.calledTwice( keyEvtData.preventDefault );
sinon.assert.calledTwice( keyEvtData.stopPropagation );

view.items.add( focusable() );
view.items.add( nonFocusable() );
view.items.add( focusable() );
view.items.add( focusable(), nonFocusable(), focusable() );

// Mock the last item is focused.
view.focusTracker.isFocused = true;
Expand All @@ -130,17 +126,14 @@ describe( 'ListView', () => {
sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );

view.items.add( nonFocusable() );
view.items.add( nonFocusable() );
view.items.add( nonFocusable(), nonFocusable() );

// No focusable children.
view.keystrokes.press( keyEvtData );
sinon.assert.calledTwice( keyEvtData.preventDefault );
sinon.assert.calledTwice( keyEvtData.stopPropagation );

view.items.add( focusable() );
view.items.add( nonFocusable() );
view.items.add( focusable() );
view.items.add( focusable(), nonFocusable(), focusable() );

// Mock the last item is focused.
view.focusTracker.isFocused = true;
Expand All @@ -162,9 +155,7 @@ describe( 'ListView', () => {
view.focus();

// The second child is focusable.
view.items.add( nonFocusable() );
view.items.add( focusable() );
view.items.add( nonFocusable() );
view.items.add( nonFocusable(), focusable(), nonFocusable() );

const spy = sinon.spy( view.items.get( 1 ), 'focus' );
view.focus();
Expand All @@ -179,9 +170,7 @@ describe( 'ListView', () => {
view.focusLast();

// The second child is focusable.
view.items.add( nonFocusable() );
view.items.add( focusable() );
view.items.add( nonFocusable() );
view.items.add( nonFocusable(), focusable(), nonFocusable() );

const spy = sinon.spy( view.items.get( 1 ), 'focus' );
view.focusLast();
Expand Down
Loading

0 comments on commit 003bd92

Please sign in to comment.