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

Initial definition #1

Open
jxnblk opened this issue Mar 23, 2019 · 41 comments
Open

Initial definition #1

jxnblk opened this issue Mar 23, 2019 · 41 comments

Comments

@jxnblk
Copy link
Member

jxnblk commented Mar 23, 2019

This issue is meant to be the canonical place for discussion about the initial scope and details of the theme specification.

@jxnblk jxnblk pinned this issue Mar 23, 2019
@zethussuen
Copy link

zethussuen commented Mar 25, 2019

Hi Brent!

As a designer, I'm a huge fan of styled-system and all of the work you've done in this space. As an engineer, I constantly advocate for styled-system + typescript. Personally, I want to see some sort of type recommendation in theme-specification. I would love to be able to swap theme specs in and out and not worry whether widths have been defined as a number (10) vs string (10px, 10em, etc). I can also see how this may be restrictive. Would love to hear your thoughts on the topic.

@souporserious
Copy link

Super excited to see this getting started!

I've been thinking about this spec the last couple of days and wondering if you have any thoughts on how multiple sets of fontSizes could be handled? Similar to how Github uses two sets of font sizes across viewports. Do you think it would be best handled in each respective application? I'm thinking it would be nice to have that relationship accessible in the spec for things like documentation, tooling, etc.

@jxnblk
Copy link
Member Author

jxnblk commented Mar 29, 2019

@souporserious I'm hoping that the idea of scales being objects means that you would have one master fontSizes array or object, then use that as the basis for subsets like what you're describing. It could be additional custom fields like theme.mobileFontSizes and theme.desktopFontSizes or a nested object within the root scale, like theme.fontSizes.mobile. Just like colors, I think trying to make a one-size-fits all abstraction at the scale definition level will be very difficult, and this is more meant to start more from a where do I find values for font sizes perspective, if that makes sense

@jxnblk
Copy link
Member Author

jxnblk commented Mar 29, 2019

@zethussuen thanks! I think this initial spec is meant to be fairly loosely defined when it comes to what you put in the scales. That said, an extension of this spec could absolutely layer a type system on top – I think the PR here sort of leans that direction: styled-system/styled-system#432

@souporserious
Copy link

Totally! That makes sense. I like settling on a key-value structure that everyone can adhere to, which right now seems great to me 👍.

@jxnblk
Copy link
Member Author

jxnblk commented Mar 31, 2019

Just opened a PR for discussion about width and height properties here: #2

@Grsmto
Copy link

Grsmto commented Apr 4, 2019

Related to @zethussuen comment, we are using TypeScript with styled-system and the way you define aliases for a scale does not work in a strictly typed context because you are applying a value to an array, which is considered an error in TS.
I don't know if you want to consider TS for the spec but that's how we had to do to avoid the typing error:

const fontSizes = {
  0: '0.75rem', // 12px
  1: '0.875rem', // 14px
  2: '1rem', // 16px
  3: '1.125rem', // 18px
  4: '1.25rem', // 20px
  5: '1.5rem', // 24px
  6: '1.75rem', // 28px
  7: '2rem', // 32px
  8: '2.25rem', // 36px
  9: '2.625rem', // 42px
};

fontSizes.baseText = fontSizes[1];

This way the fontSizes is an object and not an array but still can be accessed just like an array.

@jxnblk
Copy link
Member Author

jxnblk commented Apr 4, 2019

@Grsmto Yeah, this spec should "work" for any language really. If you have to slightly alter what an object looks like for TS, I think that's totally valid. The aliasing example uses JS as a lingua franca to describe one possible approach to scale definition

@nniesen
Copy link

nniesen commented Apr 5, 2019

It's awkward that space is the only one that is collection and is not plural.

@timhudson
Copy link

I'm so glad to see this discussion and repo! After reading through the spec and everyones feedback, I wrote down my thoughts. This will be a bit of a dump, apologies in advance.

1. Static definitions JSON/YML

This has been discussed in styled-system/styled-system#432 (comment) and touched on above.

