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

Allowed for linking FocusTracker instances together and propagate the focus state #17206

Merged
merged 12 commits into from
Oct 22, 2024
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 @@ -209,6 +209,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
21 changes: 19 additions & 2 deletions packages/ckeditor5-ui/src/dropdown/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
global,
priorities,
logWarning,
type FocusTracker,
type Collection,
type Locale,
type ObservableChangeEvent
Expand Down Expand Up @@ -198,6 +199,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 Expand Up @@ -231,7 +233,7 @@ function addMenuToOpenDropdown(
// Nested menu panels are added to body collection, so they are not children of the `dropdownView` from DOM perspective.
// Add these panels to `dropdownView` focus tracker, so they are treated like part of the `dropdownView` for focus-related purposes.
for ( const menu of dropdownMenuRootListView.menus ) {
dropdownView.focusTracker.add( menu.panelView.element! );
dropdownView.focusTracker.add( menu );
}

dropdownMenuRootListView.ariaLabel = options.ariaLabel || t( 'Dropdown menu' );
Expand Down Expand Up @@ -360,6 +362,7 @@ function addToolbarToOpenDropdown(
}

dropdownView.panelView.children.add( toolbarView );
dropdownView.focusTracker.add( toolbarView );
toolbarView.items.delegate( 'execute' ).to( dropdownView );
}

Expand Down Expand Up @@ -537,11 +540,25 @@ function closeDropdownOnClickOutside( dropdownView: DropdownView ) {
},
contextElements: () => [
dropdownView.element!,
...dropdownView.focusTracker.elements
// Include all elements connected to the dropdown's focus tracker, but exclude those that are direct children
// of DropdownView#element. They would be identified as descendants of #element anyway upon clicking and would
// not contribute to the logic.
...getFocusTrackerTreeElements( dropdownView.focusTracker ).filter( element => !dropdownView.element!.contains( element ) )
]
} );
}

/**
* Returns all DOM elements connected to a DropdownView's focus tracker, either directly (same DOM sub-tree)
* or indirectly (external views registered in the focus tracker).
*/
function getFocusTrackerTreeElements( focusTracker: FocusTracker ): Array<Element> {
return [
...focusTracker.elements,
...focusTracker.externalViews.flatMap( view => getFocusTrackerTreeElements( view.focusTracker ) )
];
}

/**
* Adds a behavior to a dropdownView that closes the dropdown view on "execute" event.
*/
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
2 changes: 1 addition & 1 deletion packages/ckeditor5-ui/src/search/text/searchtextview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ export default class SearchTextView<
const stopPropagation = ( data: Event ) => data.stopPropagation();

for ( const focusableChild of this.focusableChildren ) {
this.focusTracker.add( focusableChild.element as Element );
this.focusTracker.add( focusableChild.element as HTMLElement );
}

// Start listening for the keystrokes coming from #element.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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
6 changes: 3 additions & 3 deletions packages/ckeditor5-ui/src/toolbar/toolbarview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,15 +260,15 @@ export default class ToolbarView extends View implements DropdownPanelFocusable

// Children added before rendering should be known to the #focusTracker.
for ( const item of this.items ) {
this.focusTracker.add( item.element! );
this.focusTracker.add( item );
}

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
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
63 changes: 62 additions & 1 deletion packages/ckeditor5-ui/tests/dropdown/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/* globals document, Event, console */

import { assertBinding } from '@ckeditor/ckeditor5-utils/tests/_utils/utils.js';
import { global, keyCodes, Locale } from '@ckeditor/ckeditor5-utils';
import { FocusTracker, global, keyCodes, Locale } from '@ckeditor/ckeditor5-utils';
import Collection from '@ckeditor/ckeditor5-utils/src/collection.js';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js';

Expand Down Expand Up @@ -193,6 +193,42 @@ describe( 'utils', () => {

documentElement.remove();
} );

it( 'considers DOM elements in different DOM sub-trees but connected via focus trackers', () => {
// <body>
// <DropdownView#element />
// <child-view />
// <secondary-child-view />
// </body>
const childView = new View();
const secondaryChildView = new View();

childView.setTemplate( { tag: 'child-view' } );
secondaryChildView.setTemplate( { tag: 'secondary-child-view' } );

childView.focusTracker = new FocusTracker();

childView.render();
secondaryChildView.render();
document.body.appendChild( childView.element );
document.body.appendChild( secondaryChildView.element );

// DropdownView#focusTracker -> child-view#focusTracker -> secondary-child-view#focusTracker
dropdownView.focusTracker.add( childView );
childView.focusTracker.add( secondaryChildView );

dropdownView.isOpen = true;

secondaryChildView.element.dispatchEvent( new Event( 'mousedown', {
bubbles: true
} ) );

// External view's element is logically connected to the dropdown view's element via focus tracker.
expect( dropdownView.isOpen ).to.be.true;

childView.element.remove();
secondaryChildView.element.remove();
} );
} );

describe( 'closeDropdownOnExecute()', () => {
Expand Down Expand Up @@ -690,6 +726,15 @@ describe( 'utils', () => {
dropdownView.element.remove();
} );

// https://github.com/cksource/ckeditor5-commercial/issues/6633
it( 'should add the ToolbarView instance of dropdown\'s focus tracker to allow for using toolbar items distributed ' +
'across the DOM sub-trees', () => {
// Lazy load.
dropdownView.isOpen = true;

expect( dropdownView.focusTracker.externalViews ).to.include( dropdownView.toolbarView );
} );

it( 'focuses active item upon dropdown opening', () => {
buttons[ 0 ].isOn = true;

Expand Down Expand Up @@ -1391,6 +1436,22 @@ describe( 'utils', () => {
expect( dropdownView.menuView.render.calledOnce ).to.be.true;
} );

// https://github.com/cksource/ckeditor5-commercial/issues/6633
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 );

dropdownView.isOpen = true;

sinon.assert.calledThrice( addSpy );
sinon.assert.calledWithExactly( addSpy.firstCall, dropdownView.menuView );
sinon.assert.calledWithExactly( addSpy.secondCall, dropdownView.menuView.menus[ 0 ] );
sinon.assert.calledWithExactly( addSpy.thirdCall, dropdownView.menuView.menus[ 1 ] );
} );

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

Expand Down
10 changes: 5 additions & 5 deletions packages/ckeditor5-ui/tests/editorui/editorui.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,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 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
Loading