Skip to content

Commit

Permalink
Merge pull request #16908 from ckeditor/ck/16719
Browse files Browse the repository at this point in the history
Fix (ui): The focus outline should remain visible upon closing a menu bar using the Esc key during keyboard navigation. Closes #16719.
  • Loading branch information
oleq authored Aug 26, 2024
2 parents 08b0dd8 + f109659 commit 9edf0aa
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
7 changes: 6 additions & 1 deletion packages/ckeditor5-ui/src/menubar/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,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 9edf0aa

Please sign in to comment.