Browse Source

Code refactoring.

Aleksander Nowodzinski 7 years ago
parent
commit
a4030d52e0

+ 12 - 19
packages/ckeditor5-media-embed/src/converters.js

@@ -9,7 +9,7 @@
 
 
 import ViewRange from '@ckeditor/ckeditor5-engine/src/view/range';
 import ViewRange from '@ckeditor/ckeditor5-engine/src/view/range';
 import first from '@ckeditor/ckeditor5-utils/src/first';
 import first from '@ckeditor/ckeditor5-utils/src/first';
-import { addMediaWrapperElementToFigure } from './utils';
+import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position';
 
 
 /**
 /**
  * Returns a function that converts the media wrapper view representation:
  * Returns a function that converts the media wrapper view representation:
@@ -34,7 +34,10 @@ export function viewFigureToModel() {
 		}
 		}
 
 
 		// Find a div wrapper element inside the figure element.
 		// Find a div wrapper element inside the figure element.
-		const viewWrapper = Array.from( data.viewItem.getChildren() ).find( viewChild => viewChild.is( 'div' ) );
+		const viewWrapper = Array.from( data.viewItem.getChildren() )
+			.find( viewChild => {
+				return viewChild.is( 'div' );
+			} );
 
 
 		// Do not convert if the div wrapper element is absent, is missing data-oembed-url attribute or was already converted.
 		// Do not convert if the div wrapper element is absent, is missing data-oembed-url attribute or was already converted.
 		if ( !viewWrapper ||
 		if ( !viewWrapper ||
@@ -63,7 +66,11 @@ export function viewFigureToModel() {
 }
 }
 
 
 export function modelToViewUrlAttributeConverter( mediaRegistry, options ) {
 export function modelToViewUrlAttributeConverter( mediaRegistry, options ) {
-	const renderMediaHtml = options.isViewPipeline || options.renderMediaHtml;
+	const mediaViewElementOptions = {
+		useSemanticWrapper: options.semanticDataOutput,
+		renderContent: !options.semanticDataOutput,
+		isViewPipeline: options.isViewPipeline
+	};
 
 
 	return dispatcher => {
 	return dispatcher => {
 		dispatcher.on( 'attribute:url:media', converter );
 		dispatcher.on( 'attribute:url:media', converter );
@@ -81,22 +88,8 @@ export function modelToViewUrlAttributeConverter( mediaRegistry, options ) {
 		// TODO: removing it and creating it from scratch is a hack. We can do better than that.
 		// TODO: removing it and creating it from scratch is a hack. We can do better than that.
 		viewWriter.remove( ViewRange.createIn( figure ) );
 		viewWriter.remove( ViewRange.createIn( figure ) );
 
 
-		const wrapperAttributes = {};
-		const useSemanticWrapper = !renderMediaHtml ||
-			( options.renderMediaHtml && !mediaRegistry.hasRenderer( url ) );
-
-		if ( url !== null ) {
-			wrapperAttributes[ renderMediaHtml ? 'data-oembed-url' : 'url' ] = url;
-		}
-
-		if ( options.isViewPipeline ) {
-			wrapperAttributes.class = 'ck-media__wrapper';
-		}
+		const mediaViewElement = mediaRegistry.getMediaViewElement( viewWriter, url, mediaViewElementOptions );
 
 
-		addMediaWrapperElementToFigure( viewWriter, figure, {
-			mediaHtml: renderMediaHtml ? mediaRegistry.getHtml( url ) : null,
-			attributes: wrapperAttributes,
-			useSemanticWrapper
-		} );
+		viewWriter.insert( ViewPosition.createAt( figure ), mediaViewElement );
 	}
 	}
 }
 }

+ 19 - 13
packages/ckeditor5-media-embed/src/mediaembedediting.js

@@ -129,8 +129,7 @@ export default class MediaEmbedEditing extends Plugin {
 		const t = editor.t;
 		const t = editor.t;
 		const conversion = editor.conversion;
 		const conversion = editor.conversion;
 		const semanticDataOutput = editor.config.get( 'mediaEmbed.semanticDataOutput' );
 		const semanticDataOutput = editor.config.get( 'mediaEmbed.semanticDataOutput' );
-
-		this.mediaRegistry = new MediaRegistry( this.editor );
+		const mediaRegistry = this.mediaRegistry = new MediaRegistry( this.editor );
 
 
 		editor.commands.add( 'insertMedia', new InsertMediaCommand( editor ) );
 		editor.commands.add( 'insertMedia', new InsertMediaCommand( editor ) );
 
 
@@ -148,31 +147,38 @@ export default class MediaEmbedEditing extends Plugin {
 			view: ( modelElement, viewWriter ) => {
 			view: ( modelElement, viewWriter ) => {
 				const url = modelElement.getAttribute( 'url' );
 				const url = modelElement.getAttribute( 'url' );
 
 
-				return createMediaFigureElement( viewWriter, {
-					useSemanticWrapper: semanticDataOutput || !this.mediaRegistry.hasRenderer( url )
+				return createMediaFigureElement( viewWriter, mediaRegistry, url, {
+					useSemanticWrapper: semanticDataOutput || !url,
+					renderContent: !semanticDataOutput
 				} );
 				} );
 			}
 			}
 		} ) );
 		} ) );
 
 
 		// Model -> Data (url -> data-oembed-url)
 		// Model -> Data (url -> data-oembed-url)
-		conversion.for( 'dataDowncast' ).add( modelToViewUrlAttributeConverter( this.mediaRegistry, {
-			renderMediaHtml: !semanticDataOutput,
-		} ) );
+		conversion.for( 'dataDowncast' ).add(
+			modelToViewUrlAttributeConverter( mediaRegistry, {
+				semanticDataOutput
+			} ) );
 
 
 		// Model -> View (element)
 		// Model -> View (element)
 		conversion.for( 'editingDowncast' ).add( downcastElementToElement( {
 		conversion.for( 'editingDowncast' ).add( downcastElementToElement( {
 			model: 'media',
 			model: 'media',
 			view: ( modelElement, viewWriter ) => {
 			view: ( modelElement, viewWriter ) => {
-				const figure = createMediaFigureElement( viewWriter );
+				const url = modelElement.getAttribute( 'url' );
+				const figure = createMediaFigureElement( viewWriter, mediaRegistry, url, {
+					isViewPipeline: true,
+					renderContent: true
+				} );
 
 
 				return toMediaWidget( figure, viewWriter, t( 'media widget' ) );
 				return toMediaWidget( figure, viewWriter, t( 'media widget' ) );
 			}
 			}
 		} ) );
 		} ) );
 
 
 		// Model -> View (url -> data-oembed-url)
 		// Model -> View (url -> data-oembed-url)
-		conversion.for( 'editingDowncast' ).add( modelToViewUrlAttributeConverter( this.mediaRegistry, {
-			isViewPipeline: true
-		} ) );
+		conversion.for( 'editingDowncast' ).add(
+			modelToViewUrlAttributeConverter( mediaRegistry, {
+				isViewPipeline: true
+			} ) );
 
 
 		// View -> Model (data-oembed-url -> url)
 		// View -> Model (data-oembed-url -> url)
 		conversion.for( 'upcast' )
 		conversion.for( 'upcast' )
@@ -187,7 +193,7 @@ export default class MediaEmbedEditing extends Plugin {
 				model: ( viewMedia, modelWriter ) => {
 				model: ( viewMedia, modelWriter ) => {
 					const url = viewMedia.getAttribute( 'url' );
 					const url = viewMedia.getAttribute( 'url' );
 
 
-					if ( this.mediaRegistry.has( url ) ) {
+					if ( mediaRegistry.has( url ) ) {
 						return modelWriter.createElement( 'media', { url } );
 						return modelWriter.createElement( 'media', { url } );
 					}
 					}
 				}
 				}
@@ -203,7 +209,7 @@ export default class MediaEmbedEditing extends Plugin {
 				model: ( viewMedia, modelWriter ) => {
 				model: ( viewMedia, modelWriter ) => {
 					const url = viewMedia.getAttribute( 'data-oembed-url' );
 					const url = viewMedia.getAttribute( 'data-oembed-url' );
 
 
-					if ( this.mediaRegistry.has( url ) ) {
+					if ( mediaRegistry.has( url ) ) {
 						return modelWriter.createElement( 'media', { url } );
 						return modelWriter.createElement( 'media', { url } );
 					}
 					}
 				}
 				}

+ 49 - 18
packages/ckeditor5-media-embed/src/mediaregistry.js

@@ -15,26 +15,17 @@ export class MediaRegistry {
 		return !!this._getMedia( url );
 		return !!this._getMedia( url );
 	}
 	}
 
 
-	hasRenderer( url ) {
-		if ( !url ) {
-			return null;
-		}
-
-		const media = this._getMedia( url );
-
-		return !!media.renderer;
-	}
+	getMediaViewElement( writer, url, options ) {
+		let media;
 
 
-	getHtml( url, options ) {
-		if ( !url ) {
-			return null;
+		if ( url ) {
+			media = this._getMedia( url );
+		} else {
+			// Generate a view for a renderer–less media.
+			media = new Media( this.editor.t, url );
 		}
 		}
 
 
-		const media = this._getMedia( url );
-
-		if ( media ) {
-			return media.getHtml( options );
-		}
+		return media.getViewElement( writer, options );
 	}
 	}
 
 
 	_getMedia( url ) {
 	_getMedia( url ) {
@@ -68,10 +59,50 @@ class Media {
 		this.renderer = renderer;
 		this.renderer = renderer;
 	}
 	}
 
 
-	getHtml() {
+	getViewElement( writer, options ) {
+		let renderFunction;
+
+		if ( options.renderContent ) {
+			const mediaHtml = this._getContentHtml();
+
+			renderFunction = function( domDocument ) {
+				const domElement = this.toDomElement( domDocument );
+
+				domElement.innerHTML = mediaHtml;
+
+				return domElement;
+			};
+		}
+
+		const attributes = {};
+
+		if ( options.useSemanticWrapper ) {
+			if ( this.url ) {
+				attributes.url = this.url;
+			}
+
+			return writer.createEmptyElement( 'oembed', attributes, renderFunction );
+		} else {
+			if ( this.url ) {
+				attributes[ 'data-oembed-url' ] = this.url;
+			}
+
+			if ( options.isViewPipeline ) {
+				attributes.class = 'ck-media__wrapper';
+			}
+
+			return writer.createUIElement( 'div', attributes, renderFunction );
+		}
+	}
+
+	_getContentHtml() {
 		if ( this.renderer ) {
 		if ( this.renderer ) {
 			return this.renderer( this.match.pop() );
 			return this.renderer( this.match.pop() );
 		} else {
 		} else {
+			if ( !this.url ) {
+				return '';
+			}
+
 			return this._getPlaceholderHtml();
 			return this._getPlaceholderHtml();
 		}
 		}
 	}
 	}

+ 2 - 26
packages/ckeditor5-media-embed/src/utils.js

@@ -35,7 +35,7 @@ export function toMediaWidget( viewElement, writer, label ) {
 // @private
 // @private
 // @param {module:engine/view/writer~Writer} writer
 // @param {module:engine/view/writer~Writer} writer
 // @returns {module:engine/view/containerelement~ContainerElement}
 // @returns {module:engine/view/containerelement~ContainerElement}
-export function createMediaFigureElement( writer, options = {} ) {
+export function createMediaFigureElement( writer, mediaRegistry, url, options ) {
 	const figure = writer.createContainerElement( 'figure', { class: 'media' } );
 	const figure = writer.createContainerElement( 'figure', { class: 'media' } );
 
 
 	// TODO: This is a hack. Without it, the figure in the data pipeline will contain   because
 	// TODO: This is a hack. Without it, the figure in the data pipeline will contain   because
@@ -44,35 +44,11 @@ export function createMediaFigureElement( writer, options = {} ) {
 	// Note: The hack comes from widget utils; it makes the figure act like it's a widget.
 	// Note: The hack comes from widget utils; it makes the figure act like it's a widget.
 	figure.getFillerOffset = getFillerOffset;
 	figure.getFillerOffset = getFillerOffset;
 
 
-	addMediaWrapperElementToFigure( writer, figure, options );
+	writer.insert( ViewPosition.createAt( figure ), mediaRegistry.getMediaViewElement( writer, url, options ) );
 
 
 	return figure;
 	return figure;
 }
 }
 
 
-export function addMediaWrapperElementToFigure( writer, figure, options ) {
-	let renderFunction;
-
-	if ( options.mediaHtml ) {
-		renderFunction = function( domDocument ) {
-			const domElement = this.toDomElement( domDocument );
-
-			domElement.innerHTML = options.mediaHtml;
-
-			return domElement;
-		};
-	}
-
-	let wrapper;
-
-	if ( options.useSemanticWrapper ) {
-		wrapper = writer.createEmptyElement( 'oembed', options.attributes, renderFunction );
-	} else {
-		wrapper = writer.createUIElement( 'div', options.attributes, renderFunction );
-	}
-
-	writer.insert( ViewPosition.createAt( figure ), wrapper );
-}
-
 export function getSelectedMediaElement( selection ) {
 export function getSelectedMediaElement( selection ) {
 	const selectedElement = selection.getSelectedElement();
 	const selectedElement = selection.getSelectedElement();
 
 

+ 2 - 1
packages/ckeditor5-media-embed/tests/manual/semanticmediaembed.html

@@ -13,6 +13,7 @@
 <h2>Test editor</h2>
 <h2>Test editor</h2>
 <div id="editor">
 <div id="editor">
 	<h2>Media with previews</h2>
 	<h2>Media with previews</h2>
+
 		<figure class="media">
 		<figure class="media">
 			<oembed url="https://www.youtube.com/watch?v=ZVv7UMQPEWk" />
 			<oembed url="https://www.youtube.com/watch?v=ZVv7UMQPEWk" />
 		</figure>
 		</figure>
@@ -25,7 +26,7 @@
 			<oembed url="https://open.spotify.com/artist/7GaxyUddsPok8BuhxN6OUW?si=LM7Q50vqQ4-NYnAIcMTNuQ" />
 			<oembed url="https://open.spotify.com/artist/7GaxyUddsPok8BuhxN6OUW?si=LM7Q50vqQ4-NYnAIcMTNuQ" />
 		</figure>
 		</figure>
 
 
-		<h2>Generic media</h2>
+	<h2>Generic media</h2>
 
 
 		<figure class="media">
 		<figure class="media">
 			<oembed url="https://twitter.com/ckeditor/status/1021777799844126720" />
 			<oembed url="https://twitter.com/ckeditor/status/1021777799844126720" />

+ 4 - 5
packages/ckeditor5-media-embed/tests/mediaembedui.js

@@ -23,7 +23,7 @@ describe( 'MediaEmbedUI', () => {
 				mediaEmbed: {
 				mediaEmbed: {
 					media: [
 					media: [
 						{
 						{
-							url: /https:\/\/valid\/(.*)/,
+							url: /^https:\/\/valid\/(.*)/,
 							html: id => `<iframe src="${ id }"></iframe>`
 							html: id => `<iframe src="${ id }"></iframe>`
 						}
 						}
 					]
 					]
@@ -129,8 +129,7 @@ describe( 'MediaEmbedUI', () => {
 				const commandSpy = sinon.spy( editor.commands.get( 'insertMedia' ), 'execute' );
 				const commandSpy = sinon.spy( editor.commands.get( 'insertMedia' ), 'execute' );
 
 
 				// The form is invalid.
 				// The form is invalid.
-				const stub = sinon.stub( form, 'isValid' ).returns( false );
-				form.url = 'http://ckeditor.com';
+				form.url = 'https://invalid/url';
 				dropdown.isOpen = true;
 				dropdown.isOpen = true;
 
 
 				dropdown.fire( 'submit' );
 				dropdown.fire( 'submit' );
@@ -140,11 +139,11 @@ describe( 'MediaEmbedUI', () => {
 				expect( dropdown.isOpen ).to.be.true;
 				expect( dropdown.isOpen ).to.be.true;
 
 
 				// The form is valid.
 				// The form is valid.
-				stub.returns( true );
+				form.url = 'https://valid/url';
 				dropdown.fire( 'submit' );
 				dropdown.fire( 'submit' );
 
 
 				sinon.assert.calledOnce( commandSpy );
 				sinon.assert.calledOnce( commandSpy );
-				sinon.assert.calledWithExactly( commandSpy, 'http://ckeditor.com' );
+				sinon.assert.calledWithExactly( commandSpy, 'https://valid/url' );
 				sinon.assert.calledOnce( viewFocusSpy );
 				sinon.assert.calledOnce( viewFocusSpy );
 				expect( dropdown.isOpen ).to.be.false;
 				expect( dropdown.isOpen ).to.be.false;
 			} );
 			} );