From 1d89e05fe2fea9038784607809ef2e050d08739c Mon Sep 17 00:00:00 2001 From: HendrikThePendric Date: Mon, 21 Oct 2024 13:47:40 +0200 Subject: [PATCH] refactor: append icon children to iconGroup using native DOM API --- .../singleValue/renderer/addIconElement.js | 46 +++---------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/src/visualizations/config/adapters/dhis_highcharts/customSVGOptions/singleValue/renderer/addIconElement.js b/src/visualizations/config/adapters/dhis_highcharts/customSVGOptions/singleValue/renderer/addIconElement.js index b1df849f8..dfa2c0c57 100644 --- a/src/visualizations/config/adapters/dhis_highcharts/customSVGOptions/singleValue/renderer/addIconElement.js +++ b/src/visualizations/config/adapters/dhis_highcharts/customSVGOptions/singleValue/renderer/addIconElement.js @@ -16,46 +16,14 @@ export function addIconElement(svgString, color) { * whitespace around it and makes scaling and translating easier. */ this.renderer.rect(0, 0, iconElWidth, iconElHeight).add(iconGroup) - /* The code below makes some assumptions about icon SVGs: - * 1. The SVG children are all elements and they are not nested - * 2. Only the `d`, `fill-rule`, `clip-rule` and `fill` attributes are used */ Array.from(svgIconDocument.documentElement.children).forEach((pathNode) => { - /* Highcharts expects an array of letters and numbers but the - * d-attribute string only puts spaces in between adjacent number, - * not between letter and numbers. */ - const d = pathNode - .getAttribute('d') - // Add space after letter when followed by number - .replace(/([A-Z])([0-9])/g, '$1 $2') - // Add space after number when followed by letter - .replace(/([0-9])([A-Z])/g, '$1 $2') - .split(' ') - .map((point) => - isNaN(parseFloat(point)) ? point : parseFloat(point) - ) - const attr = {} - const fillRule = pathNode.getAttribute('fill-rule') - const clipRule = pathNode.getAttribute('clip-rule') - const fill = pathNode.getAttribute('fill') - - if (fillRule) { - attr['fill-rule'] = fillRule - } - - if (clipRule) { - attr['clip-rule'] = clipRule - } - if (fill) { - attr['fill'] = fill - } - - this.renderer.path(d).attr(attr).add(iconGroup) - - /* NB: the line below does exactly the same as the lines above, - * but does not user the SVGRenderer. We could consider switching - * to this if it turns out that not all the assumptions about SVG - * icons are correct. */ - // iconGroup.element.appendChild(pathNode) + /* It is also possible to use the SVGRenderer to draw the icon but that + * approach is more error prone, so during review it was decided to just + * append the SVG children to the iconGroup using native the native DOM + * API. For reference see this commit, for an implementation using the + * SVVGRenderer: + * https://github.com/dhis2/analytics/pull/1698/commits/f95bee838e07f4cdfc3cab6e92f28f49a386a0ad */ + iconGroup.element.appendChild(pathNode) }) iconGroup.add()