Skip to content

Commit e35bd69

Browse files
web-padawanweb-padawan
committed
refactor: align API with iron-iconset-svg
1 parent 32c69a6 commit e35bd69

File tree

6 files changed

+100
-51
lines changed

6 files changed

+100
-51
lines changed

‎packages/vaadin-icon/src/vaadin-icon.js‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
import { PolymerElement, html } from '@polymer/polymer/polymer-element.js';
77
import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
88
import { ElementMixin } from '@vaadin/vaadin-element-mixin/vaadin-element-mixin.js';
9-
import { IconsetElement, getIconId } from './vaadin-iconset.js';
10-
import { renderSvg } from './vaadin-icon-svg.js';
9+
import { IconsetElement } from './vaadin-iconset.js';
10+
import { ensureSvgLiteral, renderSvg } from './vaadin-icon-svg.js';
1111

1212
const DEFAULT_ICONSET = 'vaadin';
1313

@@ -46,7 +46,7 @@ class IconElement extends ThemableMixin(ElementMixin(PolymerElement)) {
4646
version="1.1"
4747
xmlns="http://www.w3.org/2000/svg"
4848
xmlns:xlink="http://www.w3.org/1999/xlink"
49-
viewBox="0 0 [[size]] [[size]]"
49+
viewBox="[[__computeViewBox(size)]]"
5050
preserveAspectRatio="xMidYMid meet"
5151
></svg>
5252
`;
@@ -83,7 +83,7 @@ class IconElement extends ThemableMixin(ElementMixin(PolymerElement)) {
8383
*/
8484
size: {
8585
type: Number,
86-
value: 16
86+
value: 24
8787
},
8888

8989
/** @private */
@@ -105,12 +105,15 @@ class IconElement extends ThemableMixin(ElementMixin(PolymerElement)) {
105105
__iconChanged(icon) {
106106
if (icon) {
107107
const parts = icon.split(':');
108-
const iconName = getIconId(parts.pop());
109-
const iconsetName = parts.pop() || DEFAULT_ICONSET;
108+
const iconsetName = parts[0] || DEFAULT_ICONSET;
110109
const iconset = IconsetElement.getIconset(iconsetName);
111-
this.svg = iconset.applyIcon(iconName);
110+
const { svg, size } = iconset.applyIcon(icon);
111+
if (size !== this.size) {
112+
this.size = size;
113+
}
114+
this.svg = svg;
112115
} else {
113-
this.svg = null;
116+
this.svg = ensureSvgLiteral(null);
114117
}
115118
}
116119

@@ -122,6 +125,11 @@ class IconElement extends ThemableMixin(ElementMixin(PolymerElement)) {
122125

123126
renderSvg(svg, svgElement);
124127
}
128+
129+
/** @private */
130+
__computeViewBox(size) {
131+
return `0 0 ${size} ${size}`;
132+
}
125133
}
126134

127135
customElements.define(IconElement.is, IconElement);

‎packages/vaadin-icon/src/vaadin-iconset.d.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,16 @@ declare class IconsetElement extends ElementMixin(HTMLElement) {
1515
*/
1616
name: string;
1717

18+
/**
19+
* The size of an individual icon. Note that icons must be square.
20+
*/
21+
size: number;
22+
1823
/**
1924
* Produce SVGTemplateResult for the element matching `id` in this
2025
* iconset, or `undefined` if there is no matching element.
2126
*/
22-
applyIcon(name: string): IconSvgLiteral;
27+
applyIcon(name: string): { svg: IconSvgLiteral; size: number };
2328
}
2429

2530
declare global {

‎packages/vaadin-icon/src/vaadin-iconset.js‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import { cloneSvgNode } from './vaadin-icon-svg.js';
99

1010
const iconRegistry = {};
1111

12-
export const getIconId = (id) => (id || '').replace('vaadin-icon:', '');
13-
1412
/**
1513
* `<vaadin-iconset>` is a Web Component for creating SVG icon collections.
1614
*
@@ -38,6 +36,14 @@ class IconsetElement extends ElementMixin(PolymerElement) {
3836
name: {
3937
type: String,
4038
observer: '__nameChanged'
39+
},
40+
41+
/**
42+
* The size of an individual icon. Note that icons must be square.
43+
*/
44+
size: {
45+
type: Number,
46+
value: 24
4147
}
4248
};
4349
}
@@ -74,7 +80,7 @@ class IconsetElement extends ElementMixin(PolymerElement) {
7480
// create the icon map on-demand, since the iconset itself has no discrete
7581
// signal to know when it's children are fully parsed
7682
this._icons = this._icons || this.__createIconMap();
77-
return cloneSvgNode(this._icons[getIconId(id)]);
83+
return { svg: cloneSvgNode(this._icons[this.__getIconId(id)]), size: this.size };
7884
}
7985

8086
/**
@@ -83,11 +89,16 @@ class IconsetElement extends ElementMixin(PolymerElement) {
8389
__createIconMap() {
8490
const icons = {};
8591
this.querySelectorAll('[id]').forEach((icon) => {
86-
icons[getIconId(icon.id)] = icon;
92+
icons[this.__getIconId(icon.id)] = icon;
8793
});
8894
return icons;
8995
}
9096

97+
/** @private */
98+
__getIconId(id) {
99+
return (id || '').replace(`${this.name}:`, '');
100+
}
101+
91102
/** @private */
92103
__nameChanged(name, oldName) {
93104
if (oldName) {

‎packages/vaadin-icon/test/icon.test.js‎

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,17 @@
11
import { expect } from '@esm-bundle/chai';
22
import sinon from 'sinon';
33
import { fixtureSync } from '@vaadin/testing-helpers';
4-
import { getIconId } from '../vaadin-iconset.js';
54
import { unsafeSvgLiteral } from '../src/vaadin-icon-svg.js';
65
import '../vaadin-icon.js';
76

87
const ANGLE_DOWN = '<path d="M13 4v2l-5 5-5-5v-2l5 5z"></path>';
98
const ANGLE_UP = '<path d="M3 12v-2l5-5 5 5v2l-5-5z"></path>';
109

1110
describe('vaadin-icon', () => {
12-
let icon;
11+
let icon, svgElement;
1312

14-
function expectIcon(content, size = 16) {
15-
const svgElement = icon.shadowRoot.querySelector('svg');
13+
function expectIcon(content) {
1614
expect(svgElement.innerHTML.trim().replace(/<!--[^>]*-->/g, '')).to.equal(content);
17-
expect(svgElement.getAttribute('viewBox')).to.equal(`0 0 ${size} ${size}`);
1815
}
1916

2017
describe('custom element definition', () => {
@@ -34,6 +31,7 @@ describe('vaadin-icon', () => {
3431
describe('svg property', () => {
3532
beforeEach(() => {
3633
icon = fixtureSync('<vaadin-icon></vaadin-icon>');
34+
svgElement = icon.shadowRoot.querySelector('svg');
3735
});
3836

3937
describe('valid icon', () => {
@@ -42,17 +40,11 @@ describe('vaadin-icon', () => {
4240
expectIcon(ANGLE_DOWN);
4341
});
4442

45-
it('should update icon when size property is set', () => {
46-
icon.size = 24;
43+
it('should update icon when svg property is updated', () => {
4744
icon.svg = unsafeSvgLiteral(ANGLE_DOWN);
48-
expectIcon(ANGLE_DOWN, 24);
49-
});
50-
51-
it('should update icon when size property is updated', () => {
52-
icon.svg = unsafeSvgLiteral(ANGLE_DOWN);
53-
expectIcon(ANGLE_DOWN, 16);
54-
icon.size = 24;
55-
expectIcon(ANGLE_DOWN, 24);
45+
expectIcon(ANGLE_DOWN);
46+
icon.svg = unsafeSvgLiteral(ANGLE_UP);
47+
expectIcon(ANGLE_UP);
5648
});
5749
});
5850

@@ -87,6 +79,26 @@ describe('vaadin-icon', () => {
8779
});
8880
});
8981

82+
describe('size property', () => {
83+
beforeEach(() => {
84+
icon = fixtureSync('<vaadin-icon></vaadin-icon>');
85+
svgElement = icon.shadowRoot.querySelector('svg');
86+
});
87+
88+
it('should set size property to 24 by default', () => {
89+
expect(icon.size).to.equal(24);
90+
});
91+
92+
it('should set viewBox attribute based on size', () => {
93+
expect(svgElement.getAttribute('viewBox')).to.equal('0 0 24 24');
94+
});
95+
96+
it('should update viewBox attribute on size change', () => {
97+
icon.size = 16;
98+
expect(svgElement.getAttribute('viewBox')).to.equal('0 0 16 16');
99+
});
100+
});
101+
90102
describe('icon property', () => {
91103
let iconset, icons;
92104

@@ -95,8 +107,8 @@ describe('vaadin-icon', () => {
95107
<vaadin-iconset name="vaadin">
96108
<svg xmlns="http://www.w3.org/2000/svg">
97109
<defs>
98-
<g id="vaadin-icon:angle-down">${ANGLE_DOWN}</g>
99-
<g id="vaadin-icon:angle-up">${ANGLE_UP}</g>
110+
<g id="vaadin:angle-down">${ANGLE_DOWN}</g>
111+
<g id="vaadin:angle-up">${ANGLE_UP}</g>
100112
</defs>
101113
</svg>
102114
</vaadin-iconset>
@@ -107,24 +119,25 @@ describe('vaadin-icon', () => {
107119
describe('default', () => {
108120
beforeEach(() => {
109121
icon = fixtureSync('<vaadin-icon></vaadin-icon>');
122+
svgElement = icon.shadowRoot.querySelector('svg');
110123
});
111124

112125
it('should render icon from the iconset', () => {
113126
icons.forEach((svgIcon) => {
114-
icon.icon = getIconId(svgIcon.getAttribute('id'));
127+
icon.icon = svgIcon.getAttribute('id');
115128
expectIcon(svgIcon.innerHTML);
116129
});
117130
});
118131

119132
it('should clear icon when the value is set to null', () => {
120-
icon.icon = 'angle-up';
133+
icon.icon = 'vaadin:angle-up';
121134
expectIcon(ANGLE_UP);
122135
icon.icon = null;
123136
expectIcon('');
124137
});
125138

126139
it('should clear icon when the value is set to undefined', () => {
127-
icon.icon = 'angle-up';
140+
icon.icon = 'vaadin:angle-up';
128141
expectIcon(ANGLE_UP);
129142
icon.icon = undefined;
130143
expectIcon('');
@@ -140,9 +153,10 @@ describe('vaadin-icon', () => {
140153
document.body.removeChild(icon);
141154
});
142155

143-
it('should clear icon when the value is set to null', () => {
144-
icon.icon = 'angle-up';
156+
it('should set icon when the value is set before attach', () => {
157+
icon.icon = 'vaadin:angle-up';
145158
document.body.appendChild(icon);
159+
svgElement = icon.shadowRoot.querySelector('svg');
146160
expectIcon(ANGLE_UP);
147161
});
148162
});

‎packages/vaadin-icon/test/iconset.test.js‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ describe('vaadin-iconset', () => {
88

99
beforeEach(() => {
1010
iconset = fixtureSync(`
11-
<vaadin-iconset name="vaadin">
11+
<vaadin-iconset name="vaadin" size="16">
1212
<svg xmlns="http://www.w3.org/2000/svg">
1313
<defs>
14-
<g id="vaadin-icon:caret-down"><path d="M3 4h10l-5 7z"></path></g>
14+
<g id="vaadin:caret-down"><path d="M3 4h10l-5 7z"></path></g>
1515
<g id="caret-up"><path d="M13 12h-10l5-7z"></path></g>
1616
</defs>
1717
</svg>
@@ -20,25 +20,36 @@ describe('vaadin-iconset', () => {
2020
});
2121

2222
describe('applyIcon', () => {
23-
it('should return svg literal when applyIcon called with correct id', () => {
24-
const icon = iconset.applyIcon('caret-down');
25-
expect(isValidSvg(icon)).to.be.true;
23+
it('should return svg literal when called with correct id', () => {
24+
const { svg } = iconset.applyIcon('caret-down');
25+
expect(isValidSvg(svg)).to.be.true;
2626
});
2727

28-
it('should return svg literal when applyIcon called with prefixed id', () => {
29-
const icon = iconset.applyIcon('vaadin-icon:caret-down');
30-
expect(isValidSvg(icon)).to.be.true;
28+
it('should return svg literal when called with prefixed id', () => {
29+
const { svg } = iconset.applyIcon('vaadin:caret-down');
30+
expect(isValidSvg(svg)).to.be.true;
3131
});
3232

33-
it('should return svg literal when applyIcon called with non-prefixed id', () => {
34-
const icon = iconset.applyIcon('caret-up');
35-
expect(isValidSvg(icon)).to.be.true;
33+
it('should return svg literal when called with non-prefixed id', () => {
34+
const { svg } = iconset.applyIcon('caret-up');
35+
expect(isValidSvg(svg)).to.be.true;
3636
});
3737

38-
it('should return empty svg when applyIcon is called with incorrect id', () => {
39-
const icon = iconset.applyIcon('non-existent');
40-
expect(isValidSvg(icon)).to.be.true;
41-
expect(typeof icon).to.equal('symbol');
38+
it('should return empty svg when called with incorrect id', () => {
39+
const { svg } = iconset.applyIcon('non-existent');
40+
expect(isValidSvg(svg)).to.be.true;
41+
expect(typeof svg).to.equal('symbol');
42+
});
43+
44+
it('should return default iconset size value when called', () => {
45+
const { size } = iconset.applyIcon('caret-down');
46+
expect(size).to.equal(16);
47+
});
48+
49+
it('should return new iconset size after it is changed', () => {
50+
iconset.size = 1000;
51+
const { size } = iconset.applyIcon('caret-down');
52+
expect(size).to.equal(1000);
4253
});
4354
});
4455

‎packages/vaadin-icon/test/typings/iconset.types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ const iconset = document.createElement('vaadin-iconset');
66
assertType<IconsetElement>(iconset);
77

88
const result = iconset.applyIcon('test');
9-
assertType<IconSvgLiteral>(result);
9+
assertType<{ svg: IconSvgLiteral; size: number }>(result);

0 commit comments

Comments
 (0)