To increase the specs reach and potential adoption, it's certainly worth considering embracing a baseline of everything being statically defined. While any language could be used to generate a spec-compliant static theme, it's this common output that could be consumed predictably by tooling of any language or platform. I agree with the earlier sentiment that JSON would be a good choice.

However, we'd have to create or adopt a standard for referencing values. Style Dictionary solves this with a template-tag-esque approach.

2. Functions as values

This flies in the face of static definitions, but some scales could better be described through a function, especially when unbounded. A decent example is the space scale. This could reasonably be described as n => n * 1.5. Color scales could use a more complex function that understands color. Several systems use functions and omitting them here may limit this specs reach.

I'm on the fence as I would favor static definitions. Could they reasonably co-exist? It's a little convoluted but I could see this solved using function names which map to externally defined implementations.

"space": "fn:space" // gross prefix
export const space = n => n * 1.5

3. Abstracting away platform specific language/concepts

Considering how a design language can be applied across many platforms and technologies, how can we abstract away platform specific language without losing the specs simplicity and familiarity? While I don't think it's practical to have a completely generic spec, that sounds unnecessarily plain, I do wonder if we can take things a little further than where they are currently.

These stand out to me as potentially CSS specific:

  • mediaQueries
  • maxWidths, minWidths, maxHeights and minHeights?
    • I can't definitively say these are CSS specific but I'm skeptical
  • zIndices
  • Any mention of CSS outside the context of the Key Reference
  • Excluded Values could be reframed to discuss generally what is out of scope for this spec, using CSS as an example.

As for Key Reference mappings, we could make room for adding more key mappings beyond CSS. However, this has me questioning whether mappings should even be part of the spec. I'm tempted to consider that a tooling concern. I'm not sure.

4. Prior art

What makes this unique or more capable than existing solutions? Answering this can help folks see the value. For me personally, I see a potential ecosystem enabled by this spec. Knowing I could access tooling by adhering to spec can be compelling.

I'm sure there's more but here's the prior art I'm aware of:

  • Theo
    • Predictable Spec, easy to build tooling for
    • Doesn't support arrays or nesting
  • Style Dictionary
    • Supports nesting, not arrays
    • Supports aliasing

OK, I'll stop now 😝. I hope some of that is helpful in continuing this discussion.

@jxnblk
Copy link
Member Author

jxnblk commented Apr 7, 2019

Thanks @timhudson! This is super helpful!

  1. Static definitions JSON/YML

I agree that having a statically defined theme for this spec makes a lot of sense. Personally, I'd rather avoid inventing anything like the templating in Style Dictionary – I think that can be handled by whatever generates the theme object if needed.

  1. Functions as values

I think similarly, functions could be used to generate the object, but I don't think this spec has to deal with templates or functions at all. How the scales are defined should be outside of the scope here, IMO

  1. Abstracting away platform specific language/concepts

This point I probably disagree with. While other specs may try to "bridge-the-gap" across platforms, I don't think that should be a goal here. CSS is a fine spec, and I'd rather this theme specification build on top of that base (because I think it's good). For a lot of this, it's not too difficult to translate from web terms into proprietary platform terms or ignore parts like media queries where it isn't applicable.

  1. Prior art

Yes, prior art absolutely should be documented in this project somewhere. Until I proposed this, I'd never heard of Style Dictionary, and I do mention Design Tokens (which is what the Theo implementation is based on IIRC)in my blog post. If there are any others, please feel free to open a PR to include them.

@timhudson
Copy link

functions could be used to generate the object, but I don't think this spec has to deal with templates or functions at all

I've been thinking a lot about dynamic theme modification lately, such as mobile sizes or high-contrast colors, but I've mostly thought of this as something acting on static tokens. I really like your suggestion above. It's a good mental model and maybe worth documenting for anyone else attempting to find where dynamic theming can fit in with static theme definitions.

CSS is a fine spec, and I'd rather this theme specification build on top of that base (because I think it's good)

