greymatter-io / gm-ui-components

A library of reusable Grey Matter UI components.
MIT License
15 stars 6 forks source link

Refactor Spacing #479

Open shanberg opened 5 years ago

shanberg commented 5 years ago

Spacing

spacing() defines a new spacing system for component layout, for improved consistency and predictability. spacing() is a styling function that transforms a theme's SPACING_BASE value into a set of 10 rem-based size declarations.

image

Updated Spacing Scale

spacing(1) = "0.125rem"   // 2px
spacing(2) = "0.25rem"    // 4px
spacing(3) = "0.5rem"     // 8px
spacing(4) = "1rem"       // 16px
spacing(5) = "2rem"       // 32px
spacing(6) = "4rem"       // 64px
spacing(7) = "8rem"       // 128px
spacing(8) = "16rem"      // 256px

Converting from spacingScale() to spacing()

Converting from a legacy spacingScale() factor to the new spacing() factor follows this formula: 2^( OLD_SPACING_SCALE_FACTOR / OLD_SPACING_BASE ) / 16

spacingScale(0.25)  → spacing(1)
spacingScale(0.5)   → spacing(2)
spacingScale(1)     → spacing(3)
spacingScale(1.5)   → spacing(3.586)
spacingScale(2.x)   → spacing(4)
spacingScale(3)     → spacing(4.585)
spacingScale(4.x)   → spacing(5)
spacingScale(5)     → spacing(5.322)
spacingScale(6)     → spacing(5.585)
spacingScale(7)     → spacing(5.8075)
spacingScale(8)     → spacing(9)
spacingScale(9)     → spacing(9/10)
spacingScale(10)    → spacing(9/10)

While it’s possible to convert directly to the same pixel value using the new scale, it’s recommended in most cases to consider using an integer factor on the new scale:

spacingScale(0.25)   → spacing(1)
spacingScale(0.5)    → spacing(2)
spacingScale(1.x)    → spacing(3)
spacingScale(2.x)    → spacing(4)
...

Tasks

tilleryd commented 5 years ago

@shanberg For converting spacingScale to spacing, does spacingScale(6) → spacing(8/9) mean spacing(8) or spacing(9)?

shanberg commented 5 years ago

@tilleryd It means there is no direct mapping from one to one, and you'll have to choose based on the context.

shanberg commented 5 years ago

I realized enumerating the SPACING_X values in the theme doesn't make sense if that work is also done in the spacing() function.

I'm thinking now we should either define only SPACING_BASE in the theme, and spacing() should do the work of scaling that value up and down, or rewrite spacing() to read the series of SPACING_X values from the theme itself.

tilleryd commented 5 years ago

I'm thinking now we should either define only SPACING_BASE in the theme, and spacing() should do the work of scaling that value up and down, or rewrite spacing() to read the series of SPACING_X values from the theme itself.

That sounds right to me.

shanberg commented 5 years ago

One issue I've found when testing this implementation is that it’s impossible to change the spacing values once they're plugged in.

If we had say... SPACING_SCALE = ["1rem", "2rem"]

And spacing() simply pulls from the SPACING_SCALE array by index,

spacing(2): "2rem"

Later, if we added an intermediate value to the array... SPACING_SCALE = ["1rem", "1.5rem", "2rem"]

Then that same nice spacing() function we had changes...

spacing(2): "1.5rem"

This breaks everything.

I'm not comfortable saying the fixed scale of 0.0625–6 is all we'd ever use, so I'll have to think of a different way to implement this, and I'm pretty sure we're going to lose the elegance of spacing(nice_number).

shanberg commented 5 years ago

@tilleryd

I've rewritten spacing() to just take a number, but its output scales geometrically (I think? I didn't do great at math).

Math.pow(2, factor) / 16 + "rem";

This means the series of whole small number inputs gives us...

Function Output in rems Default in pixel size
spacing(1) 0.125 2
spacing(2) 0.25 4
spacing(3) 0.5 8
spacing(4) 1 16
spacing(5) 2 32
spacing(6) 4 64
spacing(7) 8 128
spacing(8) 16 256

Which...

  1. Covers the major values from the original spacing function
  2. Nudges users to use whole numbers, which reinforce the overall vertical rhythm and hierarchy
  3. Still allows for odd values spacing(3.25) = 0.5946035575013605rem = 9.5px when necessary