Icon config

[rofe]

Proposal to enable #523 without breaking backward compatibility.

[codecov]

Codecov Report

Merging #529 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #529   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          51     51           
  Lines        1139   1139           
=====================================
  Hits         1139   1139

Impacted Files	Coverage Δ
src/utils/icon-handler.js	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63f615e...b9c7b5a. Read the comment docs.

[trieloff]

The code looks good, but do we really need the config?

[rofe]

The code looks good, but do we really need the config?

If we want to enable #523 without disabling single SVG icon files at the same time, I thought a config would be the least intrusive/breaking way to have both. But I'm open for alternative solutions.

[trieloff]

Ok

[tripodsan]

is this really needed?

• even with the config, you can only change so much. but not the tag name (e.g. if you wanted img icons)
• a helix-pages user would not be able to change this
• I think that using action params for controlling the output is suboptimal and not scalable.

As for the configurability, we should find a way for the developper to extend the makehtml step.
maybe with pipe.configure('html').getHandler('icon').options(....). I sure we find a better way :-)

As for helix-pages, I think this should be solvable via the markup.yaml. see #516

maybe:

markup:
  icons:
    match: svg.icon
    replace: '<img src="/designs/pics/${e.data-value}.png">'

[rofe]

I agree that we'll need a solution that works for helix-pages. Closing this PR.