connectivedx / Phoenix

http://connectivedx.github.io/Phoenix/
33 stars 5 forks source link

Optional Parameters as Maps #108

Closed ajmueller closed 9 years ago

ajmueller commented 9 years ago

This changes our functions and mixins to allow for optional parameters to be passed into the function or mixin as part of an optional map. Doing so will provide a couple advantages:

That said, sometimes the syntax is more verbose than before in a way that may not be 100% desirable, such as the syntax for em functions, gradients, or aspect ratio. Some additional ideas to consider/discuss:

ajmueller commented 9 years ago

After some discussion with @elseloop, I changed the aspect-ratio and gradient mixins to have only required parameters. That leaves the em function. I think I currently would prefer to err on the side of leaving context as optional and use a map for options, even though context is the only option. Thoughts?

NOTE: As noted above, #107 has been opened to research additional possibilities with maps and the gradient mixin.

elseloop commented 9 years ago

@ajmueller, this all looks great and I'm not seeing any holes in testing. :+1:

In answer to your question about documentation, I've opened #109 to give us a place to explore that more fully.

ajmueller commented 9 years ago

Thanks @elseloop! I'll take a look at #109.

If nobody has any objections, I'll merge this PR.