Skip to content

Commit

Permalink
Code refactoring and misc tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
oleq committed Oct 2, 2024
1 parent 78cb1bb commit 1281530
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@ export default class DropdownMenuNestedMenuView extends View implements Focusabl
this.listView = new DropdownMenuListView( locale );
this.listView.bind( 'ariaLabel' ).to( this.buttonView, 'label' );

this.focusTracker.add( this.listView );

this.panelView.content.add( this.listView );

const bind = this.bindTemplate;
Expand Down Expand Up @@ -212,6 +210,7 @@ export default class DropdownMenuNestedMenuView extends View implements Focusabl

this.focusTracker.add( this.buttonView.element! );
this.focusTracker.add( this.panelView.element! );
this.focusTracker.add( this.listView );

// Listen for keystrokes coming from within #element.
this.keystrokes.listenTo( this.element! );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DropdownRootMenuBehaviors } from './dropdownmenubehaviors.js';
import type BodyCollection from '../../editorui/bodycollection.js';
import type { DropdownMenuDefinition } from './utils.js';

import { type Locale, type BaseEvent, type FocusTracker } from '@ckeditor/ckeditor5-utils';
import { type Locale, type BaseEvent } from '@ckeditor/ckeditor5-utils';

/**
* Creates and manages a multi-level menu UI structure, suitable to be used inside dropdown components.
Expand Down Expand Up @@ -85,8 +85,6 @@ export default class DropdownMenuRootListView extends DropdownMenuListView {
*/
declare public menuPanelClass: string | undefined;

declare public focusTracker: FocusTracker;

