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

Design Tokens: Switch to using new spacing tokens #1664

Merged
merged 1 commit into from
Aug 27, 2021

Conversation

ayeshakmaz
Copy link
Contributor

@ayeshakmaz ayeshakmaz commented Aug 27, 2021

Summary

First iteration of testing out design token usage w/i Gestalt components:

Layout.css, TextArea, SearchField, InternalTextField, and FormLabel

Links

  • Jira
  • [TDD](link to Paper doc)
  • [Figma](link to Figma file)

Checklist

  • Added unit and Flow Tests
  • Added documentation + accessibility tests
  • Verified accessibility: keyboard & screen reader interaction
  • Checked dark mode, responsiveness, and right-to-left support
  • Checked stakeholder feedback (e.g. Gestalt designers)

@netlify
Copy link

netlify bot commented Aug 27, 2021

✔️ Deploy Preview for gestalt ready!

🔨 Explore the source changes: a4f365f

🔍 Inspect the deploy log: https://app.netlify.com/sites/gestalt/deploys/6128fe63a463b000080566f6

😎 Browse the preview: https://deploy-preview-1664--gestalt.netlify.app

@ayeshakmaz ayeshakmaz added the patch release Patch release label Aug 27, 2021
@ayeshakmaz ayeshakmaz marked this pull request as ready for review August 27, 2021 21:22
@ayeshakmaz ayeshakmaz requested a review from a team as a code owner August 27, 2021 21:22
<Table>
<Table.Header>
<Table.Row>
<Table.HeaderCell>
Copy link
Contributor

Choose a reason for hiding this comment

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

This very much seems like something that could use some helper components to abstract the repeated elements…we should avoid hardcoding markup like this when possible

@@ -8,7 +8,7 @@
composes: xsCol12 from "./Column.css";
appearance: none;
min-width: 180px;
padding: 8px 40px;
padding: var(--space-200) calc(var(--space-500) + var(--space-200));
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting, I hadn't considered combining tokens to get in-between values. Is this something we want to encourage Pinboard devs to do as well? Or is it something we should advise against?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something we want to avoid, as it defeats the purpose of creating a limited set of tokens. But I put this in as a stress test and to avoid a design decision for right now. But it has been documented as something to re-evaluate once we know these space tokens are good to go in pinboard

@ayeshakmaz ayeshakmaz merged commit d755ec8 into pinterest:master Aug 27, 2021
@ayeshakmaz ayeshakmaz deleted the ayesha/space-tokens branch August 27, 2021 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch release Patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants