Skip to content

Conversation

@pieh
Copy link
Contributor

@pieh pieh commented Feb 25, 2019

This includes changes from #11597 which is needed for nested and also some really unrelated changes (to make preview work on netlify). Actual changes for review are limited to pieh/gatsby@58c6855...pieh:www-doc-preview

.org preview available at https://www-doc-preview--tender-curie-facda8.netlify.com/docs/node-api-helpers/

TO-DO before the merge:

Closes #4120

@pieh pieh requested review from a team February 25, 2019 22:00
@KyleAMathews
Copy link
Contributor

Page looks great! Excited to get this out there.

Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

I agree this looks great - looking forward to seeing it merged! Sorry for the review spam :(

Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

@jlengstorf jlengstorf left a comment

Choose a reason for hiding this comment

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

This is so exciting! Thanks for doing this!

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

This is amazing! I added lots of comments which are mostly questions. Great work! 💪

import { Header } from "./utils"
import { options, scale } from "../../utils/typography"

const Optional = styled.span`
Copy link
Contributor

Choose a reason for hiding this comment

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

unsure if screen readers read the content field. ping @marcysutton

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good point. It probably makes sense to adjust. I'm just not sure what semantic structure actually makes sense to use here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes they do read it! It becomes part of the computed text for an element node.

@m-allanson
Copy link
Contributor

Is this basically ready to go? @pieh do you want to handle the final TODO list you mentioned in the description and get this merged in?

I think there's huge value in in getting these docs live on .org, then we can follow up with any smaller changes as needed.

@pieh pieh force-pushed the www-doc-preview branch from 8e93a10 to a23f5b2 Compare March 13, 2019 21:25
@pieh pieh merged commit 6459e4e into gatsbyjs:master Mar 13, 2019
fk added a commit that referenced this pull request Mar 15, 2019
sidharthachatterjee pushed a commit that referenced this pull request Mar 20, 2019
* feat(www): Spring cleaning episode 5

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`

* Tidy stuff from #12087

* Tidy

* D’oh

* Use `presets.fonts` in components

… instead of `typography.options` and `join(`,`)
Move font stack definitions to tokens.

* 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“

* Fix refactoring fails

* Drop WebKit scrollbars for starters, showcase

* Drop WebKit scrollbar styles for docs/tutorials, plugins sidebars

* Realign docs/tutorial, plugins sidebars

- 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

* Fix padding-right for subitems with caret

* Move `level` info to `item-list`, hide level 0 expanded accordion border bottom if there’s only one accordion

* Remove unused

* Fix apostrophes
@pieh pieh deleted the www-doc-preview branch March 28, 2019 18:37
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.

7 participants