/**
* The definitions object used to create the whole menu structure.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ describe( 'DropdownMenuNestedMenuView', () => {
sinon.assert.calledWithExactly( focusTrackerAddSpy.secondCall, menuView.panelView.element );
} );

// https://github.com/cksource/ckeditor5-commercial/issues/6633
it( 'should add the #listView to the focus tracker to allow for linking focus trackers and sharing state of nested menus', () => {
const focusTrackerAddSpy = sinon.spy( menuView.focusTracker, 'add' );

menuView.render();

sinon.assert.calledWithExactly( focusTrackerAddSpy.thirdCall, menuView.listView );
} );

it( 'should start listening to keystrokes', () => {
const keystrokeHandlerAddSpy = sinon.spy( menuView.keystrokes, 'listenTo' );

Expand Down
10 changes: 10 additions & 0 deletions packages/ckeditor5-ui/tests/dropdown/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,16 @@ describe( 'utils', () => {
expect( dropdownView.menuView.render.calledOnce ).to.be.true;
} );

it( 'should add the menu view to dropdown\'s focus tracker to allow for linking focus trackers and keeping track of the focus ' +
'when it goes to sub-menus in other DOM sub-trees',
() => {
const addSpy = sinon.spy( dropdownView.focusTracker, 'add' );

addMenuToDropdown( dropdownView, body, definition );

sinon.assert.calledWithExactly( addSpy, dropdownView.menuView );
} );

it( 'should focus dropdown menu view after dropdown is opened', () => {
addMenuToDropdown( dropdownView, body, definition );

Expand Down
6 changes: 3 additions & 3 deletions packages/ckeditor5-ui/tests/editorui/editorui.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,13 +530,13 @@ describe( 'EditorUI', () => {
} );

describe( 'for a ToolbarView that has already been rendered', () => {
it( 'adds ToolbarView#element to the EditorUI#focusTracker', () => {
it( 'adds ToolbarView to the EditorUI#focusTracker', () => {
const spy = testUtils.sinon.spy( ui.focusTracker, 'add' );
toolbar.render();

ui.addToolbar( toolbar );

sinon.assert.calledOnce( spy );
sinon.assert.calledOnceWithExactly( spy, toolbar );
} );

it( 'adds ToolbarView#element to Editor#keystokeHandler', () => {
Expand All @@ -559,7 +559,7 @@ describe( 'EditorUI', () => {
await new Promise( resolve => {
toolbar.once( 'render', () => {
sinon.assert.calledOnce( spy );
sinon.assert.calledOnce( spy2 );
sinon.assert.calledOnceWithExactly( spy2, toolbar );

resolve();
} );
Expand Down
25 changes: 16 additions & 9 deletions packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,13 @@ describe( 'BalloonToolbar', () => {
expect( balloonToolbar.focusTracker.isFocused ).to.true;
} );

// https://github.com/cksource/ckeditor5-commercial/issues/6633
it( 'should track the ToolbarView instance (not just its element) to allow using complex toolbar items scattered across DOM ' +
'sub-trees and keep track of the focus',
() => {
expect( balloonToolbar.focusTracker.externalViews ).to.include( balloonToolbar.toolbarView );
} );

it( 'it should track the focus of the toolbarView#element', () => {
expect( balloonToolbar.focusTracker.isFocused ).to.false;

Expand Down Expand Up @@ -898,12 +905,12 @@ describe( 'BalloonToolbar', () => {
const clock = sinon.useFakeTimers();

expect( balloonToolbar.focusTracker.isFocused ).to.false;
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 3 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );

editor.addRoot( 'dynamicRoot' );

// Check if newly added editable is tracked in focus tracker.
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 5 );

// Check if element is added to focus tracker.
const editableElement = editor.ui.getEditableElement( 'dynamicRoot' );
Expand All @@ -927,18 +934,18 @@ describe( 'BalloonToolbar', () => {
const clock = sinon.useFakeTimers();

expect( balloonToolbar.focusTracker.isFocused ).to.false;
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 3 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );

editor.addRoot( 'dynamicRoot' );
const editableElement = editor.ui.getEditableElement( 'dynamicRoot' );

// Check if newly added editable is tracked in focus tracker.
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 5 );

editor.detachRoot( 'dynamicRoot' );

// Check if element is removed from focus tracker.
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 3 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );

// Focus is no longer tracked.
editableElement.focus();
Expand All @@ -955,19 +962,19 @@ describe( 'BalloonToolbar', () => {
addEditableOnRootAdd = false;

expect( balloonToolbar.focusTracker.isFocused ).to.false;
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 3 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );

editor.addRoot( 'dynamicRoot' );
const root = editor.model.document.getRoot( 'dynamicRoot' );

// Editable is not yet attached
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 3 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );

// Focus is no longer tracked.
const editableElement = editor.createEditable( root );

global.document.body.appendChild( editableElement );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 5 );

// Lets test focus
editableElement.focus();
Expand All @@ -977,7 +984,7 @@ describe( 'BalloonToolbar', () => {

// Detach editable element
editor.detachEditable( root );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 3 );
expect( balloonToolbar.focusTracker.elements.length ).to.be.equal( 4 );

editableElement.remove();
clock.restore();
Expand Down
18 changes: 13 additions & 5 deletions packages/ckeditor5-ui/tests/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,28 +256,36 @@ describe( 'ToolbarView', () => {

view.render();

sinon.assert.calledOnce( spyAdd );
sinon.assert.calledOnceWithExactly( spyAdd, view.element );
sinon.assert.notCalled( spyRemove );

view.destroy();
} );

it( 'registers #items in #focusTracker', () => {
// https://github.com/cksource/ckeditor5-commercial/issues/6633
it( 'registers #items in #focusTracker as View instances (not just DOM elements) to alow for complex Views scattered across ' +
'multiple DOM sub-trees',
() => {
const view = new ToolbarView( locale );
const spyAdd = sinon.spy( view.focusTracker, 'add' );
const spyRemove = sinon.spy( view.focusTracker, 'remove' );

view.items.add( focusable() );
view.items.add( focusable() );
const focusableViewA = focusable();
const focusableViewB = focusable();

view.items.add( focusableViewA );
view.items.add( focusableViewB );
sinon.assert.notCalled( spyAdd );

view.render();

// 2 for items and 1 for toolbar itself.
sinon.assert.calledThrice( spyAdd );
sinon.assert.calledWithExactly( spyAdd.secondCall, focusableViewA );
sinon.assert.calledWithExactly( spyAdd.thirdCall, focusableViewB );

view.items.remove( 1 );
sinon.assert.calledOnce( spyRemove );
sinon.assert.calledOnceWithExactly( spyRemove, focusableViewB );

view.destroy();
} );
Expand Down

0 comments on commit 1281530

Please sign in to comment.