Skip to content

Commit

Permalink
Merge pull request #16348 from ckeditor/ck/16153
Browse files Browse the repository at this point in the history
Fix (ckbox): The image toolbar stays attached to the image after closing CKBox. Closes #16153.
  • Loading branch information
niegowski authored May 22, 2024
2 parents d442cc3 + eb569a3 commit 3790aa2
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@
*/

import { Command, PendingActions, type Editor } from 'ckeditor5/src/core.js';
import { CKEditorError, abortableDebounce, createElement, retry, type AbortableFunc } from 'ckeditor5/src/utils.js';
import {
CKEditorError,
abortableDebounce,
createElement,
retry,
delay,
type AbortableFunc
} from 'ckeditor5/src/utils.js';
import type { Element as ModelElement } from 'ckeditor5/src/engine.js';
import { Notification } from 'ckeditor5/src/ui.js';
import { isEqual } from 'lodash-es';
Expand Down Expand Up @@ -54,6 +61,15 @@ export default class CKBoxImageEditCommand extends Command {
*/
private _prepareOptions: AbortableFunc<[ ProcessingState ], Promise<Record<string, unknown>>>;

/**
* CKBox's onClose function runs before the final cleanup, potentially causing
* page layout changes after it finishes. To address this, we use a setTimeout hack
* to ensure that floating elements on the page maintain their correct position.
*
* See: https://github.com/ckeditor/ckeditor5/issues/16153.
*/
private _updateUiDelayed = delay( () => this.editor.ui.update(), 0 );

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -130,6 +146,8 @@ export default class CKBoxImageEditCommand extends Command {

this._prepareOptions.abort();

this._updateUiDelayed.cancel();

for ( const state of this._processInProgress.values() ) {
state.controller.abort();
}
Expand Down Expand Up @@ -234,6 +252,8 @@ export default class CKBoxImageEditCommand extends Command {

this.editor.editing.view.focus();

this._updateUiDelayed();

this.refresh();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,69 @@ describe( 'CKBoxImageEditCommand', () => {
expect( command.value ).to.be.false;
sinon.assert.calledOnce( refreshSpy );
} );

it( 'should update ui after closing the CKBox Image Editor dialog', async () => {
const ckboxImageId = 'example-id';
const clock = sinon.useFakeTimers();

setModelData( model,
`[<imageBlock alt="alt text" ckboxImageId="${ ckboxImageId }" src="/assets/sample.png"></imageBlock>]`
);

const imageElement = editor.model.document.selection.getSelectedElement();

const options = await command._prepareOptions( {
element: imageElement,
ckboxImageId,
controller: new AbortController()
} );

const updateUISpy = testUtils.sinon.spy( editor.ui, 'update' );

expect( command.value ).to.be.false;

command.execute();
expect( command.value ).to.be.true;

options.onClose();

await clock.tickAsync( 10 );

expect( command.value ).to.be.false;
sinon.assert.calledOnce( updateUISpy );
clock.restore();
} );

it( 'should clear timer on editor destroy', async () => {
const ckboxImageId = 'example-id';

setModelData( model,
`[<imageBlock alt="alt text" ckboxImageId="${ ckboxImageId }" src="/assets/sample.png"></imageBlock>]`
);

const imageElement = editor.model.document.selection.getSelectedElement();

const options = await command._prepareOptions( {
element: imageElement,
ckboxImageId,
controller: new AbortController()
} );

const clearTimeoutSpy = sinon.spy( command._updateUiDelayed, 'cancel' );

editor.fire( 'ready' );

expect( command.value ).to.be.false;

command.execute();

options.onClose();

command.destroy();
sinon.assert.calledTwice( clearTimeoutSpy );

expect( command.value ).to.be.false;
} );
} );

describe( 'saving edited asset', () => {
Expand Down

0 comments on commit 3790aa2

Please sign in to comment.