Giter VIP home page Giter VIP logo

Comments (16)

c1rrus avatar c1rrus commented on May 18, 2024 8

I'm definitely in favour of allowing both description and type on groups as well as tokens. Furthermore, if/when we add any other properties in future we should always consider if they are applicable to tokens, groups or both.

Note, that with type, I think its meaning when applied to a group should be subtly different to when it's applied to a token:

  • type on a token is explicitly stating what type that token is.
  • type on a group is setting a default type for all tokens within that group (and any nested groups) which applies unless the tokens have their own type property, which then takes precedence.

Example:

{
  "group 1": {
    "type": "color",

    "token A": {
      // This token has the type color because it inherits it from group 1
      "value": "#00ff00"
    },

    "token B": {
      // This token has the type dimension, because its own type property
      // takes precedence the inherited color type
      "type": "dimension",
      "value": "12rem"
    },

    "group 1.1": {
      "token B": {
        // This token also has type color because it inherits it from group 1
        // since that is the nearest group with a type property
        "value": "#ff0000"
      }
    },

    "group 1.2": {
      "type": "duration",
      
      "token C": {
        // This token has type duration because it inherits it from group 1.2
        // (group 1.2's type takes precedence over its parent group's type)
        "value": "150ms"
      }
  },

  "token X": {
    // This token is not within a group with a type, so it does not inherit a type.
    // It also does not have its own type property. Therefore, tools must fall back
    // to the JSON type of its value, which is "string".
    // This token therefore has type "string" (don't be fooled by its value which
    // *looks* like a color!)
    "value": "#abcdef"
  }
}

from community-group.

sbaechler avatar sbaechler commented on May 18, 2024 4

Could we name group types childrenType instead of type? This would be more specific. It would also prevent bugs when group nodes are changed to leaf nodes.

I disagree with @gossi here that the token file should just be a dumb interchange format. The format is targeted to humans first, not machine first.

I can see a lot of developers and designers editing the file directly with minimal help from tools. Therefore it should be as human-readable as possible. And #AAA should throw an error when used as a dimension type.

from community-group.

c1rrus avatar c1rrus commented on May 18, 2024 3

Thanks everyone for your comments. There's some interesting points being made and we're planning to discuss them at an upcoming spec editors meeting.

I think a few ideas & concerns have been raised in this thread, so I'll attempt to call out and summarise them here. Please comment if you feel this isn't accurate or I've missed out something.

  1. Is it OK for some group properties to be inherited by child tokens (e.g. type) and others apply to the group itself (e.g. description)
  2. Is it OK for groups to have properties that are inherited by child tokens at all?
    • Concerns have been raised that this might make the format harder to understand and/or parse. Also diffing could become tricky in some situations as the same information could be written in several ways.
  3. Assuming we want groups to be able to set a default type for tokens within that do not set their own explicit type, should we name that group propert something other than type to avoid confusion? (childrenType, tokenProperties.type and contains have all been proposed as alternatives)

from community-group.

dbanksdesign avatar dbanksdesign commented on May 18, 2024 3

I agree with @kevinmpowell's points. I would put developer experience (reading and editing) over parsing complexity, I think having an inheritable type property will greatly simplify writing design token files by hand. For which properties are inheritable and which aren't, I would think about it the same was as CSS properties and the cascade. Some CSS properties are inherited (color) and others are not (padding).

from community-group.

gossi avatar gossi commented on May 18, 2024 2

Setting rules on groups will mean this:

Groups have an impact on tokens within them - or - reading the inverse tokens become dependent on the group they are in.

Case A: Groups are only there for organizing tokens, then this shouldn't be allowed as it shouldn't matter in which group a token is in.

Case B: Groups become contexts. Means, moving a token from one group to another will change its meaning.

Let's make this an example:

{
  "colors": {
    "type": "color",
	"example-token": {
	  "value": "#AAA"
    }
  },

  "dimensions": {
    "type": "dimension",
	"example-token": {
	  "value": "#AAA"
    }
  }
}

So, what shall be the algorithm to resolve the value of a token? Possible scenarios for case B:

  • colors.example-token = #AAA
  • dimensions.example-token = 2730px (explanation: 0xAAA = 2730d, pixel is assumed)

If groups act as context, then the spec shall deliever a resolving algorithm for how to reach at the value. I can compare it to the heading resolving algorithm for html.

The benefit I see is in manually writing groups of tokens all of one type and only defining it once. More chill authoring tokens for sure.

I consider this spec to describe the storage of tokens only and would avoid situations where reading rules become necessary.

from community-group.

kevinmpowell avatar kevinmpowell commented on May 18, 2024 2

Thanks for the summary @c1rrus

  1. Is it OK for some group properties to be inherited by child tokens (e.g. type) and others apply to the group itself (e.g. description)

I vote "yes", this is okay. I think we can outline this in the spec when we define group properties and denote whether or not each property is inheritable. In my mind it's similar to whether or not javascript events bubble (though that's going from child -> parent).

  1. Is it OK for groups to have properties that are inherited by child tokens at all?
    Concerns have been raised that this might make the format harder to understand and/or parse. Also diffing could become tricky in some situations as the same information could be written in several ways.