I agree. I'm not concerned with using, or even favoring, CSS terminology. It's familiar, stable and in-line with typical design tokens.

I'm more thinking about the concepts that don't port well to other platforms. Rather than single out any one example, it may be better to map out my thought process as I evaluate some of these keys:

Is the key…

  1. a commonly used term (colors) or visual concept (shadows)?
  2. broadly useful (space) or fundamental to visual systems (fontSize)?
  3. well, thats all I got. Ha!

it's not too difficult to translate from web terms into proprietary platform terms or ignore parts like media queries where it isn't applicable

I'm particularly interested in translation and portability. This is mostly where I'm coming from.

Regarding ignoring parts of the spec, the question I'm inclined to consider is how often or in how many contexts would folks ignore this key? If this number felt low, I would begin to ponder that keys value and the specs increased surface area.

My feedback above isn't really actionable right now as it's focused more on thinking than actual output. But hopefully that is within the nature of this issue.

@dakebl
Copy link

dakebl commented Apr 17, 2019

What do people think of merging all of font tokens into a single object?
I've been toying with it a little in experimentation:

// Create your initial tokens
const tokens = {
  font: {
    families: {
      body: 'sans-serif',
      heading: 'Helvetica, sans-serif',
      subHeading: 'serif',
      monospace: 'monospace',
    }, 
    sizes: [
      12, 14, 16, 20, 24, 32
    ],
    weights: [
      200, 300, 400, 600, 700, 900
    ]
  }
}

// Add any aliases you feel are useful
tokens.font.sizes.body = tokens.font.sizes[2]
tokens.font.sizes.h1 = tokens.font.sizes[5]
tokens.font.sizes.h2 = tokens.font.sizes[4]
tokens.font.sizes.h3 = tokens.font.sizes[3]
tokens.font.sizes.h4 = tokens.font.sizes[2]
tokens.font.sizes.h5 = tokens.font.sizes[1]
tokens.font.sizes.h6 = tokens.font.sizes[0]

tokens.font.weights.normal = tokens.font.weights[2]
tokens.font.weights.bold = tokens.font.weights[4]
tokens.font.weights.extraBold = tokens.font.weights[5]

I've also been playing with a typographically based root object that includes things like letterSpacings:

// Create your initial tokens
const tokens = {
  typography: {
    fonts: {
      body: 'sans-serif',
      heading: 'Helvetica, sans-serif',
      subHeading: 'serif',
      monospace: 'monospace',
    }, 
    fontSizes: [
      12, 14, 16, 20, 24, 32
    ],
    fontWeights: [
      200, 300, 400, 600, 700, 900
    ],
    letterSpacings: [
      1, 1.25, 1.5, 1.75, 2
    ]
  }
}

// Add any aliases you feel are useful
tokens.typography.fontSizes.body = tokens.typography.fontSizes[2]
tokens.typography.fontSizes.h1 = tokens.typography.fontSizes[5]
tokens.typography.fontSizes.h2 = tokens.typography.fontSizes[4]
tokens.typography.fontSizes.h3 = tokens.typography.fontSizes[3]
tokens.typography.fontSizes.h4 = tokens.typography.fontSizes[2]
tokens.typography.fontSizes.h5 = tokens.typography.fontSizes[1]
tokens.typography.fontSizes.h6 = tokens.typography.fontSizes[0]

tokens.typography.fontWeights.normal = tokens.typography.fontWeights[2]
tokens.typography.fontWeights.bold = tokens.typography.fontWeights[4]
tokens.typography.fontWeights.extraBold = tokens.typography.fontWeights[5]

I'd be interested to hear what others think?

@russpitre
Copy link

It's awkward that space is the only one that is collection and is not plural.

I like spacings

@zethussuen
Copy link

@dakebl I do like the idea of anything font-related being grouped together. I would go a step further and think about line-height pairings as well.

tokens.typography.fontSizes.body = tokens.typography.fontSizes[2]
tokens.typography.lineHeights.body = tokens.typography.lineHeights[2]

