Skip to content

Conversation

@fk
Copy link
Contributor

@fk fk commented Mar 15, 2019

Attention, noooooooooob Friday stuff … potential public humiliation stuff 😅🥳

Since we want to define presets.space in relation to Typography.js’ rhythm/baseLineHeight, but doing so directly in presets would have introduced a circular dependency among utils/presets and utils/typography (from what I think I understand 🤓), up to now we had to put values from presets.space through rhythm for each usage in components:

import { space } from "../../utils/presets"
import { rhythm } from "../../utils/typography"

<div css={{
  padding: `${rhythm(space[7])} ${rhythm(space[4])} ${rhythm(space[4])}`
}} />

This caused a lot of unnecessary noise, and was an ugly derivation from the norm for that one group of design tokens in presets … and AFAICT smells a lot.

This PR cleans that up — as good as I can. I would appreciate any input and am happy to adjust this, crossing fingers that this PR at least shows where I want things to go:

import { space } from "../../utils/presets"

<div css={{
  padding: `${space[7]} ${space[4]} ${space[4]}`
}} />

TL; DR:

  • minimize direct usage of Typography.js’ scale and rhythm
  • maximize usage of presets tokens for consistency, and make that easy and as concise as possible

What's in the bag:

  • split up design tokens in utils/presets and move them to utils/tokens/* — theme-style FTW?!
  • move utils/colors to utils/tokens/colors
  • refactor to remove circular dependency among utils/typography and utils.presets — this enables us to use Typography.js’ rhythm in utils.presets so we can avoid manual „composition“ of space values in components
  • move paddingLeft helper function used in components/sidebar/accordion and components/sidebar/section-title to utils/sidebar/indention
  • adjust changes from docs: add API reference for params passed to gatsby-node APIs #12087 to use space design tokens instead of Typography.js' rhythm
  • move font stack definitions to tokens, and use presets.fonts in components instead of typography.options and join(,`)
  • realigned/uncluttered docs/tutorial/features, plugins sidebars:
    image .
    image
    • opt out of styling scrollbars in WebKit in favor of user settings on Mac OS X (where users can pick among always show scrollbars, show them depending on mouse/trackpad, show them while scrolling — our WebKit scrollbar CSS interfered with the latter two), and a perfectly fine scrollbar on Windows that users know and recognize
    • move padding to anchor/title — allows for full-width background/hover, etc.
    • calmer „Expand/Collapse All“ button
    • increase sidebar „utility“ height (containing the „Expand/Collapse All“ button) height/whitespace
    • move a couple sidebar variables to utils/sidebars/presets, enables configuration of a couple of things:
      • sidebar item hover background color
      • sidebar item borders — I decided to drop those for now, thought hard about if they are really needed; can be brought back via changing a color variable
      • minimum item height
      • active „section item“ background
    • sidebar background color for offscreen/small screens, and persistent/larger screens
    • adjust colors.ui
      • drop colors.ui.border
      • freshen up light, bright, and whisper
    • bump default sidebar width
    • simplify „stepsUI“ (used for anchor links in „Tutorial“) — remove dashed lines
    • reduce sidebar active item font weight — dial it back to 600, which should be supported by the majority of the system font stack families
    • improved distinction among top-level sections (via border-bottom and vertical whitespace)
    • sidebar expand/collapse chevron button now covers 100% of the item's height
  • realigned main navigation
    image
    • drop uppercase and tracking for main nav items — lowercase is (subjectively) more legible and friendly, takes less space, matches the mobile navigation items, and magically resolves space issues on screens below „lg“

fk added 5 commits March 15, 2019 20:59
Attention, noooooooooob Friday stuff … potential public humiliation stuff 😅🥳

Since we want to define `presets.space` in relation to Typography.js’ `rhythm`, but doing so directly in `presets`  would have introduced a circular dependency among `utils/presets` and `utils/typography` (from what _I_ think I understand 🤓), up to now we had to put values from `presets.space` through `rhythm` for each usage in components:

```
import { space } from "../../utils/presets"
import { rhythm } from "../../utils/typography"

padding: ${rhythm(space[7])} ${rhythm(space[4])} ${rhythm(space[4])};
```

This caused a lot of unnecessary noise, and was an ugly derivation from the norm for that one group of design tokens in `presets` … and of course smells a lot.

This PR cleans that up — as good as I can.

I would appreciate any input and I’m happy to adjust this, crossing fingers that this PR at least shows where I want things to go:

```
import { space } from "../../utils/presets"

padding: ${space[7]} ${space[4]} ${space[4]};
```

- minimize direct usage of Typography.js’ `scale` and `rhythm`
- maximize usage of `presets` tokens for consistency, and make that easy and as concise as possible

—

- split up design tokens in `utils/presets` and move them to `utils/tokens/*` — theme-style FTW?!
- move `utils/colors` to `utils/tokens/colors`
- refactor to remove circular dependency among `utils/typography` and `utils.presets` — this enables us to use Typography.js’ `rhythm` in `utils.presets` so we can avoid manual „composition“ of `scale` values in components
- move `paddingLeft` helper function used in `components/sidebar/accordion` and `components/sidebar/section-title` to `utils/sidebar/indention`
… instead of `typography.options` and `join(`,`)
Move font stack definitions to tokens.
@fk fk requested a review from a team March 15, 2019 21:38
fk added 9 commits March 17, 2019 18:16
Lowercase is (subjectively) more legible and friendly, takes less space, matches the mobile navigation items, and magically resolves space issues on screens below „lg“
- move padding to anchor/title — allow
- calmer „Expand/Collapse All“ button
- increase sidebar „utility“ height (containing the „Expand/Collapse All“ button) height/whitespace
- move a couple sidebar variables to `utils/sidebars/presets`, enables configuration of a couple of things:
  - sidebar item hover background color
  - sidebar item borders — I decided to drop those for now, can be brought back via changing a color variable
  - minimum item height
  - active „section item“ background
  - sidebar background color for offscreen/small screens, and persistent/larger screens
- adjust `colors.ui`
  - drop `colors.ui.border`
  - freshen up `light`, `bright`, and `whisper`
- bump default sidebar width
- simplify „stepsUI“ (used for anchor links in „Tutorial“) — remove dashed lines
- reduce sidebar active item font weight — dial it back to 600, which should be supported by the majority of the system font stack
- improved distinction among top-level sections (via border-bottom and vertical whitespace)
- sidebar expand/collapse chevron button now covers 100% of the item's height
…der bottom if there’s only one accordion
@fk
Copy link
Contributor Author

fk commented Mar 18, 2019

Good to go! #trustme

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in 🙌

@sidharthachatterjee sidharthachatterjee merged commit 39a14fa into master Mar 20, 2019
@sidharthachatterjee sidharthachatterjee deleted the topics/www-spring-cleaning-episode-5 branch March 21, 2019 08:44
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 this pull request may close these issues.

3 participants