Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The dropdown menu component should not cause an editor blur if used in a BalloonToolbar #17140

Closed
wants to merge 14 commits into from
3 changes: 2 additions & 1 deletion docs/_snippets/framework/ui/ui-toolbar-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
const locale = new Locale();

const text = new View();
text.element = document.createTextNode( 'Toolbar text' );
text.element = document.createElement( 'span' );
text.element.innerHTML = 'Toolbar text';

const toolbarText = new ToolbarView( locale );
toolbarText.items.add( text );
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-theme-lark/tests/manual/theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class TextView extends View {
constructor() {
super();

this.element = document.createTextNode( 'Sample text' );
this.element = document.createElement( 'span' );
this.element.innerHTML = 'Sample text';
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ 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
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, BaseEvent } from '@ckeditor/ckeditor5-utils';
import { type Locale, type BaseEvent, FocusTracker } 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,6 +85,8 @@ 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 Expand Up @@ -122,6 +124,8 @@ export default class DropdownMenuRootListView extends DropdownMenuListView {
this._bodyCollection = bodyCollection;
this._definition = definition;

this.focusTracker = new FocusTracker();

this.set( 'menuPanelClass', undefined );
}

Expand Down Expand Up @@ -152,6 +156,8 @@ export default class DropdownMenuRootListView extends DropdownMenuListView {

DropdownRootMenuBehaviors.toggleMenusAndFocusItemsOnHover( this );
DropdownRootMenuBehaviors.closeMenuWhenAnotherOnTheSameLevelOpens( this );

this.menus.forEach( menu => this.focusTracker.add( menu ) );
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-ui/src/dropdown/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export function addMenuToDropdown(
ariaLabel?: string;
} = {} ): void {
dropdownView.menuView = new DropdownMenuRootListView( dropdownView.locale!, body, definition );
dropdownView.focusTracker.add( dropdownView.menuView );

if ( dropdownView.isOpen ) {
addMenuToOpenDropdown( dropdownView, options );
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-ui/src/editorui/editorui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,11 @@ export default abstract class EditorUI extends /* #__PURE__ */ ObservableMixin()
*/
public addToolbar( toolbarView: ToolbarView, options: FocusableToolbarOptions = {} ): void {
if ( toolbarView.isRendered ) {
this.focusTracker.add( toolbarView.element! );
this.focusTracker.add( toolbarView );
this.editor.keystrokes.listenTo( toolbarView.element! );
} else {
toolbarView.once<UIViewRenderEvent>( 'render', () => {
this.focusTracker.add( toolbarView.element! );
this.focusTracker.add( toolbarView );
this.editor.keystrokes.listenTo( toolbarView.element! );
} );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export default class BalloonToolbar extends Plugin {

// Track focusable elements in the toolbar and the editable elements.
this._trackFocusableEditableElements();
this.focusTracker.add( this.toolbarView.element! );
this.focusTracker.add( this.toolbarView );

// Register the toolbar so it becomes available for Alt+F10 and Esc navigation.
editor.ui.addToolbar( this.toolbarView, {
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-ui/src/toolbar/toolbarview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ export default class ToolbarView extends View implements DropdownPanelFocusable
}

this.items.on<CollectionAddEvent<View>>( 'add', ( evt, item ) => {
this.focusTracker.add( item.element! );
this.focusTracker.add( item );
} );

this.items.on<CollectionRemoveEvent<View>>( 'remove', ( evt, item ) => {
this.focusTracker.remove( item.element! );
this.focusTracker.remove( item );
} );

// Start listening for the keystrokes coming from #element.
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-ui/tests/editorui/editorui.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ describe( 'EditorUI', () => {
} );

it( 'should reset editables array', () => {
ui.setEditableElement( 'foo', {} );
ui.setEditableElement( 'bar', {} );
ui.setEditableElement( 'foo', document.createElement( 'div' ) );
ui.setEditableElement( 'bar', document.createElement( 'div' ) );

expect( [ ...ui.getEditableElementsNames() ] ).to.deep.equal( [ 'foo', 'bar' ] );

Expand Down
37 changes: 21 additions & 16 deletions packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ describe( 'BalloonToolbar', () => {
} );

describe( 'MultiRoot editor integration', () => {
let rootsElements, addEditableOnRootAdd;
let rootsElements, addEditableOnRootAdd, focusHolder;

beforeEach( async () => {
addEditableOnRootAdd = true;
Expand All @@ -849,6 +849,9 @@ describe( 'BalloonToolbar', () => {

editor = await createMultiRootEditor();
balloonToolbar = editor.plugins.get( BalloonToolbar );

focusHolder = document.createElement( 'input' );
document.body.appendChild( focusHolder );
} );

afterEach( async () => {
Expand All @@ -858,6 +861,8 @@ describe( 'BalloonToolbar', () => {

await editor.destroy();
editor = null;

focusHolder.remove();
} );

it( 'should create plugin instance', () => {
Expand All @@ -877,11 +882,11 @@ describe( 'BalloonToolbar', () => {
for ( const editableName of editables ) {
const editableElement = editor.ui.getEditableElement( editableName );

editableElement.dispatchEvent( new Event( 'focus' ) );
editableElement.focus();
clock.tick( 50 );
expect( balloonToolbar.focusTracker.isFocused ).to.true;

editableElement.dispatchEvent( new Event( 'blur' ) );
focusHolder.focus();
clock.tick( 50 );
expect( balloonToolbar.focusTracker.isFocused ).to.false;
}
Expand All @@ -893,24 +898,24 @@ describe( 'BalloonToolbar', () => {
const clock = sinon.useFakeTimers();

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

editor.addRoot( 'dynamicRoot' );

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

// Check if element is added to focus tracker.
const editableElement = editor.ui.getEditableElement( 'dynamicRoot' );
expect( balloonToolbar.focusTracker._elements ).contain( editableElement );

// Watch focus and blur events.
editableElement.dispatchEvent( new Event( 'focus' ) );
editableElement.focus();
clock.tick( 50 );

expect( balloonToolbar.focusTracker.isFocused ).to.true;

editableElement.dispatchEvent( new Event( 'blur' ) );
focusHolder.focus();
clock.tick( 50 );
expect( balloonToolbar.focusTracker.isFocused ).to.false;

Expand All @@ -922,21 +927,21 @@ describe( 'BalloonToolbar', () => {
const clock = sinon.useFakeTimers();

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

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

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

editor.detachRoot( 'dynamicRoot' );

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

// Focus is no longer tracked.
editableElement.dispatchEvent( new Event( 'focus' ) );
editableElement.focus();
clock.tick( 50 );

expect( balloonToolbar.focusTracker.isFocused ).to.false;
Expand All @@ -950,29 +955,29 @@ describe( 'BalloonToolbar', () => {
addEditableOnRootAdd = false;

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

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

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

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

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

// Lets test focus
editableElement.dispatchEvent( new Event( 'focus' ) );
editableElement.focus();
clock.tick( 50 );

expect( balloonToolbar.focusTracker.isFocused ).to.true;

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

editableElement.remove();
clock.restore();
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"type": "module",
"main": "src/index.ts",
"dependencies": {
"lodash-es": "4.17.21"
"lodash-es": "4.17.21",
"@ckeditor/ckeditor5-ui": "43.1.0"
},
"devDependencies": {
"@ckeditor/ckeditor5-build-classic": "43.1.0",
Expand Down
Loading