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

Reference modifiers #59

Closed
dbanksdesign opened this issue Feb 13, 2018 · 17 comments
Closed

Reference modifiers #59

dbanksdesign opened this issue Feb 13, 2018 · 17 comments
Labels
Core Architecture This is an issue related to the core architecture of Style Dictionary invalid

Comments

@dbanksdesign
Copy link
Member

Would it be useful to add in support to modify property references in a style dictionary?

Some examples:

  • Having a base font size, and modify by multipliers
  • Lighten or darken colors, although maybe not recommended because the developer would not have explicit control over exactly what the color will be. Multiply font sizes is easy to understand (16 * 1.5 = 24)
  • Something else?
@elliotdickison
Copy link

I'd be interested in helping with this. What API did you have in mind? I was thinking these could just be implemented as transforms which would keep the API minimal. For example a transform "value/math" could look for values like "math({font.size.base} * 1.5)" and evaluate them (after font.size.base is resolved). Not sure if "math" is the right term in this particular case, but things like "calc" and "eq" already have specific meanings for css.

@dbanksdesign
Copy link
Member Author

Sorry for the delay, I wrote something then lost it.

There are a few ways we could do it, one is like what you describe, another way we could do it is allow a 'modifier' attribute on a property/token that is an object that looks something like this:

{ 
  "value": "{font.size.base}",
  "modifier": {
    "multiply": 1.5
  }
}

Your way would be more flexible, but probably harder to maintain as we would need to eval that string or do some complex string parsing.

Another thing to think about is the order of things happening at build time. Maybe we need to rethink this, but right now the framework does transforms first (only transforming non-reference values), then it does the replacements, then it builds the files. My original thinking of this order is:

  1. Cuts down the number of transforms that need to happen to the minimal to speed up build time
  2. Allows the transforms to be triggered based on the original property. For example, all the built-in transforms are matched by the CTI in the attributes object, which is generated by the object path of the property. So anything in the 'color' category gets 'color/hex' transform, but you could potentially reference that property outside of the 'color' category (thinking ahead to component styles).

As it stands, we would need to flip the transform/resolve order either way I think, which may be better in the long run? I don't know. Let me know if this all makes sense. I think it warrants a good discussion and thinking about it.

@ryngonzalez
Copy link

I think the modifier property syntax makes the most sense here. Making it an expression that would be evaluated would be more accessible, but doesn't seem worth the tradeoff of the additional code would need to parse and evaluate it.

The mental model of resolving the references first, and then doing the transforms makes much more sense from a user perspective—generate final state of the system, then use that state to feed into transforms for output. One way you could do that is to resolve references (with modifiers) while still holding onto original category info on that value, so that you can tell how to map the value to a transform.

@chazzmoney chazzmoney added this to the 2.2 milestone Apr 17, 2018
@chazzmoney chazzmoney added the Core Architecture This is an issue related to the core architecture of Style Dictionary label Jun 6, 2018
@chazzmoney
Copy link
Collaborator

After further conversation, this is probably a bad plan. The main use case we see if in color modification or size modification (especially in creating a "ramp" of sizes or various saturations of a color). These groups can be generated using existing tools online, or could be part of a GUI to create or edit StyleDictionaries. Unless there are additional use-cases, we believe that the dictionary file itself should be explicit in defining the colors or sizes used.

@chazzmoney
Copy link
Collaborator

@elliotdickison @ryngonzalez @dbanksdesign would love your thoughts on this - should we explicitly say we are not doing this / close it?

@dbanksdesign
Copy link
Member Author

I vote that we shouldn't do this.

@ryngonzalez
Copy link

I think if we decide to forego any ability to how to derive individual values, we leave a lot of power to whatever tools we'll need to generate these color systems and type ramps. I think these use cases are very common (given how I've seen various companies define these systems), and that leaving it outside the responsibility of style-dictionary will just cause more work for a majority of users.

@elliotdickison
Copy link

elliotdickison commented Aug 11, 2018

I agree that the ability to derive/modify values is important. I also think that given the wide range of needs in that area some sort of flexible, pluggable, JS-based solution would be better than trying to come up with special domain-specific syntax in property values. Not sure what that would look like yet, but I think it would be good to consider.

@dbanksdesign
Copy link
Member Author

Oh yea with the recent addition of allowing any 'require'-able file (node modules and JSON files), you could write a JS file that does some magic of iterating over some values to create color ramps and then exporting the object. I would say that would be the way to do it, or to do any programatic creation of styles.

@ryngonzalez
Copy link

Oh yea with the recent addition of allowing any 'require'-able file (node modules and JSON files), you could write a JS file that does some magic of iterating over some values to create color ramps and then exporting the object.

Is there documentation on this @dbanksdesign?

@dbanksdesign
Copy link
Member Author

Unfortunately, no :/

Something to put on the backlog! General idea is rather than write a normal color.json property file, you could write it as a node module like this:

//...
module.exports = {
  color: {
    base: {
      red: generativeColorFunction('#F44336'),
      blue: generativeColorFunction('#2196F3'),
      green: generativeColorFunction('#4CAF50'),
//...
}

@ryngonzalez
Copy link

@dbanksdesign Ah nice! I think this would solve some of the issues I was imagining. We should definitely get documentation up for this. Is there an issue up for this describing what we need?

@chazzmoney
Copy link
Collaborator

@ryngonzalez Just added #116 - thanks for the recommendation!

@custa1200
Copy link

I could still see how this could be of benefit to be added. I like the modifier example, it's a simple model to understand.

@chazzmoney
Copy link
Collaborator

chazzmoney commented May 14, 2019

Similar functionality available via direct manipulation in JS, added in #89

@didoo
Copy link
Contributor

didoo commented May 15, 2019

Agree with @dbanksdesign and @chazzmoney here. If it's possible to do it via JS functions, then let's avoid adding complexity to the core "engine". Good documentation (and possibly an advanced example) is perfect, for me.

@chazzmoney
Copy link
Collaborator

With the addition of #89, most use cases are likely covered. We aren't looking at doing this currently. If you stumble upon this issue and have a use case that requires it, please open a new issue that references this one and we can look at adding it in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Architecture This is an issue related to the core architecture of Style Dictionary invalid
Projects
None yet
Development

No branches or pull requests

6 participants