In terms of implementation, this feels quite verbose to type out all the time:

const Foo = styled.div`
  font-size: ${theme.tokens.typography.body}
  line-height: ${theme.tokens.typography.body}
`

I would love an interface that abstracts common pairings away and allows for something like:

const Foo = styled.div`
  ${theme.???.body}; // "font-size: 14px; line-height: 20px;"
`

@matthova
Copy link

These stand out to me as potentially CSS specific:

* `mediaQueries`

* `maxWidths`, `minWidths`, `maxHeights` and `minHeights`?
  
  * I can't definitively say these are CSS specific but I'm skeptical

* `zIndices`

Expanding on feedback from @dakebl, I have been referring to zIndicies as elevations, which I feel like is a bit more of a non-platform-specific way of thinking about element "height"

@VinSpee
Copy link

VinSpee commented Jun 26, 2019

as brought up in system-ui/theme-ui#85 (comment) - would CSS variables be allowed as values in the theme specification? Their types are unpredictable, but I can definitely see the need.

@jxnblk
Copy link
Member Author

jxnblk commented Jun 26, 2019

Worth mentioning here again, but I would like to avoid bikeshedding on space specifically. It stems from usage like white space and negative space – it's more of a quirk with the English language IMO. Most of the other names stem from CSS properties, but in this case margins or paddings would be too limiting in scope

@alex-page
Copy link

alex-page commented Jun 28, 2019

Yooo, I was wondering are units intentionally not a part of the examples in this specification or are they optional? Does this specification default to pixels? I think either way I would be happy to document it so not to confuse others.

Do you think that this would support global design properties such as reading direction ( left-to-right, right-to-left ) or potentially language (english, spanish, german)? These are potentially variables that could drastically change a design. It might not be the right fit but I am interested how this specification could support something like this. These unfortunately do not line up to CSS properties like the others 😞

@VinSpee
Copy link

VinSpee commented Jun 28, 2019

What's the purpose of separate keys for space and fontSize? Typically having a single key of size which encompasses both has been reliable for me.

Some downsides:

  • you're opening yourself up to font sizes that are too small. While a space entry of 4px might make sense, but a fontSize of 4 obviously would never be acceptable.
  • your "root" size becomes space[3] or space[4] in most cases, meaning you're probably always going to alias it.

Some upsides:

  • you're truly using the same scale for all "sizes", be it margin, font-size, etc.
  • you create a consistency where you know and can rely on what size[1] is, vs having to remember "is space[4] the same as fontSize[1], or is it space[3]?"

Any thoughts?

@alex-page
Copy link

alex-page commented Jul 19, 2019

Has there been any thoughts on splitting transitions into duration and timing-function. I am not too sure how transition is intended to be used but there might be multiple timing functions and duration's that could be mixed and matched together.

I also assume people don't want to add the property as they may use multiple animations on different properties e.g. opacity and transform.

This is how I am using it right now it feels a bit off:

{  
  transitions: [
    '100ms cubic-bezier(0.4, 0.22, 0.28, 1)',
    '150ms cubic-bezier(0.4, 0.22, 0.28, 1)',
    '300ms cubic-bezier(0.4, 0.22, 0.28, 1)'
  ],
}
/* Output from theme specification */
:root {
  --🍭-transition-1: 100ms cubic-bezier(0.4, 0.22, 0.28, 1);
}

.box {
  transition: opacity var(--🍭-transition-1);
}

I also have similar thoughts to the shadows and how they could or could not use color.

@mrmartineau
Copy link

@alex-page in design-system-utils which uses a tokens schema similar to system-ui, I did exactly what you describe, albeit with just one variation for the duration and timing function.

See one example here

const transitions = {
  duration: '300ms',
  timing: 'cubic-bezier(0.77, 0, 0.175, 1)',
}

and its usage here

  transition: {
    default: {
      duration: transitions.duration,
      timing: transitions.timing,
      transition: `all ${transitions.duration} ${transitions.timing}`,
    },
  },

