search

ocsigen / tyxml

Build valid HTML and SVG documents
https://ocsigen.org/tyxml/
Other
166 stars 62 forks source link

Add svg fill-rule attribute #294

Closed dedbox closed 2 years ago

dedbox commented 2 years ago

Hello!

I noticed that there is a `Fill_rule constructor in Svg_types.presentation_attr, but I could not find an attribute function that uses it. So, this PR adds an a_fill_rule attribute function that accepts `Nonzero or `Evenodd.

See https://www.w3.org/TR/SVG11/painting.html#FillRuleProperty for details on the fill-rule attribute.

I believe this branch is at least mostly ready for review. Since this is my first contribution, I'm not entirely sure. I have succeeded in using a_fill_rule from my local fork in another project, and the existing unit tests are passing, but I have not added any new unit tests.

Drup commented 2 years ago

Looks great, thanks !

Since it has a new type and a new ppx rule, please add a least a small test there, and then I'll merge.

dedbox commented 2 years ago

Nice! Since there are only two valid inputs to a_fill_rule, I just added checks for both to each of the jsx and ppx test suites.

Drup commented 2 years ago

Thanks @dedbox ! :)