search

andrewgioia / mana

Magic: the Gathering mana symbol pictographic font
299 stars 32 forks source link

Split Cost/Hybrid Icons splitted by default? #30

Closed phrozen closed 5 years ago

phrozen commented 6 years ago

Is there a reason I'm not seeing for requiring the extra .ms-split class for hybrid classes and split costs? Parsing from MTGJson data requires an additional step, where classes like .ms-gw or .ms-2b should be splitted by default as there is no other use for them.

andrewgioia commented 5 years ago

@phrozen Not that I can think of honestly, I probably did it this way to reduce the lines of less a little bit and didn't really foresee the extra handling in an application. I'll remove the requirement to include .ms-split for these in this next update and keep the class for compatibility.

phrozen commented 5 years ago

Thanks!

I checked the code and you could easily use a mixin to include the code of .ms-split into all the hybrid classes and reduce a lot of lines of code. I also recommend separating all colors into variables for readability and a little less code.

andrewgioia commented 5 years ago

@phrozen 1.4.0 now has this!

I agree, I should be able to get to those optimizations soonish but if you want to submit a PR that'd be fantastic too.