@coreybruyere
Copy link

@Grsmto what does your theme interface look like to support aliases?

@Grsmto
Copy link

Grsmto commented Aug 20, 2019

@coreybruyere I created a Scale type like this:

export interface Scale<TValue> {
  [id: string]: TValue;
}

and I use it like:

const fontSizes: Scale<string> = {
  0: '0.75rem', // 12px
  1: '0.875rem', // 14px
  ...
  10: '3.75rem', // 60px
};

@coreybruyere
Copy link

Thanks @Grsmto - New to TS. How are you getting intellisense hints that don't just display the interface that was defined?

Screen Shot 2019-08-20 at 1 49 17 PM

@Grsmto
Copy link

Grsmto commented Aug 21, 2019

@coreybruyere I think it kinda depends of your setup, tbh TS with React in general is really complicated and with Styled-Components it's a nightmare (my opinion :) ).
So don't expect too much regarding auto-completion with this.
That being said, you can get it working but my setup is pretty hacky.

Screenshot 2019-08-20 at 21 17 36

Basically I managed to get this by defining the type on my styled component using the StyledProps type of SC like this: import { StyledProps } from 'styled-components';
And then you declare your component like:

styled.div<StyleProps>(({ theme }) => ({
   ...your styles
});

I would advice you to check the styled-components Github issues or Definitely Typed repo, there are resources there. Also that stuff I sending is a few months old, probably it changed since that, not sure.

@jkillian
Copy link

Yooo, I was wondering are units intentionally not a part of the examples in this specification or are they optional? Does this specification default to pixels?

I was wondering this as well. One benefit of keeping things unitless is that it's easier to do math since it's just a number as opposed to a string (i.e. theme.space[3] * 2 or whatever). I agree that it'd be great if the documentation talked a bit more about units!

@zce
Copy link

zce commented Nov 9, 2019

Should theme.sizes be applied to flex-basis?

@dance2die
Copy link

dance2die commented Nov 16, 2019

Any reason for

The space key is a specially-named
?

I believe a plural naming convention would make it

  1. memorable - No need for weird "one-off" (to borrow from the spec, unintentionally) rule to remember.
  2. consistent - conforms to a plural naming convention.
  3. readable - as theme => theme.spaces[0] makes it intentional that it contains a space list, not a space
    • As space would rarely ever contain one space.

I'd appreciate could you provide a reason, as I'd like to understand the design decision behind it and learn from it 🙂

@jxnblk
Copy link
Member Author

jxnblk commented Nov 18, 2019

In English, space can be an uncountable noun, like it is used here. The word spaces generally means something different

@dance2die
Copy link

In English, space can be an uncountable noun, like it is used here. The word spaces generally means something different

Thank you, @jxnblk. I wasn't aware of the difference.
Just learned the difference here https://www.italki.com/question/409560?hl=en,

You are right, space makes more sense, here :)

@nniesen
Copy link

nniesen commented Nov 26, 2019

But if you mean the area for specific purpose & you can actually count it, then you use the plural form.
For example: "parking spaces" or "spaces between books on the shelf".

It's a collection of space so it should be spaces. Just like fontSizes is a collection of fontSize.

@souporserious
Copy link

souporserious commented Nov 26, 2019

Any thoughts on spacings? It goes nicely with letterSpacings and has the benefit of aligning with Material and Apple language.

@gsklee
Copy link

gsklee commented Dec 5, 2019

Agree that "space" here is a misnomer... When we're talking about typography, "space"/"spaces" usually means the whitespace characters you put between other text characters, while "spacing"/"spacings" usually refers to certain space areas that are used or existing for layout purposes. @jxnblk we should change this.

@knowler
Copy link

knowler commented Dec 5, 2019

🙂 I think this is worth repeating:

I would like to avoid bikeshedding on space specifically. #1 (comment)

@kyranjamie
Copy link

kyranjamie commented Feb 3, 2020

Here to second a desire for functions defined within the theme definition. As said above, it may slightly go against the static nature of a theme spec, but there are too many benefits to ignore, and we can still conform to certain structures Function | TypeOfStaticStucture.

Firstly, it doesn't make sense to pre-calculate values, when a function will do.

Second, if you're really trying to enforce a consistent styled, it'd be great to proactively disallow illegal values. Just throw an error. This would be super helpful dev-time feedback from designers to developers.

Consider, you're following a multiples of 4 design style, but the designs don't use 7 * 4 = 28, only 24 and 32. SystemUI & Styled System will, in effect, silently fail if you enter this value. Maybe you've defined the value, or left it blank.

With a fn we could easily take this to the next level:

function spacing (multiple: number) {
  const allowedMultiples = [1,2,3,4,5,6,8,10,12,14,18];
  if (!allowedMultiples.includes(multiple)) {
    throw new Error('Illegal spacing multiple, see design system guide');
  }
  return multiple * 4;
}

@The-Code-Monkey
Copy link

I feel the same way as @jxnblk does with space as a base token, what could be done is with the styled system package is to add a way to override the keywords that the functions within styled system use e.g.

{
    scales: {
        space: "spaces"
    }
}

so on and so forth.

@ItsWendell
Copy link

3. Abstracting away platform specific language/concepts

Considering how a design language can be applied across many platforms and technologies, how can we abstract away platform specific language without losing the specs simplicity and familiarity? While I don't think it's practical to have a completely generic spec, that sounds unnecessarily plain, I do wonder if we can take things a little further than where they are currently.

These stand out to me as potentially CSS specific:

  • mediaQueries

Like @timhudson mentioned here, abstracting away these platform specific concepts is important, I think some customization here could be relevant for example in this situation:

I recently published the first version of emotion-native-extended, which adds better 'native' support for Emotion (CSS in JSX), including responsive media queries.

The whole reason I built this because I desperately wanted styled system to properly work with React Native. I had to write a specific condition though to replace the @media **screen** (min-width: 40em), part of a media query, since screen is not a media type in react native, and so this prevented the media queries to work.

@johnlobster
Copy link

Hi, just saw the spec, it's great idea

I've been thinking a bit, and I think your stated purpose is problematic

Should this be a portable template to be used to communicate the details of a theme definition ?

Should it be something that stores values that javascript programs can read and turn into css values?

Either way, I think that it would be reasonable for a particular tool or css library to read the information and turn it into a spec that works for that library. This gets rid of all sorts of issues

  • Typescript
  • using arrays or objects, the reader will have to look at the type and decide what to do
  • special features for a particular css library
  • use of web/css specific terms, communicate breakpoints, not @media for instance

Such a reader could have checks for unimplementable styles and be used as a tool to communicate this back to the designer

Two more things

  • Does the Adobe world have something similar?
  • how do wire frames type tools communicate theme information to the web coding team?

@azinasili
Copy link

@coreybruyere I created a Scale type like this:

export interface Scale<TValue> {
  [id: string]: TValue;
}

and I use it like:

const fontSizes: Scale<string> = {
  0: '0.75rem', // 12px
  1: '0.875rem', // 14px
  ...
  10: '3.75rem', // 60px
};

To piggy back off of this, if you prefer to use array's for scales instead of an object they can be typed like below:

interface Scale<S> extends Array<S> {
  small: S;
}

const space = [0, 8, 16, 24, 32] as Scale<number>;
space.small = space[1];

console.log(space.small); // returns `8`;

If you prefer not to use the as keyword it can be written like the following:

interface Scale<S> extends Array<S> {
  small?: S;
}

const space: Scale<number> = [0, 8, 16, 24, 32];
space.small = space[1];

console.log(space.small);

@hugobqd
Copy link

hugobqd commented Jan 4, 2021

Monospace in pre and code is misspelled :

current : font-family: Menlo,monspace;
correct: font-family: Menlo,monospace;

Capture d’écran du 2021-01-04 14-18-19

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

No branches or pull requests