Skip to content

Commit

Permalink
Merge branch 'master' into ck/epic/1944-bookmark
Browse files Browse the repository at this point in the history
  • Loading branch information
pszczesniak committed Oct 17, 2024
2 parents 8a56943 + 6257c78 commit 4132b53
Show file tree
Hide file tree
Showing 14 changed files with 339 additions and 73 deletions.
26 changes: 17 additions & 9 deletions packages/ckeditor5-image/src/image/converters.ts
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
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
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
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>]' );
} );
} );
} );
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="ck ck-content" id="snippet-media-embed-preview">
<h3>Follow us on Twitter for the latest news:</h3>
<h3>Follow us on X for the latest news:</h3>
<figure class="media">
<oembed url="https://twitter.com/ckeditor/status/1441013225554948096"></oembed>
<oembed url="https://x.com/ckeditor/status/1441013225554948096"></oembed>
</figure>
<h3>And check our latest posts on Facebook:</h3>
<figure class="media">
Expand All @@ -21,4 +21,4 @@ <h3>And check our latest posts on Facebook:</h3>
input.example-input {
min-width: 500px;
}
</style>
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ <h3>Vimeo</h3>
<oembed url="https://vimeo.com/1084537"></oembed>
</figure>

<h3>Twitter</h3>
<h3>X (form. Twitter)</h3>

<figure>
<oembed url="https://twitter.com/ckeditor/status/1430561610494627841"></oembed>
<oembed url="https://x.com/ckeditor/status/1430561610494627841"></oembed>
</figure>

<h3>Google Maps</h3>
Expand Down
12 changes: 6 additions & 6 deletions packages/ckeditor5-media-embed/docs/features/media-embed.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ Optionally, by setting `mediaEmbed.previewsInData` to `true` you can configure t
</figure>
```

Currently, the preview is only available for content providers for which CKEditor&nbsp;5 can predict the `<iframe>` code: YouTube, Vimeo, Dailymotion, Spotify, etc. For other providers like Twitter (X) or Instagram, the editor cannot produce an `<iframe>` code. It also does not allow retrieving this code from an external oEmbed service. Therefore, for non-previewable media, it produces the default semantic output:
Currently, the preview is only available for content providers for which CKEditor&nbsp;5 can predict the `<iframe>` code: YouTube, Vimeo, Dailymotion, Spotify, etc. For other providers like X (Twitter) or Instagram, the editor cannot produce an `<iframe>` code. It also does not allow retrieving this code from an external oEmbed service. Therefore, for non-previewable media, it produces the default semantic output:

```html
<figure class="media">
Expand Down Expand Up @@ -250,11 +250,11 @@ First, having [secured the API key](https://iframely.com/docs/allow-origins), lo

#### Semantic data

You can convert all `<oembed>` elements like the following Twitter (X) post produced by CKEditor&nbsp;5:
You can convert all `<oembed>` elements like the following X (Twitter) post produced by CKEditor&nbsp;5:

```html
<figure class="media">
<oembed url="https://twitter.com/ckeditor/status/1021777799844126720"></oembed>
<oembed url="https://x.com/ckeditor/status/1021777799844126720"></oembed>
</figure>
```

Expand All @@ -274,7 +274,7 @@ When you configure the feature to [include media previews](#including-previews-i

```html
<figure class="media">
<div data-oembed-url="https://twitter.com/ckeditor/status/1021777799844126720">
<div data-oembed-url="https://x.com/ckeditor/status/1021777799844126720">
[Media preview]
</div>
</figure>
Expand Down Expand Up @@ -316,7 +316,7 @@ You can convert `<oembed>` elements like the following Twitter (X) post produced

```html
<figure class="media">
<oembed url="https://twitter.com/ckeditor/status/1021777799844126720"></oembed>
<oembed url="https://x.com/ckeditor/status/1021777799844126720"></oembed>
</figure>
```

Expand Down Expand Up @@ -377,7 +377,7 @@ If the automatic embedding was unexpected, for instance when the link was meant

## Styling media in the editor content

While the editor comes with default styles for popular media providers like Facebook, Instagram or Twitter, you can create additional styles for non-previewable media in your editor content to help users identify them.
While the editor comes with default styles for popular media providers like Facebook, Instagram or X, you can create additional styles for non-previewable media in your editor content to help users identify them.

### Styling non-previewable media

Expand Down
9 changes: 3 additions & 6 deletions scripts/release/deploycdn.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,9 @@ const tasks = new Listr( [
}
], getListrOptions( cliArguments ) );

( async () => {
try {
await tasks.run();
} catch ( err ) {
tasks.run()
.catch( err => {
process.exitCode = 1;

console.error( err );
}
} )();
} );
Loading

0 comments on commit 4132b53

Please sign in to comment.