search

FortAwesome / fa-icon-chooser

an icon chooser implemented as a web component
MIT License
15 stars 6 forks source link

Fix parseSvgText() for duotone layer ordering #38

Closed mlwilkerson closed 2 years ago

mlwilkerson commented 2 years ago

When implementing parseSvgText(), I mistakenly thought that the primary layer appears before the secondary layer in the FA Standard SVGs. It's the other way: secondary first.

The parsing function and tests should be updated accordingly.

Bibhavshah commented 2 years ago

Can I work on this issue?

mlwilkerson commented 2 years ago

@Bibhavshah Of course! That would be great. To help with that, I've added this commit on a new branch that has revised tests that should all pass when parseSvgText() implementation is correct.

(You could cherry-pick that commit or just copy/paste the tests into your branch. We don't need necessarily need to keep that branch.)

mlwilkerson commented 2 years ago

I'm including a fix for this in #39. Turns out, the duotone icons were rendering visually correctly, because while parseSvgText() was getting the order backwards, the fa-icon component was also doing it backwards. In this one case in the universe, two wrongs made it right.

39 fixes both, which results in correctness, but no visual change in rendering.