From 3b4b925288c78b6f041ea0272775599f52eb40b5 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Wed, 2 Aug 2023 15:39:51 +0200 Subject: [PATCH 1/3] Fix (ckbox): set focus on ckbox dialog after initialization --- packages/ckeditor5-ckbox/src/ckboxcommand.ts | 21 +++++++++++++++++++ .../ckeditor5-ckbox/tests/ckboxcommand.js | 20 ++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/packages/ckeditor5-ckbox/src/ckboxcommand.ts b/packages/ckeditor5-ckbox/src/ckboxcommand.ts index f134f24ed71..7008265abd3 100644 --- a/packages/ckeditor5-ckbox/src/ckboxcommand.ts +++ b/packages/ckeditor5-ckbox/src/ckboxcommand.ts @@ -177,6 +177,8 @@ export default class CKBoxCommand extends Command { document.body.appendChild( this._wrapper ); window.CKBox.mount( this._wrapper, this._prepareOptions() ); + + focusDialog( this._wrapper ); } ); // Handle closing of the CKBox dialog. @@ -387,6 +389,25 @@ function getAssetUrl( asset: CKBoxRawAssetDefinition ) { return url.toString(); } +/** + * Focuses the CKBox dialog. + * + * @param wrapper The element in DOM which wrap CKBox component. + */ +function focusDialog( wrapper: Element | null ) { + // If the DOM is not fully loaded and rendered by the time the focus() method is invoked, + // the element might not get focused. So we call the focus() when the thread becomes idle. + setTimeout( () => { + // Wrapper cannot be focused by default because it's covered by it's children entirely. + // Then we getting to the nearest focusable child( dialog container ) inside wrapper. + const dialogContainer = wrapper && wrapper.children[ 0 ] && wrapper.children[ 0 ].children[ 0 ]; + + if ( dialogContainer instanceof HTMLElement ) { + dialogContainer.focus(); + } + } ); +} + /** * Fired when the command is executed, the dialog is closed or the assets are chosen. * diff --git a/packages/ckeditor5-ckbox/tests/ckboxcommand.js b/packages/ckeditor5-ckbox/tests/ckboxcommand.js index 92889694b63..a1869104694 100644 --- a/packages/ckeditor5-ckbox/tests/ckboxcommand.js +++ b/packages/ckeditor5-ckbox/tests/ckboxcommand.js @@ -159,6 +159,26 @@ describe( 'CKBoxCommand', () => { expect( document.body.appendChild.args[ 0 ][ 0 ] ).to.equal( wrapper ); } ); + it( 'should focus dialog after initialization', () => { + const wrapper = document.createElement( 'div' ); + const resetContainer = document.createElement( 'div' ); + const ckboxDialogContainer = document.createElement( 'div' ); + + wrapper.appendChild( resetContainer ); + resetContainer.appendChild( ckboxDialogContainer ); + + Object.defineProperty( command, '_wrapper', { + get: sinon.stub().returns( wrapper ), + set: sinon.stub().callsFake( value => value ) + } ); + + const spy = sinon.spy( ckboxDialogContainer, 'focus' ); + command.execute(); + + spy.calledOnce; + sinon.restore(); + } ); + it( 'should create and mount a wrapper only once', () => { command.execute(); From 64f5d88d6a167998a2b8876cf31b1b091f61f146 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 8 Aug 2023 20:32:26 +0200 Subject: [PATCH 2/3] Fix (ckbox): implement function which focus first element in ckbox gallery --- packages/ckeditor5-ckbox/src/ckboxcommand.ts | 49 ++++++++--- .../ckeditor5-ckbox/tests/ckboxcommand.js | 87 ++++++++++++++++--- 2 files changed, 111 insertions(+), 25 deletions(-) diff --git a/packages/ckeditor5-ckbox/src/ckboxcommand.ts b/packages/ckeditor5-ckbox/src/ckboxcommand.ts index 7008265abd3..adbb396a953 100644 --- a/packages/ckeditor5-ckbox/src/ckboxcommand.ts +++ b/packages/ckeditor5-ckbox/src/ckboxcommand.ts @@ -178,7 +178,9 @@ export default class CKBoxCommand extends Command { window.CKBox.mount( this._wrapper, this._prepareOptions() ); - focusDialog( this._wrapper ); + const MAX_NUMBER_OF_ATTEMPTS_TO_FOCUS = 50; + + focusCKBoxItem( MAX_NUMBER_OF_ATTEMPTS_TO_FOCUS ); } ); // Handle closing of the CKBox dialog. @@ -390,22 +392,45 @@ function getAssetUrl( asset: CKBoxRawAssetDefinition ) { } /** - * Focuses the CKBox dialog. + * Focuses the CKBox first item in gallery. + * This is a temporary fix. A permanent solution to this issue will be provided soon. * - * @param wrapper The element in DOM which wrap CKBox component. + * @param limiter Max number of attempts to focus the ckbox item. */ -function focusDialog( wrapper: Element | null ) { - // If the DOM is not fully loaded and rendered by the time the focus() method is invoked, - // the element might not get focused. So we call the focus() when the thread becomes idle. +function focusCKBoxItem( limiter: number ): void { + // Trying every 100 ms get access to the CKBox component until component will be loaded. setTimeout( () => { - // Wrapper cannot be focused by default because it's covered by it's children entirely. - // Then we getting to the nearest focusable child( dialog container ) inside wrapper. - const dialogContainer = wrapper && wrapper.children[ 0 ] && wrapper.children[ 0 ].children[ 0 ]; + if ( limiter === 0 ) { + return; + } + + const ckboxGallery = document.getElementsByClassName( 'ckbox-gallery' )[ 0 ]; + // In case there is no items, "upload button" will be appeared in "div" with + // classname ".ckbox-empty-view". + const uploadButton = document.querySelector( '.ckbox-empty-view .ckbox-btn' ); + + // In case "upload button" is loaded in ".ckbox-empty-view" we focus actual button. + if ( uploadButton && uploadButton instanceof HTMLElement ) { + uploadButton.focus(); + return; + } - if ( dialogContainer instanceof HTMLElement ) { - dialogContainer.focus(); + if ( !ckboxGallery ) { + focusCKBoxItem( limiter - 1 ); + return; + } + + const firstItem = ckboxGallery.children[ 0 ]; + + if ( firstItem && + firstItem.className === 'ckbox-gallery-item' && + firstItem instanceof HTMLElement + ) { + firstItem.focus(); + } else { + focusCKBoxItem( limiter - 1 ); } - } ); + }, 100 ); } /** diff --git a/packages/ckeditor5-ckbox/tests/ckboxcommand.js b/packages/ckeditor5-ckbox/tests/ckboxcommand.js index a1869104694..217824a076a 100644 --- a/packages/ckeditor5-ckbox/tests/ckboxcommand.js +++ b/packages/ckeditor5-ckbox/tests/ckboxcommand.js @@ -148,6 +148,14 @@ describe( 'CKBoxCommand', () => { describe( 'events', () => { describe( 'opening dialog ("ckbox:open")', () => { + beforeEach( () => { + sinon.useFakeTimers( { now: Date.now() } ); + } ); + + afterEach( () => { + sinon.restore(); + } ); + it( 'should create a wrapper if it is not yet created and mount it in the document body', () => { command.execute(); @@ -159,24 +167,77 @@ describe( 'CKBoxCommand', () => { expect( document.body.appendChild.args[ 0 ][ 0 ] ).to.equal( wrapper ); } ); - it( 'should focus dialog after initialization', () => { - const wrapper = document.createElement( 'div' ); - const resetContainer = document.createElement( 'div' ); - const ckboxDialogContainer = document.createElement( 'div' ); + it( 'should focus ckbox gallery item after initialization', () => { + const ckboxGallery = document.createElement( 'div' ); + const ckboxGalleryItem = document.createElement( 'div' ); - wrapper.appendChild( resetContainer ); - resetContainer.appendChild( ckboxDialogContainer ); + ckboxGallery.setAttribute( 'class', 'ckbox-gallery' ); + ckboxGalleryItem.setAttribute( 'class', 'ckbox-gallery-item' ); - Object.defineProperty( command, '_wrapper', { - get: sinon.stub().returns( wrapper ), - set: sinon.stub().callsFake( value => value ) - } ); + ckboxGallery.appendChild( ckboxGalleryItem ); + document.body.append( ckboxGallery ); - const spy = sinon.spy( ckboxDialogContainer, 'focus' ); + const spy = sinon.spy( ckboxGalleryItem, 'focus' ); command.execute(); - spy.calledOnce; - sinon.restore(); + sinon.clock.tick( 100 ); + expect( spy.calledOnce ).to.be.true; + ckboxGallery.remove(); + } ); + + it( 'should focus upload button item after initialization', () => { + const emptyView = document.createElement( 'div' ); + const uploadButton = document.createElement( 'button' ); + + emptyView.setAttribute( 'class', 'ckbox-empty-view' ); + uploadButton.setAttribute( 'class', 'ckbox-btn' ); + + emptyView.appendChild( uploadButton ); + document.body.append( emptyView ); + + const spy = sinon.spy( uploadButton, 'focus' ); + command.execute(); + + sinon.clock.tick( 100 ); + expect( spy.calledOnce ).to.be.true; + emptyView.remove(); + } ); + + it( 'should wait until ckbox will be loaded', () => { + const ckboxGallery = document.createElement( 'div' ); + const ckboxGalleryItem = document.createElement( 'div' ); + + ckboxGallery.setAttribute( 'class', 'ckbox-gallery' ); + ckboxGalleryItem.setAttribute( 'class', 'ckbox-gallery-item' ); + + const spy = sinon.spy( ckboxGalleryItem, 'focus' ); + command.execute(); + + sinon.clock.tick( 100 ); + expect( spy.notCalled ).to.be.true; + + document.body.append( ckboxGallery ); + + sinon.clock.tick( 100 ); + expect( spy.notCalled ).to.be.true; + + ckboxGallery.appendChild( ckboxGalleryItem ); + sinon.clock.tick( 100 ); + + expect( spy.calledOnce ).to.be.true; + ckboxGallery.remove(); + } ); + + it( 'should giveup after 5 seconds', () => { + const ckboxGalleryItem = document.createElement( 'div' ); + + ckboxGalleryItem.setAttribute( 'class', 'ckbox-gallery-item' ); + + const spy = sinon.spy( ckboxGalleryItem, 'focus' ); + command.execute(); + + sinon.clock.tick( 5100 ); + expect( spy.notCalled ).to.be.true; } ); it( 'should create and mount a wrapper only once', () => { From 87305f3c9d09ec2b08f4e70bfd3a02ee3ae2219d Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Wed, 9 Aug 2023 14:59:20 +0200 Subject: [PATCH 3/3] Fix (ckbox): tiny improvements --- packages/ckeditor5-ckbox/src/ckboxcommand.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-ckbox/src/ckboxcommand.ts b/packages/ckeditor5-ckbox/src/ckboxcommand.ts index adbb396a953..dc958c0d80a 100644 --- a/packages/ckeditor5-ckbox/src/ckboxcommand.ts +++ b/packages/ckeditor5-ckbox/src/ckboxcommand.ts @@ -404,7 +404,7 @@ function focusCKBoxItem( limiter: number ): void { return; } - const ckboxGallery = document.getElementsByClassName( 'ckbox-gallery' )[ 0 ]; + const ckboxGalleryFirstItem = document.querySelector( '.ckbox-gallery .ckbox-gallery-item' ); // In case there is no items, "upload button" will be appeared in "div" with // classname ".ckbox-empty-view". const uploadButton = document.querySelector( '.ckbox-empty-view .ckbox-btn' ); @@ -415,18 +415,8 @@ function focusCKBoxItem( limiter: number ): void { return; } - if ( !ckboxGallery ) { - focusCKBoxItem( limiter - 1 ); - return; - } - - const firstItem = ckboxGallery.children[ 0 ]; - - if ( firstItem && - firstItem.className === 'ckbox-gallery-item' && - firstItem instanceof HTMLElement - ) { - firstItem.focus(); + if ( ckboxGalleryFirstItem && ckboxGalleryFirstItem instanceof HTMLElement ) { + ckboxGalleryFirstItem.focus(); } else { focusCKBoxItem( limiter - 1 ); }