Skip to content

Commit

Permalink
Merge branch 'master' into cc/6633-v2a
Browse files Browse the repository at this point in the history
  • Loading branch information
@oleq
oleq committed Oct 22, 2024
2 parents 59f1e88 + 926400f commit c106fdf
Show file tree
Hide file tree
Showing 18 changed files with 568 additions and 63 deletions.
26 changes: 17 additions & 9 deletions packages/ckeditor5-image/src/image/converters.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,6 @@ export function downcastSourcesAttribute( imageUtils: ImageUtils ): ( dispatcher
const attributeNewValue = data.attributeNewValue as null | Array<ViewElementAttributes>;

if ( attributeNewValue && attributeNewValue.length ) {
// Make sure <picture> does not break attribute elements, for instance <a> in linked images.
const pictureElement = viewWriter.createContainerElement( 'picture', null,
attributeNewValue.map( sourceAttributes => {
return viewWriter.createEmptyElement( 'source', sourceAttributes );
} )
);

// Collect all wrapping attribute elements.
const attributeElements = [];
let viewElement = imgElement.parent;
Expand All @@ -249,8 +242,23 @@ export function downcastSourcesAttribute( imageUtils: ImageUtils ): ( dispatcher
viewElement = parentElement;
}

// Insert the picture and move img into it.
viewWriter.insert( viewWriter.createPositionBefore( imgElement ), pictureElement );
const hasPictureElement = imgElement.parent!.is( 'element', 'picture' );

// Reuse existing <picture> element (ckeditor5#17192) or create a new one.
const pictureElement = hasPictureElement ? imgElement.parent : viewWriter.createContainerElement( 'picture', null );

if ( !hasPictureElement ) {
viewWriter.insert( viewWriter.createPositionBefore( imgElement ), pictureElement );
}

viewWriter.remove( viewWriter.createRangeIn( pictureElement ) );

viewWriter.insert( viewWriter.createPositionAt( pictureElement, 'end' ),
attributeNewValue.map( sourceAttributes => {
return viewWriter.createEmptyElement( 'source', sourceAttributes );
} )
);

viewWriter.move( viewWriter.createRangeOn( imgElement ), viewWriter.createPositionAt( pictureElement, 'end' ) );

// Apply collected attribute elements over the new picture element.
Expand Down
152 changes: 152 additions & 0 deletions packages/ckeditor5-image/tests/pictureediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1865,6 +1865,158 @@ describe( 'PictureEditing', () => {

expect( editor.getData() ).to.equal( '<p>foo<img src="/assets/sample.png">bar</p>' );
} );

it( 'should downcast changed "sources" attribute on an existing picture element', () => {
editor.setData(
'<figure class="image">' +
'<picture>' +
'<source srcset="">' +
'<img src="/assets/sample.png">' +
'</picture>' +
'<figcaption>Caption</figcaption>' +
'</figure>'
);

model.change( writer => {
writer.setAttribute(
'sources',
[
{
srcset: '/assets/sample2.png'
}
],
modelDocument.getRoot().getChild( 0 )
);
} );

expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false">' +
'<picture>' +
'<source srcset="/assets/sample2.png"></source>' +
'<img src="/assets/sample.png"></img>' +
'</picture>' +
'<figcaption ' +
'aria-label="Caption for the image" ' +
'class="ck-editor__editable ck-editor__nested-editable" ' +
'contenteditable="true" ' +
'data-placeholder="Enter image caption" ' +
'role="textbox" ' +
'tabindex="-1">' +
'Caption' +
'</figcaption>' +
'</figure>'
);
} );

it( 'should downcast changed "sources" attribute on an existing linked picture element', () => {
editor.setData(
'<figure class="image">' +
'<a href="https://ckeditor.com">' +
'<picture>' +
'<source srcset="/assets/sample.png">' +
'<img src="/assets/sample.png">' +
'</picture>' +
'</a>' +
'<figcaption>Caption</figcaption>' +
'</figure>'
);

model.change( writer => {
writer.setAttribute(
'sources',
[
{
srcset: '/assets/sample2.png'
}
],
modelDocument.getRoot().getChild( 0 )
);
} );

expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false">' +
'<a href="https://ckeditor.com">' +
'<picture>' +
'<source srcset="/assets/sample2.png"></source>' +
'<img src="/assets/sample.png"></img>' +
'</picture>' +
'</a>' +
'<figcaption ' +
'aria-label="Caption for the image" ' +
'class="ck-editor__editable ck-editor__nested-editable" ' +
'contenteditable="true" ' +
'data-placeholder="Enter image caption" ' +
'role="textbox" ' +
'tabindex="-1">' +
'Caption' +
'</figcaption>' +
'</figure>'
);
} );

it( 'should keep existing picture element attributes when downcasting "sources" attribute', () => {
editor.model.schema.extend( 'imageBlock', {
allowAttributes: [ 'pictureClass' ]
} );

editor.conversion.for( 'upcast' ).add( dispatcher => {
dispatcher.on( 'element:picture', ( _evt, data, conversionApi ) => {
const viewItem = data.viewItem;
const modelElement = data.modelCursor.parent;

conversionApi.writer.setAttribute( 'pictureClass', viewItem.getAttribute( 'class' ), modelElement );
} );
} );

editor.conversion.for( 'downcast' ).add( dispatcher => {
dispatcher.on( 'attribute:pictureClass:imageBlock', ( evt, data, conversionApi ) => {
const element = conversionApi.mapper.toViewElement( data.item );
const pictureElement = element.getChild( 0 );

conversionApi.writer.setAttribute( 'class', data.attributeNewValue, pictureElement );
} );
} );

editor.setData(
'<figure class="image">' +
'<picture class="test-class">' +
'<source srcset="">' +
'<img src="/assets/sample.png">' +
'</picture>' +
'<figcaption>Caption</figcaption>' +
'</figure>'
);

model.change( writer => {
writer.setAttribute(
'sources',
[
{
srcset: '/assets/sample2.png'
}
],
modelDocument.getRoot().getChild( 0 )
);
} );

expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false">' +
'<picture class="test-class">' +
'<source srcset="/assets/sample2.png"></source>' +
'<img src="/assets/sample.png"></img>' +
'</picture>' +
'<figcaption ' +
'aria-label="Caption for the image" ' +
'class="ck-editor__editable ck-editor__nested-editable" ' +
'contenteditable="true" ' +
'data-placeholder="Enter image caption" ' +
'role="textbox" ' +
'tabindex="-1">' +
'Caption' +
'</figcaption>' +
'</figure>'
);
} );
} );
} );
} );
Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-link/src/utils/automaticdecorators.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ export default class AutomaticDecorators {
const linkInImage = Array.from( viewFigure.getChildren() )
.find( ( child ): child is ViewElement => child.is( 'element', 'a' ) )!;

// It's not guaranteed that the anchor is present in the image block during execution of this dispatcher.
// It might have been removed during the execution of unlink command that runs the image link downcast dispatcher
// that is executed before this one and removes the anchor from the image block.
if ( !linkInImage ) {
return;
}

for ( const item of this._definitions ) {
const attributes = toMap( item.attributes );

Expand Down
43 changes: 43 additions & 0 deletions packages/ckeditor5-link/tests/unlinkcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import { global } from '@ckeditor/ckeditor5-utils';
import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor.js';
import UnlinkCommand from '../src/unlinkcommand.js';
import LinkEditing from '../src/linkediting.js';
import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js';
import LinkImageEditing from '../src/linkimageediting.js';
import Image from '@ckeditor/ckeditor5-image/src/image.js';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js';

describe( 'UnlinkCommand', () => {
let editor, model, document, command;
Expand Down Expand Up @@ -508,4 +512,43 @@ describe( 'UnlinkCommand', () => {
expect( getData( model ) ).to.equal( '<paragraph>[<linkableInline></linkableInline>]</paragraph>' );
} );
} );

describe( '`Image` plugin integration', () => {
let editorElement;

beforeEach( async () => {
await editor.destroy();

editorElement = global.document.body.appendChild(
global.document.createElement( 'div' )
);

return ClassicTestEditor.create( editorElement, {
extraPlugins: [ LinkEditing, LinkImageEditing, Image ],
link: {
addTargetToExternalLinks: true
}
} )
.then( newEditor => {
editor = newEditor;
model = editor.model;
document = model.document;
} );
} );

afterEach( async () => {
await editor.destroy();
editorElement.remove();
} );

it( 'should not crash during removal of external `linkHref` from `imageBlock` when `Image` plugin is present', () => {
setData( model, '[<imageBlock linkHref="url"></imageBlock>]' );

expect( () => {
editor.execute( 'unlink' );
} ).not.to.throw();

expect( getData( model ) ).to.equal( '[<imageBlock></imageBlock>]' );
} );
} );
} );
5 changes: 5 additions & 0 deletions packages/ckeditor5-theme-lark/theme/ckeditor5-ui/components/dialog/dialog.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,17 @@
max-height: var(--ck-dialog-max-height);
max-width: var(--ck-dialog-max-width);
border: 1px solid var(--ck-color-base-border);
overscroll-behavior: contain;

& .ck.ck-form__header {
border-bottom: 1px solid var(--ck-color-dialog-form-header-border);
}
}

.ck-dialog-body-scroll-locked {
overflow: hidden;
}

@keyframes ck-dialog-fade-in {
0% {
background: hsla( 0, 0%, 0%, 0 );
Expand Down
31 changes: 31 additions & 0 deletions packages/ckeditor5-ui/src/dialog/dialog.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ export default class Dialog extends Plugin {
} );
}

/**
* @inheritDoc
*/
public override destroy(): void {
super.destroy();

this._unlockBodyScroll();
}

/**
* Initiates listeners for the `show` and `hide` events emitted by this plugin.
*
Expand Down Expand Up @@ -302,6 +311,10 @@ export default class Dialog extends Plugin {
position = isModal ? DialogViewPosition.SCREEN_CENTER : DialogViewPosition.EDITOR_CENTER;
}

if ( isModal ) {
this._lockBodyScroll();
}

view.set( {
position,
_isVisible: true,
Expand Down Expand Up @@ -349,6 +362,10 @@ export default class Dialog extends Plugin {
const editor = this.editor;
const view = this.view;

if ( view.isModal ) {
this._unlockBodyScroll();
}

// Reset the content view to prevent its children from being destroyed in the standard
// View#destroy() (and collections) chain. If the content children were left in there,
// they would have to be re-created by the feature using the dialog every time the dialog
Expand All @@ -368,6 +385,20 @@ export default class Dialog extends Plugin {
this.isOpen = false;
Dialog._visibleDialogPlugin = null;
}

/**
* Makes the <body> unscrollable (e.g. when the modal shows up).
*/
private _lockBodyScroll(): void {
document.body.classList.add( 'ck-dialog-body-scroll-locked' );
}

/**
* Makes the <body> scrollable again (e.g. once the modal hides).
*/
private _unlockBodyScroll(): void {
document.body.classList.remove( 'ck-dialog-body-scroll-locked' );
}
}

/**
Expand Down
Loading

0 comments on commit c106fdf

Please sign in to comment.