Conversation
| classNames = classNames.concat(size.alias) | ||
| size = size.value | ||
| } | ||
| return `.f${classNames.join(', .f')} { font-size: ${size}rem; }` |
There was a problem hiding this comment.
100% absolutely love this. Only thing I wonder about is if we make the alias not auto prefixed with f?
So:
{value: 1.5, alias: ["copy", "error"]}Becomes
.f3, .title, .copy
And if you desire the f prefix you can add in the aliases: ["fcopy", "ferror"]. Might make it a touch more flexible. Thoughts?
There was a problem hiding this comment.
this PR for typescale is just a proof of concept. if we implement this, it should be generally available for all scales (spacings, heights, widths, etc...).
so omitting the class prefix leads to 2 problems:
- we have to validate the aliases globally so the user doesn't override his aliases across different scales
- it will be harder to understand the classes when reading them in html code, because if it says
class="error", you have no idea what exactly this class is defining. if it'sclass="ferror"you will always know (by tachyons naming conventions), this class has something todo with fonts
There was a problem hiding this comment.
Yeah, definitely agree it should be handled for all the things. I think we can look for identical aliases and warn when that occurs while also piping the output css through immutable-css to log any conflicting class names.
I'd like to see the ability to actually "eject" from the Tachyons naming conventions if they so choose, but that's prolly a bit different than aliasing. So perhaps aliases always are prefixed with the class base and one can actually change the class base when they desire something different:
{
typeScale: {
values: [1,2,3, { value: 4, alias: ['big', 'headline' }],
classBase: 'heading'
}
}I'll do some thinking on this over the weekend, as it'd be great to come up with a config that can handle all of this while still maintaining a sensible config api.
There was a problem hiding this comment.
also, I was wondering, why do you handle spacing different then the other settings?
https://github.com/tachyons-css/tachyons-generator/blob/master/lib/spacing.js#L54
with this step/ratio setting, we wouldn't be able to apply this proposal
There was a problem hiding this comment.
I think that was a poor api choice on my behalf. I think we should prolly move it to become inline with everything else (which should be okay since we're not 1.0.0 yet).
addressing my idea from #67