search

feathericons / elm-feather

Feather icons for elm
http://package.elm-lang.org/packages/1602/elm-feather/latest
BSD 3-Clause "New" or "Revised" License
76 stars 4 forks source link

The toHtml produces an SVG element containing an SVG element (double nesting) #14

Closed tad-lispy closed 5 years ago

tad-lispy commented 5 years ago

Try inspecting the DOM of the icon here: https://ellie-app.com/4hJYQY82WJQa1 It looks something like:

<svg attrs>
  <svg attrs>
    <path attrs>
  </svg>
</svg>

I think it's because every icon contains Svg.svg element with fixed set of attributes (e.g. width and height) e.g. feather here

https://github.com/feathericons/elm-feather/blob/f64d398e3140c7e93075d38305937d15f7bf6835/src/FeatherIcons.elm#L1398-L1402

Then makeBuilder puts it in the src field of the record tagged by Icon constructor:

https://github.com/feathericons/elm-feather/blob/f64d398e3140c7e93075d38305937d15f7bf6835/src/FeatherIcons.elm#L224-L226

Finally toHtml wraps it in Svg.svg again

https://github.com/feathericons/elm-feather/blob/f64d398e3140c7e93075d38305937d15f7bf6835/src/FeatherIcons.elm#L214-L216

Probably a solution could be to change each icons' definition to only contain a list of paths, unwrapped from the SVG element, like this:

feather : Icon
feather =
    makeBuilder "feather"
        [ Svg.path [ d "M20.24 12.24a6 6 0 0 0-8.49-8.49L5 10.5V19h8.5z" ] []
        , Svg.line [ x1 "16", y1 "8", x2 "2", y2 "22" ] []
        , Svg.line [ x1 "17.5", y1 "15", x2 "9", y2 "15" ] [] 
        ]
1602 commented 5 years ago

Thanks for spotting and investigating. This is indeed a bug, which has been recently introduced because of api change in dependency feather-icons (npm package) which has been used to generate this package, which now works like this:

node
> require('feather-icons').icons['feather'].toSvg()
'<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-feather"><path d="M20.24 12.24a6 6 0 0 0-8.49-8.49L5 10.5V19h8.5z"></path><line x1="16" y1="8" x2="2" y2="22"></line><line x1="17.5" y1="15" x2="9" y2="15"></line></svg>'

Solution that you've suggested should solve an issue, and in order to do so I need to amend generator for this to cope with upstream library API change.

1602 commented 5 years ago

Bug fixed in 1.2.2.