Skip to content

Commit

Permalink
Refactor removing ckboxImageId logic
Browse files Browse the repository at this point in the history
  • Loading branch information
illia-stv committed Oct 13, 2023
1 parent 4de11ab commit d9074e5
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 17 deletions.
8 changes: 8 additions & 0 deletions packages/ckeditor5-ckbox/src/ckboxediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,16 @@ export default class CKBoxEditing extends Plugin {
public async init(): Promise<void> {
const editor = this.editor;
const hasConfiguration = !!editor.config.get( 'ckbox' );
const replaceImageSourceCommand = editor.commands.get( 'replaceImageSource' );
const isLibraryLoaded = !!window.CKBox;

if ( replaceImageSourceCommand ) {
// After replacing image, "ckboxImageId" attribute will be removed.
replaceImageSourceCommand.registerImageCallback( ( writer, image ) => {
writer.removeAttribute( 'ckboxImageId', image );
} );
}

// Proceed with plugin initialization only when the integrator intentionally wants to use it, i.e. when the `config.ckbox` exists or
// the CKBox JavaScript library is loaded.
if ( !hasConfiguration && !isLibraryLoaded ) {
Expand Down
20 changes: 19 additions & 1 deletion packages/ckeditor5-ckbox/tests/ckboxediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import TokenMock from '@ckeditor/ckeditor5-cloud-services/tests/_utils/tokenmock
const CKBOX_API_URL = 'https://upload.example.com';

describe( 'CKBoxEditing', () => {
let editor, model, view, originalCKBox;
let editor, model, view, originalCKBox, replaceImageSourceCommand;

testUtils.createSinonSandbox();

Expand All @@ -50,6 +50,7 @@ describe( 'CKBoxEditing', () => {
}
} );

replaceImageSourceCommand = editor.commands.get( 'replaceImageSource' );
model = editor.model;
view = editor.editing.view;
} );
Expand Down Expand Up @@ -2056,6 +2057,23 @@ describe( 'CKBoxEditing', () => {
} );
} );
} );

it( 'should remove ckboxImageId attribute on image replace', () => {
const schema = model.schema;
schema.extend( 'imageBlock', { allowAttributes: 'ckboxImageId' } );

setModelData( model, `[<imageBlock
ckboxImageId="id"
></imageBlock>]` );

const element = model.document.selection.getSelectedElement();

expect( element.getAttribute( 'ckboxImageId' ) ).to.equal( 'id' );

replaceImageSourceCommand.execute( { source: 'bar/foo.jpg' } );

expect( element.getAttribute( 'ckboxImageId' ) ).to.be.undefined;
} );
} );

function createTestEditor( config = {} ) {
Expand Down
56 changes: 42 additions & 14 deletions packages/ckeditor5-image/src/image/replaceimagesourcecommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import { Command } from 'ckeditor5/src/core';
import { Command, type Editor } from 'ckeditor5/src/core';
import type ImageUtils from '../imageutils';
import type { Writer, Element } from 'ckeditor5/src/engine';

/**
* @module image/image/replaceimagesourcecommand
Expand All @@ -22,6 +23,14 @@ import type ImageUtils from '../imageutils';
export default class ReplaceImageSourceCommand extends Command {
declare public value: string | null;

declare private _imageCallbacks: Array<( writer: Writer, image: Element ) => void>;

constructor( editor: Editor ) {
super( editor );

this._imageCallbacks = [];
}

/**
* @inheritDoc
*/
Expand All @@ -34,6 +43,15 @@ export default class ReplaceImageSourceCommand extends Command {
this.value = this.isEnabled ? element.getAttribute( 'src' ) as string : null;
}

/**
* Register callback which will be executed after command execution.
*
* @param callback Callback which will be called after command execution.
*/
public registerImageCallback( callback: ( writer: Writer, image: Element ) => void ): void {
this._imageCallbacks.push( callback );
}

/**
* Executes the command.
*
Expand All @@ -43,20 +61,30 @@ export default class ReplaceImageSourceCommand extends Command {
*/
public override execute( options: { source: string } ): void {
const image = this.editor.model.document.selection.getSelectedElement()!;
this.editor.model.change( writer => {

this.editor.model.enqueueChange( writer => {
writer.setAttribute( 'src', options.source, image );
writer.removeAttribute( 'srcset', image );
writer.removeAttribute( 'sizes', image );

/**
* In case responsive images some attributes should be cleaned up.
* Check: https://github.com/ckeditor/ckeditor5/issues/15093
*/
writer.removeAttribute( 'ckboxImageId', image );
writer.removeAttribute( 'sources', image );
writer.removeAttribute( 'width', image );
writer.removeAttribute( 'height', image );
writer.removeAttribute( 'alt', image );

this._cleanupImage( writer, image );
} );
}

/**
* Cleanup some image attributes.
*/
private _cleanupImage( writer: Writer, image: Element ) {
writer.removeAttribute( 'srcset', image );
writer.removeAttribute( 'sizes', image );

/**
* In case responsive images some attributes should be cleaned up.
* Check: https://github.com/ckeditor/ckeditor5/issues/15093
*/
writer.removeAttribute( 'sources', image );
writer.removeAttribute( 'width', image );
writer.removeAttribute( 'height', image );
writer.removeAttribute( 'alt', image );

this._imageCallbacks.forEach( callback => callback( writer, image ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ describe( 'ReplaceImageSourceCommand', () => {
const element = model.document.selection.getSelectedElement();

expect( element.getAttribute( 'src' ) ).to.equal( 'foo/bar.jpg' );
expect( element.getAttribute( 'ckboxImageId' ) ).to.equal( 'id' );
expect( element.getAttribute( 'sources' ) ).to.equal( '[{srcset:\'url\', sizes:\'100vw, 1920px\', type: \'image/webp\'}]' );
expect( element.getAttribute( 'width' ) ).to.equal( 100 );
expect( element.getAttribute( 'height' ) ).to.equal( 200 );
Expand All @@ -67,7 +66,6 @@ describe( 'ReplaceImageSourceCommand', () => {
command.execute( { source: 'bar/foo.jpg' } );

expect( element.getAttribute( 'src' ) ).to.equal( 'bar/foo.jpg' );
expect( element.getAttribute( 'ckboxImageId' ) ).to.be.undefined;
expect( element.getAttribute( 'sources' ) ).to.be.undefined;
expect( element.getAttribute( 'width' ) ).to.be.undefined;
expect( element.getAttribute( 'height' ) ).to.be.undefined;
Expand Down

0 comments on commit d9074e5

Please sign in to comment.