Skip to content

Commit

Permalink
refactor: append icon children to iconGroup using native DOM API
Browse files Browse the repository at this point in the history
  • Loading branch information
HendrikThePendric committed Oct 22, 2024
1 parent 9380a0c commit 1d89e05
Showing 1 changed file with 7 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path> 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()
Expand Down

0 comments on commit 1d89e05

Please sign in to comment.