Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit 728a691

Browse files
authored
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).
2 parents b45e338 + 578b751 commit 728a691

File tree

5 files changed

+124
-46
lines changed

5 files changed

+124
-46
lines changed

src/button/buttonview.js

+11-12
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,19 @@ export default class ButtonView extends View {
8181
this.labelView = this._createLabelView();
8282

8383
/**
84-
* (Optional) The icon view of the button. Only present when the {@link #icon icon attribute} is defined.
84+
* The icon view of the button. Will be added to {@link #children} when the
85+
* {@link #icon icon attribute} is defined.
8586
*
8687
* @readonly
8788
* @member {module:ui/icon/iconview~IconView} #iconView
8889
*/
90+
this.iconView = new IconView();
91+
92+
this.iconView.extendTemplate( {
93+
attributes: {
94+
class: 'ck-button__icon'
95+
}
96+
} );
8997

9098
/**
9199
* Tooltip of the button bound to the template.
@@ -147,17 +155,8 @@ export default class ButtonView extends View {
147155
super.render();
148156

149157
if ( this.icon ) {
150-
const iconView = this.iconView = new IconView();
151-
152-
iconView.bind( 'content' ).to( this, 'icon' );
153-
154-
iconView.extendTemplate( {
155-
attributes: {
156-
class: 'ck-button__icon'
157-
}
158-
} );
159-
160-
this.children.add( iconView );
158+
this.iconView.bind( 'content' ).to( this, 'icon' );
159+
this.children.add( this.iconView );
161160
}
162161

163162
this.children.add( this.tooltipView );

src/icon/iconview.js

+31-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ export default class IconView extends View {
4545
*/
4646
this.set( 'viewBox', '0 0 20 20' );
4747

48+
/**
49+
* The fill color of the child `path.ck-icon__fill`.
50+
*
51+
* @observable
52+
* @default ''
53+
* @member {String} #fillColor
54+
*/
55+
this.set( 'fillColor', '' );
56+
4857
this.setTemplate( {
4958
tag: 'svg',
5059
ns: 'http://www.w3.org/2000/svg',
@@ -62,10 +71,18 @@ export default class IconView extends View {
6271
super.render();
6372

6473
this._updateXMLContent();
74+
this._colorFillPaths();
6575

6676
// This is a hack for lack of innerHTML binding.
6777
// See: https://github.com/ckeditor/ckeditor5-ui/issues/99.
68-
this.on( 'change:content', () => this._updateXMLContent() );
78+
this.on( 'change:content', () => {
79+
this._updateXMLContent();
80+
this._colorFillPaths();
81+
} );
82+
83+
this.on( 'change:fillColor', () => {
84+
this._colorFillPaths();
85+
} );
6986
}
7087

7188
/**
@@ -90,4 +107,17 @@ export default class IconView extends View {
90107
}
91108
}
92109
}
110+
111+
/**
112+
* Fills all child `path.ck-icon__fill` with the `#fillColor`.
113+
*
114+
* @private
115+
*/
116+
_colorFillPaths() {
117+
if ( this.fillColor ) {
118+
this.element.querySelectorAll( '.ck-icon__fill' ).forEach( path => {
119+
path.style.fill = this.fillColor;
120+
} );
121+
}
122+
}
93123
}

tests/button/buttonview.js

+10-3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ describe( 'ButtonView', () => {
3636
it( 'creates #labelView', () => {
3737
expect( view.labelView ).to.be.instanceOf( View );
3838
} );
39+
40+
it( 'creates #iconView', () => {
41+
expect( view.iconView ).to.be.instanceOf( IconView );
42+
} );
3943
} );
4044

4145
describe( '<button> bindings', () => {
@@ -233,12 +237,15 @@ describe( 'ButtonView', () => {
233237
} );
234238

235239
describe( 'icon', () => {
236-
it( 'is not initially set', () => {
240+
it( 'is omited in #children when view#icon is not defined', () => {
241+
view = new ButtonView( locale );
242+
view.render();
243+
237244
expect( view.element.childNodes ).to.have.length( 2 );
238-
expect( view.iconView ).to.be.undefined;
245+
expect( view.iconView.element ).to.be.null;
239246
} );
240247

241-
it( 'is set when view#icon is defined', () => {
248+
it( 'is added to the #children when view#icon is defined', () => {
242249
view = new ButtonView( locale );
243250
view.icon = '<svg></svg>';
244251
view.render();

tests/icon/iconview.js

+72-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import IconView from '../../src/icon/iconview';
7+
import normalizeHtml from '@ckeditor/ckeditor5-utils/tests/_utils/normalizehtml';
78

89
describe( 'IconView', () => {
910
let view;
@@ -22,6 +23,10 @@ describe( 'IconView', () => {
2223
expect( view.viewBox ).to.equal( '0 0 20 20' );
2324
} );
2425

26+
it( 'sets #fillColor', () => {
27+
expect( view.fillColor ).to.equal( '' );
28+
} );
29+
2530
it( 'creates element from template', () => {
2631
expect( view.element.tagName ).to.equal( 'svg' );
2732
expect( view.element.getAttribute( 'class' ) ).to.equal( 'ck-icon' );
@@ -42,29 +47,90 @@ describe( 'IconView', () => {
4247

4348
describe( 'inline svg', () => {
4449
it( 'should react to changes in view#content', () => {
45-
expect( view.element.innerHTML = '' );
50+
assertIconInnerHTML( view, '' );
4651

4752
view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg"><g id="test"></g></svg>';
48-
expect( view.element.innerHTML = '<g id="test"></g>' );
53+
assertIconInnerHTML( view, '<g id="test"></g>' );
4954

5055
view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg"></svg>';
51-
expect( view.element.innerHTML = '' );
56+
assertIconInnerHTML( view, '' );
5257
} );
5358

5459
it( 'works for #content with more than <svg> declaration', () => {
55-
expect( view.element.innerHTML = '' );
60+
assertIconInnerHTML( view, '' );
5661

5762
view.content =
5863
'<?xml version="1.0" encoding="utf-8"?><svg version="1.1" xmlns="http://www.w3.org/2000/svg"><g id="test"></g></svg>';
59-
expect( view.element.innerHTML = '<g id="test"></g>' );
64+
assertIconInnerHTML( view, '<g id="test"></g>' );
6065
} );
6166

6267
it( 'should respect parsed <svg>\'s viewBox attribute', () => {
63-
expect( view.element.innerHTML = '' );
68+
assertIconInnerHTML( view, '' );
6469

6570
view.content = '<svg version="1.1" viewBox="10 20 30 40" xmlns="http://www.w3.org/2000/svg"><g id="test"></g></svg>';
6671
expect( view.viewBox ).to.equal( '10 20 30 40' );
6772
} );
6873
} );
74+
75+
describe( 'fill color', () => {
76+
it( 'should be set intially based on view#fillColor', () => {
77+
view.fillColor = 'red';
78+
view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
79+
'<path class="ck-icon__fill"/>' +
80+
'<path/>' +
81+
'<path class="ck-icon__fill"/>' +
82+
'</svg>';
83+
84+
expect( view.element.children[ 0 ].style.fill ).to.equal( 'red' );
85+
expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
86+
expect( view.element.children[ 2 ].style.fill ).to.equal( 'red' );
87+
} );
88+
89+
it( 'should react to changes in view#fillColor', () => {
90+
view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
91+
'<path class="ck-icon__fill"/>' +
92+
'<path/>' +
93+
'<path class="ck-icon__fill"/>' +
94+
'</svg>';
95+
96+
expect( view.element.children[ 0 ].style.fill ).to.equal( '' );
97+
expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
98+
expect( view.element.children[ 2 ].style.fill ).to.equal( '' );
99+
100+
view.fillColor = 'red';
101+
102+
expect( view.element.children[ 0 ].style.fill ).to.equal( 'red' );
103+
expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
104+
expect( view.element.children[ 2 ].style.fill ).to.equal( 'red' );
105+
} );
106+
107+
it( 'should react to changes in view#content', () => {
108+
view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
109+
'<path class="ck-icon__fill"/>' +
110+
'<path/>' +
111+
'<path class="ck-icon__fill"/>' +
112+
'</svg>';
113+
114+
view.fillColor = 'red';
115+
116+
expect( view.element.children[ 0 ].style.fill ).to.equal( 'red' );
117+
expect( view.element.children[ 1 ].style.fill ).to.equal( '' );
118+
expect( view.element.children[ 2 ].style.fill ).to.equal( 'red' );
119+
120+
view.content = '<svg version="1.1" xmlns="http://www.w3.org/2000/svg">' +
121+
'<path/>' +
122+
'<path class="ck-icon__fill"/>' +
123+
'</svg>';
124+
125+
expect( view.element.children[ 0 ].style.fill ).to.equal( '' );
126+
expect( view.element.children[ 1 ].style.fill ).to.equal( 'red' );
127+
} );
128+
} );
69129
} );
70130
} );
131+
132+
function assertIconInnerHTML( icon, expected ) {
133+
// Edge adds the xmlns attribute to each node when obtaining from parent's innerHTML.
134+
expect( normalizeHtml( icon.element.innerHTML.replace( /xmlns="[^"]+"/, '' ) ) )
135+
.to.equal( expected );
136+
}

theme/components/icon/icon.css

-24
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,5 @@
44
*/
55

66
svg.ck-icon {
7-
/* Multiplied by the height of the line in "px" should give SVG "viewport" dimensions */
8-
font-size: 0.8333333333em;
9-
10-
/* Must be consistent with .ck-button__label's vertical align. Otherwise, buttons with and
11-
without labels (but with icons) have different sizes in Chrome */
127
vertical-align: middle;
13-
14-
color: inherit;
15-
16-
/* Inherit cursor style (#5). */
17-
cursor: inherit;
18-
19-
/* This will prevent blurry icons on Firefox. See #340. */
20-
will-change: transform;
21-
22-
& * {
23-
/* Inherit cursor style (#5). */
24-
cursor: inherit;
25-
26-
/* Allows dynamic coloring of the icons. */
27-
color: inherit;
28-
29-
/* Needed by FF. */
30-
fill: currentColor;
31-
}
328
}

0 commit comments

Comments
 (0)