Merge pull request #374 from ckeditor/t/ckeditor5-theme-lark/148

Feature: Implemented the `IconView#fillColor` observable which fills child `.ck-icon__fill` paths  with the color (see ckeditor/ckeditor5-theme-lark#148).

src/button/buttonview.js

@@ -81,11 +81,19 @@ export default class ButtonView extends View {
 		this.labelView = this._createLabelView();
 
 		/**
-		 * (Optional) The icon view of the button. Only present when the {@link #icon icon attribute} is defined.
+		 * The icon view of the button. Will be added to {@link #children} when the
+		 * {@link #icon icon attribute} is defined.
 		 *
 		 * @readonly
 		 * @member {module:ui/icon/iconview~IconView} #iconView
 		 */
+		this.iconView = new IconView();
+
+		this.iconView.extendTemplate( {
+			attributes: {
+				class: 'ck-button__icon'
+			}
+		} );
 
 		/**
 		 * Tooltip of the button bound to the template.
@@ -147,17 +155,8 @@ export default class ButtonView extends View {
 		super.render();
 
 		if ( this.icon ) {
-			const iconView = this.iconView = new IconView();
-
-			iconView.bind( 'content' ).to( this, 'icon' );
-
-			iconView.extendTemplate( {
-				attributes: {
-					class: 'ck-button__icon'
-				}
-			} );
-
-			this.children.add( iconView );
+			this.iconView.bind( 'content' ).to( this, 'icon' );
+			this.children.add( this.iconView );
 		}
 
 		this.children.add( this.tooltipView );

src/icon/iconview.js

@@ -45,6 +45,15 @@ export default class IconView extends View {
 		 */
 		this.set( 'viewBox', '0 0 20 20' );
 
+		/**
+		 * The fill color of the child `path.ck-icon__fill`.
+		 *
+		 * @observable
+		 * @default ''
+		 * @member {String} #fillColor
+		 */
+		this.set( 'fillColor', '' );
+
 		this.setTemplate( {
 			tag: 'svg',
 			ns: 'http://www.w3.org/2000/svg',
@@ -62,10 +71,18 @@ export default class IconView extends View {
 		super.render();
 
 		this._updateXMLContent();
+		this._colorFillPaths();
 
 		// This is a hack for lack of innerHTML binding.
 		// See: https://github.com/ckeditor/ckeditor5-ui/issues/99.
-		this.on( 'change:content', () => this._updateXMLContent() );
+		this.on( 'change:content', () => {
+			this._updateXMLContent();
+			this._colorFillPaths();
+		} );
+
+		this.on( 'change:fillColor', () => {
+			this._colorFillPaths();
+		} );
 	}
 
 	/**
@@ -90,4 +107,17 @@ export default class IconView extends View {
 			}
 		}
 	}
+
+	/**
+	 * Fills all child `path.ck-icon__fill` with the `#fillColor`.
+	 *
+	 * @private
+	 */
+	_colorFillPaths() {
+		if ( this.fillColor ) {
+			this.element.querySelectorAll( '.ck-icon__fill' ).forEach( path => {
+				path.style.fill = this.fillColor;
+			} );
+		}
+	}
 }

tests/button/buttonview.js

@@ -36,6 +36,10 @@ describe( 'ButtonView', () => {
 		it( 'creates #labelView', () => {
 			expect( view.labelView ).to.be.instanceOf( View );
 		} );
+
+		it( 'creates #iconView', () => {
+			expect( view.iconView ).to.be.instanceOf( IconView );
+		} );
 	} );
 
 	describe( '<button> bindings', () => {
@@ -233,12 +237,15 @@ describe( 'ButtonView', () => {
 	} );
 
 	describe( 'icon', () => {
-		it( 'is not initially set', () => {
+		it( 'is omited in #children when view#icon is not defined', () => {
+			view = new ButtonView( locale );
+			view.render();
+
 			expect( view.element.childNodes ).to.have.length( 2 );
-			expect( view.iconView ).to.be.undefined;
+			expect( view.iconView.element ).to.be.null;
 		} );
 
-		it( 'is set when view#icon is defined', () => {
+		it( 'is added to the #children when view#icon is defined', () => {
 			view = new ButtonView( locale );
 			view.icon = '<svg></svg>';
 			view.render();

tests/icon/iconview.js

@@ -4,6 +4,7 @@
  */
 
 import IconView from '../../src/icon/iconview';
+import normalizeHtml from '@ckeditor/ckeditor5-utils/tests/_utils/normalizehtml';
 
 describe( 'IconView', () => {
 	let view;
@@ -22,6 +23,10 @@ describe( 'IconView', () => {
 			expect( view.viewBox ).to.equal( '0 0 20 20' );
 		} );
 
+		it( 'sets #fillColor', () => {
+			expect( view.fillColor ).to.equal( '' );
+		} );
+
 		it( 'creates element from template', () => {
 			expect( view.element.tagName ).to.equal( 'svg' );
 			expect( view.element.getAttribute( 'class' ) ).to.equal( 'ck-icon' );
@@ -42,29 +47,90 @@ describe( 'IconView', () => {
 
 		describe( 'inline svg', () => {
 			it( 'should react to changes in view#content', () => {
-				expect( view.element.innerHTML = '' );
+				assertIconInnerHTML( view, '' );
 
 				view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg"><g id="test"></g></svg>';
-				expect( view.element.innerHTML = '<g id="test"></g>' );
+				assertIconInnerHTML( view, '<g id="test"></g>' );
 
 				view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg"></svg>';
-				expect( view.element.innerHTML = '' );
+				assertIconInnerHTML( view, '' );
 			} );
 
 			it( 'works for #content with more than <svg> declaration', () => {
-				expect( view.element.innerHTML = '' );
+				assertIconInnerHTML( view, '' );
 
 				view.content =
 					'<?xml version="1.0" encoding="utf-8"?><svg version="1.1" xmlns="http://www.w3.org/2000/svg"><g id="test"></g></svg>';
-				expect( view.element.innerHTML = '<g id="test"></g>' );
+				assertIconInnerHTML( view, '<g id="test"></g>' );
 			} );
 
 			it( 'should respect parsed <svg>\'s viewBox attribute', () => {
-				expect( view.element.innerHTML = '' );
+				assertIconInnerHTML( view, '' );
 
 				view.content = '<svg version="1.1" viewBox="10 20 30 40" xmlns="http://www.w3.org/2000/svg"><g id="test"></g></svg>';
 				expect( view.viewBox ).to.equal( '10 20 30 40' );
 			} );
 		} );
+
+		describe( 'fill color', () => {
+			it( 'should be set intially based on view#fillColor', () => {
+				view.fillColor = 'red';
+				view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
+						'<path class="ck-icon__fill"/>' +
+						'<path/>' +
+						'<path class="ck-icon__fill"/>' +
+					'</svg>';
+
+				expect( view.element.children[ 0 ].style.fill ).to.equal( 'red' );
+				expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
+				expect( view.element.children[ 2 ].style.fill ).to.equal( 'red' );
+			} );
+
+			it( 'should react to changes in view#fillColor', () => {
+				view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
+						'<path class="ck-icon__fill"/>' +
+						'<path/>' +
+						'<path class="ck-icon__fill"/>' +
+					'</svg>';
+
+				expect( view.element.children[ 0 ].style.fill ).to.equal( '' );
+				expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
+				expect( view.element.children[ 2 ].style.fill ).to.equal( '' );
+
+				view.fillColor = 'red';
+
+				expect( view.element.children[ 0 ].style.fill ).to.equal( 'red' );
+				expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
+				expect( view.element.children[ 2 ].style.fill ).to.equal( 'red' );
+			} );
+
+			it( 'should react to changes in view#content', () => {
+				view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
+						'<path class="ck-icon__fill"/>' +
+						'<path/>' +
+						'<path class="ck-icon__fill"/>' +
+					'</svg>';
+
+				view.fillColor = 'red';
+
+				expect( view.element.children[ 0 ].style.fill ).to.equal( 'red' );
+				expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
+				expect( view.element.children[ 2 ].style.fill ).to.equal( 'red' );
+
+				view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
+						'<path/>' +
+						'<path class="ck-icon__fill"/>' +
+					'</svg>';
+
+				expect( view.element.children[ 0 ].style.fill ).to.equal( '' );
+				expect( view.element.children[ 1 ].style.fill ).to.equal( 'red' );
+			} );
+		} );
 	} );
 } );
+
+function assertIconInnerHTML( icon, expected ) {
+	// Edge adds the xmlns attribute to each node when obtaining from parent's innerHTML.
+	expect( normalizeHtml( icon.element.innerHTML.replace( /xmlns="[^"]+"/, '' ) ) )
+		.to.equal( expected );
+}

theme/components/icon/icon.css

@@ -4,29 +4,5 @@
  */
 
 svg.ck-icon {
-	/* Multiplied by the height of the line in "px" should give SVG "viewport" dimensions */
-	font-size: 0.8333333333em;
-
-	/* Must be consistent with .ck-button__label's vertical align. Otherwise, buttons with and
-	without labels (but with icons) have different sizes in Chrome */
 	vertical-align: middle;
-
-	color: inherit;
-
-	/* Inherit cursor style (#5). */
-	cursor: inherit;
-
-	/* This will prevent blurry icons on Firefox. See #340. */
-	will-change: transform;
-
-	& * {
-		/* Inherit cursor style (#5). */
-		cursor: inherit;
-
-		/* Allows dynamic coloring of the icons. */
-		color: inherit;
-
-		/* Needed by FF. */
-		fill: currentColor;
-	}
 }