From fffbd88ad78ef18a071c89e0bc8f1c60b725247c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Thu, 19 Sep 2024 15:30:03 +0200 Subject: [PATCH 01/12] WIP --- .../ckeditor5-ui/src/dropdown/dropdownview.ts | 4 +- .../src/dropdown/menu/dropdownmenulistview.ts | 2 + .../menu/dropdownmenunestedmenuview.ts | 4 +- .../dropdown/menu/dropdownmenurootlistview.ts | 8 +- packages/ckeditor5-ui/src/dropdown/utils.ts | 1 + .../ckeditor5-ui/src/editorui/editorui.ts | 6 +- .../src/toolbar/balloon/balloontoolbar.ts | 6 +- .../ckeditor5-ui/src/toolbar/toolbarview.ts | 4 +- .../tests/manual/dropdown/dropdown.js | 2 + packages/ckeditor5-utils/src/focustracker.ts | 146 ++++++++++++++++-- packages/ckeditor5-utils/src/index.ts | 2 +- .../src/widgettoolbarrepository.ts | 2 + 12 files changed, 164 insertions(+), 23 deletions(-) diff --git a/packages/ckeditor5-ui/src/dropdown/dropdownview.ts b/packages/ckeditor5-ui/src/dropdown/dropdownview.ts index a33b00e463d..0a9abcf6061 100644 --- a/packages/ckeditor5-ui/src/dropdown/dropdownview.ts +++ b/packages/ckeditor5-ui/src/dropdown/dropdownview.ts @@ -222,7 +222,9 @@ export default class DropdownView extends View { this.panelView.bind( 'isVisible' ).to( this, 'isOpen' ); this.keystrokes = new KeystrokeHandler(); - this.focusTracker = new FocusTracker(); + this.focusTracker = new FocusTracker( () => { + return `DropdownView: "${ buttonView.label }"`; + } ); this.setTemplate( { tag: 'div', diff --git a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts index 154efca9ab6..e37c292c608 100644 --- a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts +++ b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts @@ -40,5 +40,7 @@ export default class DropdownMenuListView extends ListView { ] } } ); + + this.focusTracker._label = 'DropdownMenuListView'; } } diff --git a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts index 701d04c18a1..fb9800e30e1 100644 --- a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts +++ b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts @@ -137,7 +137,7 @@ export default class DropdownMenuNestedMenuView extends View implements Focusabl } ); this.keystrokes = new KeystrokeHandler(); - this.focusTracker = new FocusTracker(); + this.focusTracker = new FocusTracker( `dropdown menu "${ id }"` ); this.buttonView = new DropdownMenuButtonView( locale ); this.buttonView.delegate( 'mouseenter' ).to( this ); @@ -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; diff --git a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts index 7ad075a955c..34cc1729067 100644 --- a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts +++ b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts @@ -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. @@ -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. */ @@ -122,6 +124,8 @@ export default class DropdownMenuRootListView extends DropdownMenuListView { this._bodyCollection = bodyCollection; this._definition = definition; + this.focusTracker = new FocusTracker( 'DropdownMenuRootListView' ); + this.set( 'menuPanelClass', undefined ); } @@ -152,6 +156,8 @@ export default class DropdownMenuRootListView extends DropdownMenuListView { DropdownRootMenuBehaviors.toggleMenusAndFocusItemsOnHover( this ); DropdownRootMenuBehaviors.closeMenuWhenAnotherOnTheSameLevelOpens( this ); + + this.menus.forEach( menu => this.focusTracker.add( menu ) ); } /** diff --git a/packages/ckeditor5-ui/src/dropdown/utils.ts b/packages/ckeditor5-ui/src/dropdown/utils.ts index 02918783076..6ed79f77e8c 100644 --- a/packages/ckeditor5-ui/src/dropdown/utils.ts +++ b/packages/ckeditor5-ui/src/dropdown/utils.ts @@ -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 ); diff --git a/packages/ckeditor5-ui/src/editorui/editorui.ts b/packages/ckeditor5-ui/src/editorui/editorui.ts index 21851581a36..16b13f75b57 100644 --- a/packages/ckeditor5-ui/src/editorui/editorui.ts +++ b/packages/ckeditor5-ui/src/editorui/editorui.ts @@ -151,7 +151,7 @@ export default abstract class EditorUI extends /* #__PURE__ */ ObservableMixin() this.editor = editor; this.componentFactory = new ComponentFactory( editor ); - this.focusTracker = new FocusTracker(); + this.focusTracker = new FocusTracker( 'EditorUI' ); this.tooltipManager = new TooltipManager( editor ); this.poweredBy = new PoweredBy( editor ); this.ariaLiveAnnouncer = new AriaLiveAnnouncer( editor ); @@ -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( 'render', () => { - this.focusTracker.add( toolbarView.element! ); + this.focusTracker.remove( toolbarView ); this.editor.keystrokes.listenTo( toolbarView.element! ); } ); } diff --git a/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts b/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts index 42e59c0d3b2..71029298232 100644 --- a/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts +++ b/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts @@ -115,11 +115,11 @@ export default class BalloonToolbar extends Plugin { this._balloonConfig = normalizeToolbarConfig( editor.config.get( 'balloonToolbar' ) ); this.toolbarView = this._createToolbarView(); - this.focusTracker = new FocusTracker(); + this.focusTracker = new FocusTracker( 'balloon toolbar' ); // 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, { @@ -213,6 +213,8 @@ export default class BalloonToolbar extends Plugin { isFloating: true } ); + toolbarView.focusTracker._label = 'balloon toolbar\'s toolbar'; + toolbarView.ariaLabel = t( 'Editor contextual toolbar' ); toolbarView.render(); diff --git a/packages/ckeditor5-ui/src/toolbar/toolbarview.ts b/packages/ckeditor5-ui/src/toolbar/toolbarview.ts index 728b49fa645..685fe0a68a8 100644 --- a/packages/ckeditor5-ui/src/toolbar/toolbarview.ts +++ b/packages/ckeditor5-ui/src/toolbar/toolbarview.ts @@ -264,11 +264,11 @@ export default class ToolbarView extends View implements DropdownPanelFocusable } this.items.on>( 'add', ( evt, item ) => { - this.focusTracker.add( item.element! ); + this.focusTracker.add( item ); } ); this.items.on>( 'remove', ( evt, item ) => { - this.focusTracker.remove( item.element! ); + this.focusTracker.remove( item ); } ); // Start listening for the keystrokes coming from #element. diff --git a/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js b/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js index e56349795af..9b3d84a4a55 100644 --- a/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js +++ b/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js @@ -361,6 +361,8 @@ function testMenu() { const dropdownView = createDropdown( locale ); + dropdownView.focusTracker._label = 'Menu dropdown'; + addMenuToDropdown( dropdownView, bodyCollection, definitions ); dropdownView.buttonView.set( { diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index 296041a24c4..91f3af720d8 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -12,6 +12,24 @@ import DomEmitterMixin from './dom/emittermixin.js'; import ObservableMixin from './observablemixin.js'; import CKEditorError from './ckeditorerror.js'; +import { type View } from '@ckeditor/ckeditor5-ui'; + +// window.FOCUS_TRACKERS = []; + +// window.logFocusTrackers = function() { +// Array.from( window.FOCUS_TRACKERS ) +// .filter( a => a._getLabel() !== 'generic' ) +// .forEach( tracker => { +// console.group( logFT( tracker ) ); +// console.log( ' isFocused:', tracker.isFocused ); +// console.log( ' focusedElement:', tracker.focusedElement ); +// console.log( ' elements:', tracker.elements ); +// console.log( ' chainedFocusTrackers:', tracker.chainedFocusTrackers ); +// console.groupEnd(); +// } ); +// }; + +const DEBUG = true; /** * Allows observing a group of `Element`s whether at least one of them is focused. @@ -53,14 +71,28 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ */ public _elements: Set = new Set(); + /** + * TODO + */ + public _externalFocusTrackers: Set = new Set(); + /** * Event loop timeout. */ private _nextEventLoopTimeout: ReturnType | null = null; - constructor() { + /** + * TODO + */ + private _label: string | ( () => string ) | undefined; + + constructor( label?: string ) { super(); + this._label = label; + + // ( window.FOCUS_TRACKERS as any ).push( this ); + this.set( 'isFocused', false ); this.set( 'focusedElement', null ); } @@ -69,14 +101,50 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ * List of registered elements. */ public get elements(): Array { - return Array.from( this._elements.values() ); + return Array.from( this._elements.values() ) + .concat( this.externalFocusTrackers.flatMap( otherFocusTracker => otherFocusTracker.elements ) ); + } + + /** + * TODO + */ + public get externalFocusTrackers(): Array { + return Array.from( this._externalFocusTrackers.values() ); } /** * Starts tracking the specified element. */ - public add( element: Element ): void { - if ( this._elements.has( element ) ) { + public add( elementOrView: Element | ViewWithFocusTracker ): void { + if ( isViewWithFocusTracker( elementOrView ) ) { + const otherFocusTracker = elementOrView.focusTracker; + + if ( DEBUG ) { + console.log( `[FT] Add external ${ logFT( otherFocusTracker ) } to ${ logFT( this ) }` ); + } + + this.listenTo( otherFocusTracker, 'change:isFocused', () => { + if ( DEBUG ) { + console.group( `[FT] External ${ logFT( otherFocusTracker ) } change:isFocused =`, otherFocusTracker.isFocused ); + } + + if ( otherFocusTracker.isFocused ) { + this._focus( otherFocusTracker.focusedElement! ); + } else { + this._blur(); + } + + if ( DEBUG ) { + console.groupEnd(); + } + } ); + + this._externalFocusTrackers.add( otherFocusTracker ); + + return; + } + + if ( this._elements.has( elementOrView ) ) { /** * This element is already tracked by {@link module:utils/focustracker~FocusTracker}. * @@ -85,22 +153,39 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ throw new CKEditorError( 'focustracker-add-element-already-exist', this ); } - this.listenTo( element, 'focus', () => this._focus( element ), { useCapture: true } ); - this.listenTo( element, 'blur', () => this._blur(), { useCapture: true } ); - this._elements.add( element ); + this.listenTo( elementOrView, 'focus', () => this._focus( elementOrView ), { useCapture: true } ); + this.listenTo( elementOrView, 'blur', () => this._blur(), { useCapture: true } ); + this._elements.add( elementOrView ); } /** * Stops tracking the specified element and stops listening on this element. */ - public remove( element: Element ): void { - if ( element === this.focusedElement ) { + public remove( elementOrView: Element | ViewWithFocusTracker ): void { + if ( isViewWithFocusTracker( elementOrView ) ) { + const otherFocusTracker = elementOrView.focusTracker; + + if ( DEBUG ) { + console.log( `[FT] Remove external ${ logFT( otherFocusTracker ) } from ${ logFT( this ) }` ); + } + + this.stopListening( otherFocusTracker ); + this._externalFocusTrackers.delete( otherFocusTracker ); + + if ( otherFocusTracker.isFocused ) { + this._blur(); + } + + return; + } + + if ( elementOrView === this.focusedElement ) { this._blur(); } - if ( this._elements.has( element ) ) { - this.stopListening( element ); - this._elements.delete( element ); + if ( this._elements.has( elementOrView ) ) { + this.stopListening( elementOrView ); + this._elements.delete( elementOrView ); } } @@ -117,6 +202,10 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ * Stores currently focused element and set {@link #isFocused} as `true`. */ private _focus( element: Element ): void { + if ( DEBUG ) { + console.log( `[FT] ${ logFT( this ) }#focus()` ); + } + clearTimeout( this._nextEventLoopTimeout! ); this.focusedElement = element; @@ -128,11 +217,44 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ * This method uses `setTimeout` to change order of fires `blur` and `focus` events. */ private _blur(): void { + // Filter out blurs that are unnecessary: + // * Chained FT blurs (e.g. when the focus still remains in one of the "local" elements), + // * DOM blurs (e.g. when the focus still remains in one of chained focus trackers). + if ( + this.elements.find( element => element.contains( document.activeElement ) ) || + this.externalFocusTrackers.find( ( { isFocused } ) => isFocused ) + ) { + return; + } + clearTimeout( this._nextEventLoopTimeout! ); this._nextEventLoopTimeout = setTimeout( () => { + if ( DEBUG ) { + console.log( `[FT] ${ logFT( this ) }#blur()` ); + } + this.focusedElement = null; this.isFocused = false; }, 0 ); } + + public _getLabel(): string | undefined { + return ( typeof this._label == 'function' ? this._label() : this._label ) || 'generic'; + } +} + +export type ViewWithFocusTracker = View & { focusTracker: FocusTracker }; + +/** + * Checks whether a view is an instance of {@link ~ViewWithFocusTracker}. + * + * @param view A view to be checked. + */ +export function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracker { + return 'focusTracker' in view && view.focusTracker instanceof FocusTracker; +} + +function logFT( ft: FocusTracker ) { + return `[${ ft._getLabel() }]`; } diff --git a/packages/ckeditor5-utils/src/index.ts b/packages/ckeditor5-utils/src/index.ts index 0cc02b52f64..ade8aaebc97 100644 --- a/packages/ckeditor5-utils/src/index.ts +++ b/packages/ckeditor5-utils/src/index.ts @@ -80,7 +80,7 @@ export { type CollectionRemoveEvent } from './collection.js'; export { default as first } from './first.js'; -export { default as FocusTracker } from './focustracker.js'; +export { default as FocusTracker, isViewWithFocusTracker, type ViewWithFocusTracker } from './focustracker.js'; export { default as KeystrokeHandler, type KeystrokeHandlerOptions } from './keystrokehandler.js'; export { default as toArray, type ArrayOrItem, type ReadonlyArrayOrItem } from './toarray.js'; export { default as toMap } from './tomap.js'; diff --git a/packages/ckeditor5-widget/src/widgettoolbarrepository.ts b/packages/ckeditor5-widget/src/widgettoolbarrepository.ts index 777e0b0f572..6ec99735770 100644 --- a/packages/ckeditor5-widget/src/widgettoolbarrepository.ts +++ b/packages/ckeditor5-widget/src/widgettoolbarrepository.ts @@ -170,6 +170,8 @@ export default class WidgetToolbarRepository extends Plugin { const t = editor.t; const toolbarView = new ToolbarView( editor.locale ); + toolbarView.focusTracker._label = 'widget toolbar: ' + ariaLabel; + toolbarView.ariaLabel = ariaLabel || t( 'Widget toolbar' ); if ( this._toolbarDefinitions.has( toolbarId ) ) { From fe9b6eecdfc292ddefd2c5d4a079ea903af42f40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Thu, 19 Sep 2024 16:01:25 +0200 Subject: [PATCH 02/12] Disabled debug --- packages/ckeditor5-utils/src/focustracker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index 91f3af720d8..e5d3112772b 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -29,7 +29,7 @@ import { type View } from '@ckeditor/ckeditor5-ui'; // } ); // }; -const DEBUG = true; +const DEBUG = false; /** * Allows observing a group of `Element`s whether at least one of them is focused. From 4cbf9bed635ce927c9f8463ed82da7eff6355a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Fri, 20 Sep 2024 16:12:36 +0200 Subject: [PATCH 03/12] Code refactoring. --- packages/ckeditor5-utils/src/focustracker.ts | 138 +++++++++++++------ 1 file changed, 93 insertions(+), 45 deletions(-) diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index e5d3112772b..b24b217875b 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -12,7 +12,7 @@ import DomEmitterMixin from './dom/emittermixin.js'; import ObservableMixin from './observablemixin.js'; import CKEditorError from './ckeditorerror.js'; -import { type View } from '@ckeditor/ckeditor5-ui'; +import { View } from '@ckeditor/ckeditor5-ui'; // window.FOCUS_TRACKERS = []; @@ -115,36 +115,63 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ /** * Starts tracking the specified element. */ - public add( elementOrView: Element | ViewWithFocusTracker ): void { - if ( isViewWithFocusTracker( elementOrView ) ) { - const otherFocusTracker = elementOrView.focusTracker; - - if ( DEBUG ) { - console.log( `[FT] Add external ${ logFT( otherFocusTracker ) } to ${ logFT( this ) }` ); - } - - this.listenTo( otherFocusTracker, 'change:isFocused', () => { - if ( DEBUG ) { - console.group( `[FT] External ${ logFT( otherFocusTracker ) } change:isFocused =`, otherFocusTracker.isFocused ); + public add( elementOrView: Element | View ): void { + if ( isView( elementOrView ) ) { + if ( isViewWithFocusTracker( elementOrView ) ) { + this._addFocusTracker( elementOrView.focusTracker ); + } else { + if ( !elementOrView.element ) { + /** + * The {@link module:ui/view~View} added to the {@link module:utils/focustracker~FocusTracker} does not have an + * {@link module:ui/view~View#element}. Make sure the view is {@link module:ui/view~View#render} before adding + * it to the focus tracker. + * + * @error focustracker-add-view-missing-element + */ + throw new CKEditorError( 'focustracker-add-view-missing-element', { + focusTracker: this, + view: elementOrView + } ); } - if ( otherFocusTracker.isFocused ) { - this._focus( otherFocusTracker.focusedElement! ); - } else { - this._blur(); - } + this._addElement( elementOrView.element ); + } + } else { + this._addElement( elementOrView ); + } + } - if ( DEBUG ) { - console.groupEnd(); + /** + * Stops tracking the specified element and stops listening on this element. + */ + public remove( elementOrView: Element | View ): void { + if ( isView( elementOrView ) ) { + if ( isViewWithFocusTracker( elementOrView ) ) { + this._removeFocusTracker( elementOrView.focusTracker ); + } else { + if ( !elementOrView.element ) { + /** + * The {@link module:ui/view~View} removed from the {@link module:utils/focustracker~FocusTracker} does not have an + * {@link module:ui/view~View#element}. Make sure the view is {@link module:ui/view~View#render} before removing + * it from the focus tracker. + * + * @error focustracker-remove-view-missing-element + */ + throw new CKEditorError( 'focustracker-remove-view-missing-element', { + focusTracker: this, + view: elementOrView + } ); } - } ); - - this._externalFocusTrackers.add( otherFocusTracker ); - return; + this._removeElement( elementOrView.element ); + } + } else { + this._removeElement( elementOrView ); } + } - if ( this._elements.has( elementOrView ) ) { + private _addElement( element: Element ): void { + if ( this._elements.has( element ) ) { /** * This element is already tracked by {@link module:utils/focustracker~FocusTracker}. * @@ -153,39 +180,56 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ throw new CKEditorError( 'focustracker-add-element-already-exist', this ); } - this.listenTo( elementOrView, 'focus', () => this._focus( elementOrView ), { useCapture: true } ); - this.listenTo( elementOrView, 'blur', () => this._blur(), { useCapture: true } ); - this._elements.add( elementOrView ); + this.listenTo( element, 'focus', () => this._focus( element ), { useCapture: true } ); + this.listenTo( element, 'blur', () => this._blur(), { useCapture: true } ); + this._elements.add( element ); } - /** - * Stops tracking the specified element and stops listening on this element. - */ - public remove( elementOrView: Element | ViewWithFocusTracker ): void { - if ( isViewWithFocusTracker( elementOrView ) ) { - const otherFocusTracker = elementOrView.focusTracker; + private _removeElement( element: Element ): void { + if ( element === this.focusedElement ) { + this._blur(); + } + if ( this._elements.has( element ) ) { + this.stopListening( element ); + this._elements.delete( element ); + } + } + + private _addFocusTracker( focusTracker: FocusTracker ): void { + if ( DEBUG ) { + console.log( `[FT] Add external ${ logFT( focusTracker ) } to ${ logFT( this ) }` ); + } + + this.listenTo( focusTracker, 'change:isFocused', () => { if ( DEBUG ) { - console.log( `[FT] Remove external ${ logFT( otherFocusTracker ) } from ${ logFT( this ) }` ); + console.group( `[FT] External ${ logFT( focusTracker ) } change:isFocused =`, focusTracker.isFocused ); } - this.stopListening( otherFocusTracker ); - this._externalFocusTrackers.delete( otherFocusTracker ); - - if ( otherFocusTracker.isFocused ) { + if ( focusTracker.isFocused ) { + this._focus( focusTracker.focusedElement! ); + } else { this._blur(); } - return; - } + if ( DEBUG ) { + console.groupEnd(); + } + } ); - if ( elementOrView === this.focusedElement ) { - this._blur(); + this._externalFocusTrackers.add( focusTracker ); + } + + private _removeFocusTracker( focusTracker: FocusTracker ): void { + if ( DEBUG ) { + console.log( `[FT] Remove external ${ logFT( focusTracker ) } from ${ logFT( this ) }` ); } - if ( this._elements.has( elementOrView ) ) { - this.stopListening( elementOrView ); - this._elements.delete( elementOrView ); + this.stopListening( focusTracker ); + this._externalFocusTrackers.delete( focusTracker ); + + if ( focusTracker.isFocused ) { + this._blur(); } } @@ -255,6 +299,10 @@ export function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracke return 'focusTracker' in view && view.focusTracker instanceof FocusTracker; } +function isView( item: unknown ): item is View { + return item instanceof View; +} + function logFT( ft: FocusTracker ) { return `[${ ft._getLabel() }]`; } From 62f4f0463e23281fff35836320d4956579fd3a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Fri, 20 Sep 2024 16:40:27 +0200 Subject: [PATCH 04/12] WIP --- packages/ckeditor5-utils/src/focustracker.ts | 4 ++-- packages/ckeditor5-utils/src/index.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index b24b217875b..36c822ef863 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -288,14 +288,14 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ } } -export type ViewWithFocusTracker = View & { focusTracker: FocusTracker }; +type ViewWithFocusTracker = View & { focusTracker: FocusTracker }; /** * Checks whether a view is an instance of {@link ~ViewWithFocusTracker}. * * @param view A view to be checked. */ -export function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracker { +function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracker { return 'focusTracker' in view && view.focusTracker instanceof FocusTracker; } diff --git a/packages/ckeditor5-utils/src/index.ts b/packages/ckeditor5-utils/src/index.ts index ade8aaebc97..0cc02b52f64 100644 --- a/packages/ckeditor5-utils/src/index.ts +++ b/packages/ckeditor5-utils/src/index.ts @@ -80,7 +80,7 @@ export { type CollectionRemoveEvent } from './collection.js'; export { default as first } from './first.js'; -export { default as FocusTracker, isViewWithFocusTracker, type ViewWithFocusTracker } from './focustracker.js'; +export { default as FocusTracker } from './focustracker.js'; export { default as KeystrokeHandler, type KeystrokeHandlerOptions } from './keystrokehandler.js'; export { default as toArray, type ArrayOrItem, type ReadonlyArrayOrItem } from './toarray.js'; export { default as toMap } from './tomap.js'; From 9c1d2cef2c50421acdf48891cb13a175a71f5b51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Fri, 20 Sep 2024 16:56:45 +0200 Subject: [PATCH 05/12] Code refactoring. --- packages/ckeditor5-utils/src/focustracker.ts | 23 ++++++++++---------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index 36c822ef863..64df88bf4b8 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -12,7 +12,8 @@ import DomEmitterMixin from './dom/emittermixin.js'; import ObservableMixin from './observablemixin.js'; import CKEditorError from './ckeditorerror.js'; -import { View } from '@ckeditor/ckeditor5-ui'; +import { type View } from '@ckeditor/ckeditor5-ui'; +import { isElement as _isElement } from 'lodash-es'; // window.FOCUS_TRACKERS = []; @@ -116,7 +117,9 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ * Starts tracking the specified element. */ public add( elementOrView: Element | View ): void { - if ( isView( elementOrView ) ) { + if ( isElement( elementOrView ) ) { + this._addElement( elementOrView ); + } else { if ( isViewWithFocusTracker( elementOrView ) ) { this._addFocusTracker( elementOrView.focusTracker ); } else { @@ -136,8 +139,6 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ this._addElement( elementOrView.element ); } - } else { - this._addElement( elementOrView ); } } @@ -145,7 +146,9 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ * Stops tracking the specified element and stops listening on this element. */ public remove( elementOrView: Element | View ): void { - if ( isView( elementOrView ) ) { + if ( isElement( elementOrView ) ) { + this._removeElement( elementOrView ); + } else { if ( isViewWithFocusTracker( elementOrView ) ) { this._removeFocusTracker( elementOrView.focusTracker ); } else { @@ -165,8 +168,6 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ this._removeElement( elementOrView.element ); } - } else { - this._removeElement( elementOrView ); } } @@ -299,10 +300,10 @@ function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracker { return 'focusTracker' in view && view.focusTracker instanceof FocusTracker; } -function isView( item: unknown ): item is View { - return item instanceof View; -} - function logFT( ft: FocusTracker ) { return `[${ ft._getLabel() }]`; } + +function isElement( value: any ): value is Element { + return _isElement( value ); +} From 451c634b286a101cca861f897c938fffd51d5282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Fri, 20 Sep 2024 17:40:08 +0200 Subject: [PATCH 06/12] Tests: Aligned tests to the new FocusTracker behavior. --- .../ckeditor5-ui/src/editorui/editorui.ts | 2 +- .../ckeditor5-ui/tests/editorui/editorui.js | 4 +- .../tests/toolbar/balloon/balloontoolbar.js | 37 +++++++++++-------- packages/ckeditor5-utils/src/focustracker.ts | 3 +- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/packages/ckeditor5-ui/src/editorui/editorui.ts b/packages/ckeditor5-ui/src/editorui/editorui.ts index 16b13f75b57..2e289681962 100644 --- a/packages/ckeditor5-ui/src/editorui/editorui.ts +++ b/packages/ckeditor5-ui/src/editorui/editorui.ts @@ -310,7 +310,7 @@ export default abstract class EditorUI extends /* #__PURE__ */ ObservableMixin() this.editor.keystrokes.listenTo( toolbarView.element! ); } else { toolbarView.once( 'render', () => { - this.focusTracker.remove( toolbarView ); + this.focusTracker.add( toolbarView ); this.editor.keystrokes.listenTo( toolbarView.element! ); } ); } diff --git a/packages/ckeditor5-ui/tests/editorui/editorui.js b/packages/ckeditor5-ui/tests/editorui/editorui.js index 66beefbfe8a..1492766aa8c 100644 --- a/packages/ckeditor5-ui/tests/editorui/editorui.js +++ b/packages/ckeditor5-ui/tests/editorui/editorui.js @@ -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' ] ); diff --git a/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js b/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js index d624521bc89..f8163797510 100644 --- a/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js +++ b/packages/ckeditor5-ui/tests/toolbar/balloon/balloontoolbar.js @@ -828,7 +828,7 @@ describe( 'BalloonToolbar', () => { } ); describe( 'MultiRoot editor integration', () => { - let rootsElements, addEditableOnRootAdd; + let rootsElements, addEditableOnRootAdd, focusHolder; beforeEach( async () => { addEditableOnRootAdd = true; @@ -849,6 +849,9 @@ describe( 'BalloonToolbar', () => { editor = await createMultiRootEditor(); balloonToolbar = editor.plugins.get( BalloonToolbar ); + + focusHolder = document.createElement( 'input' ); + document.body.appendChild( focusHolder ); } ); afterEach( async () => { @@ -858,6 +861,8 @@ describe( 'BalloonToolbar', () => { await editor.destroy(); editor = null; + + focusHolder.remove(); } ); it( 'should create plugin instance', () => { @@ -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; } @@ -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; @@ -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; @@ -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(); diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index 64df88bf4b8..2027b643b7b 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -102,8 +102,7 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ * List of registered elements. */ public get elements(): Array { - return Array.from( this._elements.values() ) - .concat( this.externalFocusTrackers.flatMap( otherFocusTracker => otherFocusTracker.elements ) ); + return Array.from( this._elements.values() ); } /** From 2f39bffedf4277a577dcd458d4de63c5fcc8d64f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 23 Sep 2024 12:15:50 +0200 Subject: [PATCH 07/12] Code refactoring and docs. --- packages/ckeditor5-utils/src/focustracker.ts | 136 ++++++++----------- packages/ckeditor5-utils/src/index.ts | 2 +- 2 files changed, 59 insertions(+), 79 deletions(-) diff --git a/packages/ckeditor5-utils/src/focustracker.ts b/packages/ckeditor5-utils/src/focustracker.ts index 2027b643b7b..30389c18a07 100644 --- a/packages/ckeditor5-utils/src/focustracker.ts +++ b/packages/ckeditor5-utils/src/focustracker.ts @@ -15,25 +15,8 @@ import CKEditorError from './ckeditorerror.js'; import { type View } from '@ckeditor/ckeditor5-ui'; import { isElement as _isElement } from 'lodash-es'; -// window.FOCUS_TRACKERS = []; - -// window.logFocusTrackers = function() { -// Array.from( window.FOCUS_TRACKERS ) -// .filter( a => a._getLabel() !== 'generic' ) -// .forEach( tracker => { -// console.group( logFT( tracker ) ); -// console.log( ' isFocused:', tracker.isFocused ); -// console.log( ' focusedElement:', tracker.focusedElement ); -// console.log( ' elements:', tracker.elements ); -// console.log( ' chainedFocusTrackers:', tracker.chainedFocusTrackers ); -// console.groupEnd(); -// } ); -// }; - -const DEBUG = false; - /** - * Allows observing a group of `Element`s whether at least one of them is focused. + * Allows observing a group of DOM `Element`s whether at least one of them (or their child) is focused. * * Used by the {@link module:core/editor/editor~Editor} in order to track whether the focus is still within the application, * or were used outside of its UI. @@ -46,7 +29,7 @@ const DEBUG = false; */ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #__PURE__ */ ObservableMixin() ) { /** - * True when one of the registered elements is focused. + * True when one of the registered {@link #elements} or {@link #externalFocusTrackers} is focused. * * @readonly * @observable @@ -56,10 +39,11 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ /** * The currently focused element. * - * While {@link #isFocused `isFocused`} remains `true`, the focus can - * move between different UI elements. This property tracks those + * While {@link #isFocused `isFocused`} remains `true`, the focus can move between different UI elements. This property tracks those * elements and tells which one is currently focused. * + * **Note**: The values of this property are restricted to {@link #elements} or elements registered in {@link #externalFocusTrackers}. + * * @readonly * @observable */ @@ -73,7 +57,9 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ public _elements: Set = new Set(); /** - * TODO + * List of external focus trackers that contribute to the state of this focus tracker. + * + * @internal */ public _externalFocusTrackers: Set = new Set(); @@ -82,38 +68,40 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ */ private _nextEventLoopTimeout: ReturnType | null = null; - /** - * TODO - */ - private _label: string | ( () => string ) | undefined; - - constructor( label?: string ) { + constructor() { super(); - this._label = label; - - // ( window.FOCUS_TRACKERS as any ).push( this ); - this.set( 'isFocused', false ); this.set( 'focusedElement', null ); } /** - * List of registered elements. + * List of registered DOM elements. + * + * **Note**: The list does do not include elements from {@link #externalFocusTrackers}. */ public get elements(): Array { return Array.from( this._elements.values() ); } /** - * TODO + * List of external focus trackers that contribute to the state of this focus tracker. See {@link #add} to learn more. */ public get externalFocusTrackers(): Array { return Array.from( this._externalFocusTrackers.values() ); } /** - * Starts tracking the specified element. + * Starts tracking a specified DOM element or a {@link module:ui/view~View} instance. + * + * * If a DOM element is passed, the focus tracker listens to the `focus` and `blur` events on this element. + * Tracked elements are listed in {@link #elements}. + * * If a {@link module:ui/view~View} instance is passed that has a `FocusTracker` instance ({@link ~ViewWithFocusTracker}), + * the external focus tracker's state ({@link #isFocused}, {@link #focusedElement}) starts contributing to the current tracker instance. + * This allows for increasing the "reach" of a focus tracker instance, by connecting two or more focus trackers together when DOM + * elements they track are located in different subtrees in DOM. External focus trackers are listed in {@link #externalFocusTrackers}. + * * If a {@link module:ui/view~View} instance is passed that has no `FocusTracker` (**not** a {@link ~ViewWithFocusTracker}), + * its {@link module:ui/view~View#element} is used to track focus like any other DOM element. */ public add( elementOrView: Element | View ): void { if ( isElement( elementOrView ) ) { @@ -142,7 +130,7 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ } /** - * Stops tracking the specified element and stops listening on this element. + * Stops tracking focus in the specified DOM element or a {@link module:ui/view~View view instance}. See {@link #add} to learn more. */ public remove( elementOrView: Element | View ): void { if ( isElement( elementOrView ) ) { @@ -170,6 +158,9 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ } } + /** + * Adds a DOM element to the focus tracker and starts listening to the `focus` and `blur` events on it. + */ private _addElement( element: Element ): void { if ( this._elements.has( element ) ) { /** @@ -185,6 +176,9 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ this._elements.add( element ); } + /** + * Removes a DOM element from the focus tracker. + */ private _removeElement( element: Element ): void { if ( element === this.focusedElement ) { this._blur(); @@ -196,35 +190,25 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ } } + /** + * Adds an external `FocusTracker` instance to this focus tracker and makes it contribute to this focus tracker's state. + */ private _addFocusTracker( focusTracker: FocusTracker ): void { - if ( DEBUG ) { - console.log( `[FT] Add external ${ logFT( focusTracker ) } to ${ logFT( this ) }` ); - } - this.listenTo( focusTracker, 'change:isFocused', () => { - if ( DEBUG ) { - console.group( `[FT] External ${ logFT( focusTracker ) } change:isFocused =`, focusTracker.isFocused ); - } - if ( focusTracker.isFocused ) { this._focus( focusTracker.focusedElement! ); } else { this._blur(); } - - if ( DEBUG ) { - console.groupEnd(); - } } ); this._externalFocusTrackers.add( focusTracker ); } + /** + * Removes an external `FocusTracker` instance from this focus tracker. + */ private _removeFocusTracker( focusTracker: FocusTracker ): void { - if ( DEBUG ) { - console.log( `[FT] Remove external ${ logFT( focusTracker ) } from ${ logFT( this ) }` ); - } - this.stopListening( focusTracker ); this._externalFocusTrackers.delete( focusTracker ); @@ -240,16 +224,18 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ */ public destroy(): void { this.stopListening(); + + this._elements.clear(); + this._externalFocusTrackers.clear(); + + this.isFocused = false; + this.focusedElement = null; } /** - * Stores currently focused element and set {@link #isFocused} as `true`. + * Stores currently focused element as {@link #focusedElement} and sets {@link #isFocused} `true`. */ private _focus( element: Element ): void { - if ( DEBUG ) { - console.log( `[FT] ${ logFT( this ) }#focus()` ); - } - clearTimeout( this._nextEventLoopTimeout! ); this.focusedElement = element; @@ -257,13 +243,17 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ } /** - * Clears currently focused element and set {@link #isFocused} as `false`. - * This method uses `setTimeout` to change order of fires `blur` and `focus` events. + * Clears currently {@link #focusedElement} and sets {@link #isFocused} `false`. + * + * This method uses `setTimeout()` to change order of fires `blur` and `focus` events ensuring that moving focus between + * two elements within a single focus tracker's scope, will not cause `[ blurA, focusB ]` sequence but just `[ focusB ]`. + * The former would cause a momentary change of `#isFocused` to `false` which is not desired because any logic listening to + * a focus tracker state would experience UI flashes and glitches as the user focus travels across the UI. */ private _blur(): void { - // Filter out blurs that are unnecessary: - // * Chained FT blurs (e.g. when the focus still remains in one of the "local" elements), - // * DOM blurs (e.g. when the focus still remains in one of chained focus trackers). + // Avoid blurs that would be incorrect as a result of "local" elements and external focus trackers coexisting: + // * External FT blurs (e.g. when the focus still remains in one of the "local" elements or another external focus tracker), + // * "Local" element blurs (e.g. when the focus still remains in one of external focus trackers). if ( this.elements.find( element => element.contains( document.activeElement ) ) || this.externalFocusTrackers.find( ( { isFocused } ) => isFocused ) @@ -274,35 +264,25 @@ export default class FocusTracker extends /* #__PURE__ */ DomEmitterMixin( /* #_ clearTimeout( this._nextEventLoopTimeout! ); this._nextEventLoopTimeout = setTimeout( () => { - if ( DEBUG ) { - console.log( `[FT] ${ logFT( this ) }#blur()` ); - } - this.focusedElement = null; this.isFocused = false; }, 0 ); } - - public _getLabel(): string | undefined { - return ( typeof this._label == 'function' ? this._label() : this._label ) || 'generic'; - } } -type ViewWithFocusTracker = View & { focusTracker: FocusTracker }; +/** + * A {@link module:ui/view~View} instance with a {@link module:utils/focustracker~FocusTracker} instance exposed + * at the `#focusTracker` property. + */ +export type ViewWithFocusTracker = View & { focusTracker: FocusTracker }; /** * Checks whether a view is an instance of {@link ~ViewWithFocusTracker}. - * - * @param view A view to be checked. */ -function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracker { +export function isViewWithFocusTracker( view: any ): view is ViewWithFocusTracker { return 'focusTracker' in view && view.focusTracker instanceof FocusTracker; } -function logFT( ft: FocusTracker ) { - return `[${ ft._getLabel() }]`; -} - function isElement( value: any ): value is Element { return _isElement( value ); } diff --git a/packages/ckeditor5-utils/src/index.ts b/packages/ckeditor5-utils/src/index.ts index 0cc02b52f64..296ea634ecb 100644 --- a/packages/ckeditor5-utils/src/index.ts +++ b/packages/ckeditor5-utils/src/index.ts @@ -80,7 +80,7 @@ export { type CollectionRemoveEvent } from './collection.js'; export { default as first } from './first.js'; -export { default as FocusTracker } from './focustracker.js'; +export { default as FocusTracker, type ViewWithFocusTracker, isViewWithFocusTracker } from './focustracker.js'; export { default as KeystrokeHandler, type KeystrokeHandlerOptions } from './keystrokehandler.js'; export { default as toArray, type ArrayOrItem, type ReadonlyArrayOrItem } from './toarray.js'; export { default as toMap } from './tomap.js'; From 06eb6262666ab8f543cb9fe847b384fcc7ac16e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 23 Sep 2024 13:25:47 +0200 Subject: [PATCH 08/12] Code cleanup. --- packages/ckeditor5-ui/src/dropdown/dropdownview.ts | 4 +--- .../ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts | 2 -- .../src/dropdown/menu/dropdownmenunestedmenuview.ts | 2 +- .../src/dropdown/menu/dropdownmenurootlistview.ts | 2 +- packages/ckeditor5-ui/src/editorui/editorui.ts | 2 +- packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts | 4 +--- packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js | 2 -- packages/ckeditor5-widget/src/widgettoolbarrepository.ts | 2 -- 8 files changed, 5 insertions(+), 15 deletions(-) diff --git a/packages/ckeditor5-ui/src/dropdown/dropdownview.ts b/packages/ckeditor5-ui/src/dropdown/dropdownview.ts index 0a9abcf6061..a33b00e463d 100644 --- a/packages/ckeditor5-ui/src/dropdown/dropdownview.ts +++ b/packages/ckeditor5-ui/src/dropdown/dropdownview.ts @@ -222,9 +222,7 @@ export default class DropdownView extends View { this.panelView.bind( 'isVisible' ).to( this, 'isOpen' ); this.keystrokes = new KeystrokeHandler(); - this.focusTracker = new FocusTracker( () => { - return `DropdownView: "${ buttonView.label }"`; - } ); + this.focusTracker = new FocusTracker(); this.setTemplate( { tag: 'div', diff --git a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts index e37c292c608..154efca9ab6 100644 --- a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts +++ b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenulistview.ts @@ -40,7 +40,5 @@ export default class DropdownMenuListView extends ListView { ] } } ); - - this.focusTracker._label = 'DropdownMenuListView'; } } diff --git a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts index fb9800e30e1..d7de28a3f88 100644 --- a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts +++ b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenunestedmenuview.ts @@ -137,7 +137,7 @@ export default class DropdownMenuNestedMenuView extends View implements Focusabl } ); this.keystrokes = new KeystrokeHandler(); - this.focusTracker = new FocusTracker( `dropdown menu "${ id }"` ); + this.focusTracker = new FocusTracker(); this.buttonView = new DropdownMenuButtonView( locale ); this.buttonView.delegate( 'mouseenter' ).to( this ); diff --git a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts index 34cc1729067..862fafcbf60 100644 --- a/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts +++ b/packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts @@ -124,7 +124,7 @@ export default class DropdownMenuRootListView extends DropdownMenuListView { this._bodyCollection = bodyCollection; this._definition = definition; - this.focusTracker = new FocusTracker( 'DropdownMenuRootListView' ); + this.focusTracker = new FocusTracker(); this.set( 'menuPanelClass', undefined ); } diff --git a/packages/ckeditor5-ui/src/editorui/editorui.ts b/packages/ckeditor5-ui/src/editorui/editorui.ts index 2e289681962..ed5a890eef1 100644 --- a/packages/ckeditor5-ui/src/editorui/editorui.ts +++ b/packages/ckeditor5-ui/src/editorui/editorui.ts @@ -151,7 +151,7 @@ export default abstract class EditorUI extends /* #__PURE__ */ ObservableMixin() this.editor = editor; this.componentFactory = new ComponentFactory( editor ); - this.focusTracker = new FocusTracker( 'EditorUI' ); + this.focusTracker = new FocusTracker(); this.tooltipManager = new TooltipManager( editor ); this.poweredBy = new PoweredBy( editor ); this.ariaLiveAnnouncer = new AriaLiveAnnouncer( editor ); diff --git a/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts b/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts index 71029298232..de8ddb96655 100644 --- a/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts +++ b/packages/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.ts @@ -115,7 +115,7 @@ export default class BalloonToolbar extends Plugin { this._balloonConfig = normalizeToolbarConfig( editor.config.get( 'balloonToolbar' ) ); this.toolbarView = this._createToolbarView(); - this.focusTracker = new FocusTracker( 'balloon toolbar' ); + this.focusTracker = new FocusTracker(); // Track focusable elements in the toolbar and the editable elements. this._trackFocusableEditableElements(); @@ -213,8 +213,6 @@ export default class BalloonToolbar extends Plugin { isFloating: true } ); - toolbarView.focusTracker._label = 'balloon toolbar\'s toolbar'; - toolbarView.ariaLabel = t( 'Editor contextual toolbar' ); toolbarView.render(); diff --git a/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js b/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js index 9b3d84a4a55..e56349795af 100644 --- a/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js +++ b/packages/ckeditor5-ui/tests/manual/dropdown/dropdown.js @@ -361,8 +361,6 @@ function testMenu() { const dropdownView = createDropdown( locale ); - dropdownView.focusTracker._label = 'Menu dropdown'; - addMenuToDropdown( dropdownView, bodyCollection, definitions ); dropdownView.buttonView.set( { diff --git a/packages/ckeditor5-widget/src/widgettoolbarrepository.ts b/packages/ckeditor5-widget/src/widgettoolbarrepository.ts index 6ec99735770..777e0b0f572 100644 --- a/packages/ckeditor5-widget/src/widgettoolbarrepository.ts +++ b/packages/ckeditor5-widget/src/widgettoolbarrepository.ts @@ -170,8 +170,6 @@ export default class WidgetToolbarRepository extends Plugin { const t = editor.t; const toolbarView = new ToolbarView( editor.locale ); - toolbarView.focusTracker._label = 'widget toolbar: ' + ariaLabel; - toolbarView.ariaLabel = ariaLabel || t( 'Widget toolbar' ); if ( this._toolbarDefinitions.has( toolbarId ) ) { From f22898c4e9ff381e8727d1b4655bae6f44d9224b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 23 Sep 2024 14:55:40 +0200 Subject: [PATCH 09/12] Added missing dependencies to package.json in ckeditor5-utils. --- packages/ckeditor5-utils/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ckeditor5-utils/package.json b/packages/ckeditor5-utils/package.json index 486b7d3dbf2..fc6e2a65fc3 100644 --- a/packages/ckeditor5-utils/package.json +++ b/packages/ckeditor5-utils/package.json @@ -19,6 +19,7 @@ "@ckeditor/ckeditor5-editor-classic": "43.1.0", "@ckeditor/ckeditor5-core": "43.1.0", "@ckeditor/ckeditor5-engine": "43.1.0", + "@ckeditor/ckeditor5-ui": "43.1.0", "@types/lodash-es": "^4.17.12", "typescript": "5.0.4" }, From 72a96f0682106e09aa1bc834482065ff0d20e044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 23 Sep 2024 14:55:52 +0200 Subject: [PATCH 10/12] Tests: Stabilized a manual test. --- packages/ckeditor5-theme-lark/tests/manual/theme.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-theme-lark/tests/manual/theme.js b/packages/ckeditor5-theme-lark/tests/manual/theme.js index 7db630b8fc0..3a1197a0ed1 100644 --- a/packages/ckeditor5-theme-lark/tests/manual/theme.js +++ b/packages/ckeditor5-theme-lark/tests/manual/theme.js @@ -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'; } } From 1c198a93a6999812daebe7c50d5c28ba19b7302d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Mon, 23 Sep 2024 15:16:39 +0200 Subject: [PATCH 11/12] Addressed an issue with dependencies in package.json. --- packages/ckeditor5-utils/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-utils/package.json b/packages/ckeditor5-utils/package.json index fc6e2a65fc3..ff7812e0ef8 100644 --- a/packages/ckeditor5-utils/package.json +++ b/packages/ckeditor5-utils/package.json @@ -12,14 +12,14 @@ "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", "@ckeditor/ckeditor5-editor-classic": "43.1.0", "@ckeditor/ckeditor5-core": "43.1.0", "@ckeditor/ckeditor5-engine": "43.1.0", - "@ckeditor/ckeditor5-ui": "43.1.0", "@types/lodash-es": "^4.17.12", "typescript": "5.0.4" }, From 4f16ff8662d5dd045f71c77ab8306f5c6d23fd44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksander=20Nowodzi=C5=84ski?= Date: Tue, 24 Sep 2024 09:33:31 +0200 Subject: [PATCH 12/12] Docs: Addressed an invalid use of the View class causing issues with the focus tracker. --- docs/_snippets/framework/ui/ui-toolbar-text.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/_snippets/framework/ui/ui-toolbar-text.js b/docs/_snippets/framework/ui/ui-toolbar-text.js index 6c43c3b58c7..15e386084ae 100644 --- a/docs/_snippets/framework/ui/ui-toolbar-text.js +++ b/docs/_snippets/framework/ui/ui-toolbar-text.js @@ -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 );