Skip to content

Commit

Permalink
Focus outline should remain be visible upon closing a menu bar using …
Browse files Browse the repository at this point in the history
…Esc key during keyboard navigation.
  • Loading branch information
oleq committed Aug 12, 2024
1 parent 73eea9e commit ef327bb
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
8 changes: 6 additions & 2 deletions packages/ckeditor5-ui/src/menubar/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import clickOutsideHandler from '../bindings/clickoutsidehandler.js';
import type { ButtonExecuteEvent } from '../button/button.js';
import type ComponentFactory from '../componentfactory.js';
import type { FocusableView } from '../focuscycler.js';
import type { Editor } from '@ckeditor/ckeditor5-core';
import {
logWarning,
type Locale,
Expand Down Expand Up @@ -179,7 +178,12 @@ export const MenuBarBehaviors = {

menuBarView.on<ObservableChangeEvent<boolean>>( 'change:isOpen', ( _, evt, isOpen ) => {
if ( !isOpen ) {
menuBarView.isFocusBorderEnabled = false;
// Keep the focus border if the menu bar was closed by a keyboard interaction (Esc key).
// The user remains in the keyboard navigation mode and can traverse the main categories.
// See https://github.com/ckeditor/ckeditor5/issues/16719.
if ( !isKeyPressed ) {
menuBarView.isFocusBorderEnabled = false;
}

// Reset the flag when the menu bar is closed, menu items tend to intercept `keyup` event
// and sometimes, after pressing `enter` on focused item, `isKeyPressed` stuck in `true` state.
Expand Down
41 changes: 39 additions & 2 deletions packages/ckeditor5-ui/tests/menubar/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* global document, Event, KeyboardEvent */
/* global document, Event, KeyboardEvent, MouseEvent */

import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js';
import {
Expand Down Expand Up @@ -970,7 +970,7 @@ describe( 'MenuBarView utils', () => {
it( 'should set proper isFocusBorderEnabled when a clicked and focused item on opened menu', () => {
const clock = sinon.useFakeTimers();

sinon.stub( menuBarView.element, 'matches' ).withArgs( ':focus-within' ).returns( true );
sinon.stub( menuBarView.element, 'matches' ).withArgs( ':focus-within' ).returns( true );

const menuA = getMenuByLabel( menuBarView, 'A' );

Expand All @@ -991,6 +991,43 @@ describe( 'MenuBarView utils', () => {

expect( menuBarView.isFocusBorderEnabled ).to.be.false;
} );

it( 'should not clean #isFocusBorderEnabled if the menu bar was closed by an Esc key press', async () => {
const menuA = getMenuByLabel( menuBarView, 'A' );

menuA.buttonView.focus();

menuA.element.dispatchEvent( new KeyboardEvent( 'keydown', { keyCode: keyCodes.arrowdown } ) );
await wait( 10 );
menuA.element.dispatchEvent( new KeyboardEvent( 'keyup', { keyCode: keyCodes.arrowdown } ) );

await wait( 100 );
expect( menuBarView.isFocusBorderEnabled ).to.be.true;
expect( menuBarView.isOpen ).to.be.true;

menuA.element.dispatchEvent( new KeyboardEvent( 'keydown', { keyCode: keyCodes.esc } ) );
await wait( 10 );
menuA.element.dispatchEvent( new KeyboardEvent( 'keyup', { keyCode: keyCodes.esc } ) );

expect( menuBarView.isFocusBorderEnabled ).to.be.true;
expect( menuBarView.isOpen ).to.be.false;
} );

it( 'should clean #isFocusBorderEnabled if the menu bar was closed without use of a keyboard', async () => {
const menuA = getMenuByLabel( menuBarView, 'A' );

menuA.buttonView.element.dispatchEvent( new MouseEvent( 'click' ) );

await wait( 10 );
expect( menuBarView.isFocusBorderEnabled ).to.be.false;
expect( menuBarView.isOpen ).to.be.true;

menuA.buttonView.element.dispatchEvent( new MouseEvent( 'click' ) );
await wait( 10 );

expect( menuBarView.isFocusBorderEnabled ).to.be.false;
expect( menuBarView.isOpen ).to.be.false;
} );
} );
} );

Expand Down

0 comments on commit ef327bb

Please sign in to comment.