Since one of the key principles of the format is that it be human readable and editable then I think we have to keep the ability for some group properties to be inheritable.

  1. Assuming we want groups to be able to set a default type for tokens within that do not set their own explicit type, should we name that group propert something other than type to avoid confusion? (childrenType, tokenProperties.type and contains have all been proposed as alternatives)

I prefer the simplicity of type so the key name is the same whether it's at the group or token level. If we changed it to childType, contains, defaultType or something else, it'd still mean the same thing. There's not a separate concept of a type at the group level that we need to deconflict with.

from community-group.

splitinfinities avatar splitinfinities commented on May 18, 2024 1

The implicit language for grouping is what is a little confusing to me as I wrote my prototype. From my perspective, groups are the default type for any object, nested or otherwise, until an explicit type is defined.

Let's look at one chunk of the group types sample as it's proposed today.

  "text-size": {
    "type": "dimension",

    "scale": {
      "0": {
        "value": "1rem"
      },
      "1": {
        "value": "1.25rem"
      },
      "2": {
        "value": "1.563rem"
      },
      "3": {
        "value": "1.953rem"
      }
    },

    "title": {
      "value": "{text-size.scale.3}"
    },
    "heading": {
      "value": "{text-size.scale.2}"
    },
    "body-copy": {
      "value": "{text-size.scale.0}"
    }
  },

This reads to me as "this token is not a group, it's a dimension".

I'd like to propose something like...

  "text-size": {
    "contains": "dimension",

    "scale": {
      "0": {
        "value": "1rem"
      },
      "1": {
        "value": "1.25rem"
      },
      "2": {
        "value": "1.563rem"
      },
      "3": {
        "value": "1.953rem"
      }
    },

    "title": {
      "value": "{text-size.scale.3}"
    },
    "heading": {
      "value": "{text-size.scale.2}"
    },
    "body-copy": {
      "value": "{text-size.scale.0}"
    }
  },

It may feel like this is a simple "change the language of type to be groupType in circumstances" sort of proposal, but I architect the tokens file as "group until otherwise denoted", therefore everything is a type of group until it is overridden.

Building a library of the groups of tokens that can be accessed later on for abstraction layers felt helpful in my prototype, plus labeling the group with contains felt to provide "logic" - i.e. every token within this group should inherit a type value from the closest value of contains. This is nice because it can help reduce the file size at scale.

Writing this proposal to avoid overloading the term "type" would seem to be quite helpful for implementors... correct me if I'm wrong too! My proposed modification helped me write more a more reasonable implementation, and craft formatting messages to give structured feedback that had felt reasonable for folks when working with this file during compilation.

But yeah, even adding the ability to do description on a group felt inherently beneficial.

Edit: Updated my language around the "group of tokens" paragraph to improve readability.

from community-group.

c1rrus avatar c1rrus commented on May 18, 2024 1

I've just opened PR #89 to update the spec to move group properties into a metadata object. Thanks to everyone who contributed to this discussion!

from community-group.

firede avatar firede commented on May 18, 2024

The way to define the token is very free, and the handwriting experience is good.
However, if the tokens are modified by tools, the consistency of the format is difficult to guarantee.
After each update, the token may have more unexpected diffs.

from community-group.

TrevorBurnham avatar TrevorBurnham commented on May 18, 2024

I'd suggest that properties specified on a group for tokens within that group to inherit should be specified in a different way than properties that apply to the group itself. Otherwise, there's ambiguity over which properties are inherited and which are not. (For example, is a group-level description inherited?)

My proposal would be something like:

{
  "colorText": {
    "description": "Our palette of colors for text only.",
    "tokenProperties": {
      "type": "color"
    },
    "primary": {
      "value": "#000000",
      "description": "Use as the default text color"
    },
    "secondary": {
      "value": "#333333",
      "description": "Use for text that is not as important."
    }
  }
}

In this example, colorText is a group with only one property (a description), and all tokens within that group inherit a different property (type: "color"). This allows for groups to contain any number of properties of their own, along with any number of inherited properties.

Addendum: Avoiding name collisions
What if a design system wants to use the same name for a token as for a group property? For example, imagine that the team wants to have a text color called "description". An extension to accommodate this would be to allow group properties to be defined within a properties group. For example:
{
  "colorText": {
    "properties": {
      "description": "Our palette of colors for text only.",
    },
    "tokenProperties": {
      "type": "color"
    },
    "description": {
      "description": "Use for descriptive text, including definition lists and footnotes.",
      "value": "#3f3f3f"
    }
    // other tokens...
  }
}

The token/group names "properties" and "tokenProperties" would be disallowed, but anything else would be valid.

from community-group.

jjcm avatar jjcm commented on May 18, 2024

As a followup to this, are values allowed at a group level? Example as follows:

Let's say I want to define these css vars in a .token.json file:

--background-primary: #333;
--background-primary-hover: #444;
--background-primary-active: #222;

Can I define a value for a group like so?

{
    "background": {
        "primary" {
            "value": "#333",
            "type: "color",
            "hover": {
                value: "#444",
            },
            "active": {
                value: "#222",
            },
        },
    },
}

from community-group.

sbaechler avatar sbaechler commented on May 18, 2024

Adding a type property on group level might not be such a good idea after all. It would significantly increase the complexity.
While adding the type to each node can be tedious for manual editing, tools wouldn't mind.

Comparing a diff would be easier because one wouldn't have to worry about context.

So instead of trying to solve this problem it might be better to just remove it altogether.

from community-group.

splitinfinities avatar splitinfinities commented on May 18, 2024

A couple other fleeting thoughts I had after I posted the above:

  • it feels like there is a decision here to be made around inheritance for type's, which if this file assumes implicit inheritance, then this may altogether may be a non-issue. The proposal would need to over communicate that for consumers since inheritance is not built into JSON. A similar argument can be made here for description - are we describing the group or passing one value down to the tokens themselves? We need that as a clarifying point. This may be contentious too.

  • if we do not go inheritance, then a signal that isn't type feels necessary. Even if it was plural - types - instead of my idea above of contains, that feels like it would be helpful.

from community-group.

ChucKN0risK avatar ChucKN0risK commented on May 18, 2024

I agree with @dbanksdesign 👍

from community-group.

kevinmpowell avatar kevinmpowell commented on May 18, 2024

Action: Spec to be updated indicating $type at the group level will be inherited by tokens within the group.

Group level $description will not be inherited by tokens and serves as a description for the group.

from community-group.

c1rrus avatar c1rrus commented on May 18, 2024

The latest spec draft includes $type and $description properties for groups and clarifies that types are inherited but descriptions are not. Therefore closing this issue.

from community-group.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.