-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Errors in "W3C" output #8
Comments
Thank you for the detailed feedback @c1rrus! This is really helpful. We really appreciate the suggestions and comments, the Design Tokens spec was the main inspiration for this tool. We will prioritize these issues for our next weekly release. Regarding the output tab label, alongside the name change we will add a disclaimer below the code with a link to the DTCG. We definitely don't want to mislead users with the naming. |
Wow, I wasn't expecting a response so soon. That's amazing! 🙌 Let me know if you have any questions about the format and I'll do my best to help out. As for the name on the tab, I think "DTCG" is probably fine for now. There isn't really a broad consensus yet on how to refer to the DTCG's file format. We're currently gathering feedback and ideas here: design-tokens/community-group#164 Feel free to make suggestions there! |
Hello @c1rrus ! As @acapeletto said, thank you for the feedback. I have a few questions about how you guys see we could align the app "Group types" with the spec's "Token types". For example, the We currently have something like:
wich is wrong, as @c1rrus pointed out. If we are only using cubicBezier functions, then we could do something like so:
and if we have different timing functions we could add tokens with their specific type
I have to review the rest in a little bit more detail. |
In the current draft we only support Cubic Bézier values. I suppose you could replace CSS keywords like However, there's currently no way to represent values like Feel free to raise an issue or comment on a suitable existing one to propose how we might add support for those kinds of values. |
Maybe what we have to define is what should be the source of truth for the app @acapeletto. The user could choose/filter what type of values wants to accept. There could be a dropdown somewhere that lets you pick what type of values you want: Supported by CSS, or Android, or iOS, etc. Then we should implement inputs with masks/dropdowns depending on the property, etc. |
First off, let me quickly say thank you for making this lovely tool. It's great to see new kinds of design token tools appear and, as one of the editors on the DTCG format, I'm also very excited to see tools that support the (draft) DTCG format! 😄
That being said, the output in the "W3C" tab1 does not currently conform to the current DTCG draft specification. Specifically:
spacing
type, you should outputdimension
tokens instead.duration
type must be in milliseconds, so"3s"
should be converted to"3000ms"
. Also, the spec does not support the valuepaused
(if you feel it should, you're welcome to propose that in a new issue).easing
type, you should outputcubicBezier
tokens instead. Note thatcubicBezier
values need to be plain JSON arrays, so"cubic-bezier(0.12, 0, 0.39, 0)"
should be converted to[0.12, 0, 0.39, 0]
.transition
type.radius
type, you should outputdimension
here as well.dimension
values can only bepx
orrem
, the DTCG format doesn't currently have a proper way to represent % values (it probably should though!). You could fall back to using thestring
type for now, but be aware that type will be removed soon (another reason to add a proper type for percentage values to the spec! 😅).opacity
type. The current draft does permit anumber
type, which you could use for now. Just likestring
above, that will be removed soon too. However, I see quite a few other tools supporting opacity tokens, so we'll need to add something to the spec to support those. Feedback and ideas welcome!shadow
tokens aren't valid - they need to follow the structure defined in the shadow type section.shadow
type does not support multiple shadows. Hower, support for that has been proposed and I think it's likely that we'll update the spec to support that. Until then you could potentially split each shadow into a separate token and maybe append a number to the name or something like that (hacky, I know - but that's the best we can do right now).mediaQuery
type, you should outputdimension
tokens there too.fontFamily
tokens are probably not quite right. Right now you're outputting the whole font stack as a single string. Syntactically that's valid, but according to the spec tools should interpret that as a single font family name (so you're saying there is 1 font and it is called"Palatino Linotype, serif"
). Since these appear to be font stacks, they should be converted into arrays like["Palation Linotype", "serif"]
. See thefontFamily
type chapter for details.fontSize
orletterSpacing
types, you should outputdimension
tokens there too.lineHeight
type, you should outputnumber
ordimension
tokens there instead (with the caveat that thenumber
type will be going away soon).Finally, you probably shouldn't add the
tokens
groups. The current output is like this:However, the name
tokens
has no special meaning in the spec. So this is interpreted as: There is a group called "Color", which contains a nested group called "tokens", which contains a design token called "background-body". Since tools that convert to code would typically use the path to a token, the corresponding CSS output should be something like--color-tokens-background-body: #233043
. That probably wasn't your intent. The solution is to just remove those intermediatetokens
groups.Putting it all together, the corrected "W3C" (DTCG) output should look something like this:
In principle, I'd be willing to submit PR(s) to fix these things. However, I'm pretty busy at the moment so probably won't have time to contribute fixes anytime soon. I therefore hope you don't mind me raising this issue to at least list out the issues in case someone else would like to try and fix them.
Footnotes
Btw, we would be grateful if you could relabel that to something like "DTCG". While the DTCG is a W3C community group, we are not a W3C working group and our technical reports are not on a W3C standards track. More info here: https://www.designtokens.org/press-kit/ ↩
The text was updated successfully, but these errors were encountered: