Skip to content
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

[Format] Token value - string, number, array, object? #58

Closed
kevinmpowell opened this issue Sep 9, 2021 · 2 comments · Fixed by #71
Closed

[Format] Token value - string, number, array, object? #58

kevinmpowell opened this issue Sep 9, 2021 · 2 comments · Fixed by #71

Comments

@kevinmpowell
Copy link
Contributor

JSON supports string, number, array and object as data types. Which of these do we want to support for token values.

String:

{
  colorTextPrimary: {
    value: '#000000'
  }
}

Number:

{
  durationDefault: {
    value: 10
  }
}

Array:

{
  breakpoints: {
    value: ['320', '640', 1080]
  }
}  

Object:

{
  textHeadingLevel1: {
    value: {
      fontSize: 32
      fontWeight: 700
      lineHeight: 1.5
    }
  }
}

I suspect we'll need to support all four data types.

@c1rrus
Copy link
Member

c1rrus commented Sep 13, 2021

I'd say a token's value can potentially be any of the JSON types (also including boolean and null).

Of course, each of the DTCG types we define in our spec (color, dimension, duration, etc.) specifies exactly how values of that type must be formatted. So, when using those types (which I suspect most tokens would), we are narrowing down which underlying JSON type(s) those values can be. E.g. color values can only ever be JSON strings.

However, types we add in future and/or iterations of the current types may well be built on the full range of JSON types, so I see no reason to artificially limit their use.

Besides, this rule from the Types section pretty much necessitates all JSON types being available:

If the type property is absent, tools MUST treat values as one of the basic JSON types and not attempt to infer any other type from the value.

@kevinmpowell
Copy link
Contributor Author

@c1rrus I agree. I don't think we need to belabor this topic. I'll get a PR together to update the spec with this information so we can close this issue out.

github-actions bot added a commit that referenced this issue Sep 17, 2021
SHA: cca6827
Reason: push, by @kevinmpowell

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants