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

feat: design token and theme package documentation #103

Merged
merged 12 commits into from
Sep 27, 2019

Conversation

SiTaggart
Copy link
Contributor

@SiTaggart SiTaggart commented Sep 20, 2019

Add a page for each of the Design Token package and Theme package, with install instructions and how to use. Similar to a component or utility page.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@vercel
Copy link

vercel bot commented Sep 20, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://paste-git-feat-theme-package-docs.twilio-dsys.now.sh

Copy link
Contributor

@TheSisb TheSisb left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@aayushpi aayushpi left a comment

Choose a reason for hiding this comment

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

Some text suggestions for clarity and approachability.

packages/paste-website/src/pages/tokens/theme-package.mdx Outdated Show resolved Hide resolved
packages/paste-website/src/pages/tokens/theme-package.mdx Outdated Show resolved Hide resolved
packages/paste-website/src/pages/tokens/theme-package.mdx Outdated Show resolved Hide resolved
packages/paste-website/src/pages/tokens/theme-package.mdx Outdated Show resolved Hide resolved
@SiTaggart
Copy link
Contributor Author

@aayushpi if you want to give it a pass I've done a once through now

@SiTaggart SiTaggart added Area: Doc Site Related to the documentation website Status: Pls CR This PR is ready for Code Reviews Type: Documentation Improvements or additions to documentation labels Sep 23, 2019
</SiteNavItem>
<SiteNavItem>
<SiteNavAnchor to={`${SidebarCategoryRoutes.TOKENS}/how-to-compose-custom-ui-with-tokens`}>
How to compose custom UI with tokens
Copy link
Contributor

Choose a reason for hiding this comment

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

image

This is kind of long. Can we rename it to `Composing with tokens`?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like an improvement to me but I'll wait for a consensus before changing it as it involves changing filenames and stuff and there maybe other ideas. @aayushpi @serifluous @two24studios for input


## The happy path

Obviously the first option is to work with your design partner and identify all the existing Paste components that can be used to create the experience. If you are struggling or have quesitons come to Design System Office Hours or post in the #help-design-system slack channel, we'll get you back on track.
Copy link
Contributor

Choose a reason for hiding this comment

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

quesitons -> questions

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we link to info on Office Hours?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we link to info on Office Hours?

Do we have a link?

Copy link
Contributor

Choose a reason for hiding this comment

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

{`<Box padding="space20">
<Text as="p" marginBottom="space40">Some text</Text>
<Text as="h1" fontSize="fontSize70" fontWeight="fontWeightMedium" lineHeight="lineHeight70" mb="space80">Custom heading</Text>
<Text as="ul" ml="space60">
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a Text or a Box?

If we're making it a Text, can we pass the fontSize to it and would the children elements receive the styling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So Text right now has a set default font-size, which matches what the system classes as the default body size. The reason being is that we font set a body size as you can't really override someone's application, and we don't actually know where this will be used so can't rely on any inheritance.

So even if I did set it on the ul, it won't currently affect the children as they set their own font-size anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. Is there any guidelines or issues with making a ul element a Text as opposed to a Box though?

@vercel vercel bot temporarily deployed to staging September 24, 2019 20:24 Inactive
@vercel vercel bot temporarily deployed to staging September 26, 2019 18:34 Inactive
@aayushpi
Copy link
Contributor

aayushpi commented Sep 26, 2019

One down, three to go - design tokens package is edited

@aayushpi
Copy link
Contributor

Added:

  • Text changes to Theme Package
  • Text changes to Design Tokens Package
  • Small rewrite to Guide for Custom UI
  • Rewrote package descriptions for theme and token.

@SiTaggart SiTaggart merged commit 3060113 into master Sep 27, 2019
@SiTaggart SiTaggart deleted the feat/theme-package-docs branch September 27, 2019 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Doc Site Related to the documentation website Status: Pls CR This PR is ready for Code Reviews Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants