Add missing focus tracking after focusing menu using alt+f9

ckeditor · Jun 26, 2024 · c8c82cd · c8c82cd
1 parent 7d7b3d9
commit c8c82cd
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 14 deletions.
diff --git a/packages/ckeditor5-ui/src/menubar/menubarview.ts b/packages/ckeditor5-ui/src/menubar/menubarview.ts
@@ -56,7 +56,8 @@ export default class MenuBarView extends View implements FocusableView {
 	/**
 	 * Indicates whether the menu bar has been interacted with using the keyboard.
 	 *
-	 * It is useful for showing focus outlines only when the keyboard is used.
+	 * It is useful for showing focus outlines while hovering over the menu bar when
+	 * interaction with the keyboard was detected.
 	 *
 	 * @observable
 	 */

diff --git a/packages/ckeditor5-ui/src/menubar/utils.ts b/packages/ckeditor5-ui/src/menubar/utils.ts
@@ -56,24 +56,28 @@ export const MenuBarBehaviors = {
 	 */
 	toggleMenusAndFocusItemsOnHover( menuBarView: MenuBarView ): void {
 		menuBarView.on<MenuBarMenuMouseEnterEvent>( 'menu:mouseenter', evt => {
-			// This works only when the menu bar has already been open and the user hover over the menu bar.
-			if ( !menuBarView.isOpen ) {
+			// This behavior should be activated when one of condition is present:
+			// 1. The user opened any submenu of menubar and hover over items in the menu bar.
+			// 2. The user focused whole menubar using keyboard interaction and enabled focus borders and hover over items in the menu bar.
+			if ( !menuBarView.isFocusBorderEnabled && !menuBarView.isOpen ) {
 				return;
 			}
 
-			for ( const menuView of menuBarView.menus ) {
-				// @if CK_DEBUG_MENU_BAR // const wasOpen = menuView.isOpen;
+			if ( menuBarView.isOpen ) {
+				for ( const menuView of menuBarView.menus ) {
+					// @if CK_DEBUG_MENU_BAR // const wasOpen = menuView.isOpen;
 
-				const pathLeaf = evt.path[ 0 ];
-				const isListItemContainingMenu = pathLeaf instanceof MenuBarMenuListItemView && pathLeaf.children.first === menuView;
+					const pathLeaf = evt.path[ 0 ];
+					const isListItemContainingMenu = pathLeaf instanceof MenuBarMenuListItemView && pathLeaf.children.first === menuView;
 
-				menuView.isOpen = ( evt.path.includes( menuView ) || isListItemContainingMenu ) && menuView.isEnabled;
+					menuView.isOpen = ( evt.path.includes( menuView ) || isListItemContainingMenu ) && menuView.isEnabled;
 
-				// @if CK_DEBUG_MENU_BAR // if ( wasOpen !== menuView.isOpen ) {
-				// @if CK_DEBUG_MENU_BAR // console.log( '[BEHAVIOR] toggleMenusAndFocusItemsOnHover(): Toggle',
-				// @if CK_DEBUG_MENU_BAR // 	logMenu( menuView ), 'isOpen', menuView.isOpen
-				// @if CK_DEBUG_MENU_BAR // );
-				// @if CK_DEBUG_MENU_BAR // }
+					// @if CK_DEBUG_MENU_BAR // if ( wasOpen !== menuView.isOpen ) {
+					// @if CK_DEBUG_MENU_BAR // console.log( '[BEHAVIOR] toggleMenusAndFocusItemsOnHover(): Toggle',
+					// @if CK_DEBUG_MENU_BAR // 	logMenu( menuView ), 'isOpen', menuView.isOpen
+					// @if CK_DEBUG_MENU_BAR // );
+					// @if CK_DEBUG_MENU_BAR // }
+				}
 			}
 
 			( evt.source as FocusableView ).focus();
@@ -182,6 +186,9 @@ export const MenuBarBehaviors = {
 			}
 		} );
 
+		// After clicking menu bar list item the focus is moved to the newly opened submenu.
+		// We need to enable focus border for the submenu items because after pressing arrow down it will
+		// focus second item instead of first which is not super intuitive.
 		menuBarView.listenTo( menuBarView.element!, 'click', () => {
 			if ( menuBarView.isOpen && menuBarView.element!.matches( ':focus-within' ) ) {
 				menuBarView.isFocusBorderEnabled = true;
@@ -197,7 +204,7 @@ export const MenuBarBehaviors = {
 		}, { useCapture: true } );
 
 		menuBarView.listenTo( menuBarView.element!, 'focus', () => {
-			if ( isKeyPressed && !menuBarView.isFocusBorderEnabled ) {
+			if ( isKeyPressed ) {
 				menuBarView.isFocusBorderEnabled = true;
 			}
 		}, { useCapture: true } );

diff --git a/packages/ckeditor5-ui/tests/menubar/utils.js b/packages/ckeditor5-ui/tests/menubar/utils.js
@@ -150,6 +150,46 @@ describe( 'MenuBarView utils', () => {
 				);
 			} );
 
+			it( 'should focus hovered menu if `isFocusBorderEnabled`', () => {
+				menuBarView.isFocusBorderEnabled = true;
+
+				getMenuByLabel( menuBarView, 'A' ).buttonView.fire( 'mouseenter' );
+				expect( barDump( menuBarView ) ).to.deep.equal(
+					[
+						{
+							label: 'A', isOpen: false, isFocused: true,
+							items: []
+						},
+						{
+							label: 'B', isOpen: false, isFocused: false,
+							items: []
+						},
+						{
+							label: 'C', isOpen: false, isFocused: false,
+							items: []
+						}
+					]
+				);
+
+				getMenuByLabel( menuBarView, 'B' ).buttonView.fire( 'mouseenter' );
+				expect( barDump( menuBarView ) ).to.deep.equal(
+					[
+						{
+							label: 'A', isOpen: false, isFocused: false,
+							items: []
+						},
+						{
+							label: 'B', isOpen: false, isFocused: true,
+							items: []
+						},
+						{
+							label: 'C', isOpen: false, isFocused: false,
+							items: []
+						}
+					]
+				);
+			} );
+
 			describe( 'if the bar is already open', () => {
 				it( 'should toggle menus and move focus while moving the mouse (top-level menu -> top-level menu)', () => {
 					const menuA = getMenuByLabel( menuBarView, 'A' );