Skip to content

Conversation

@aaronccasanova
Copy link
Member

WHAT is this pull request doing?

This PR updates the CustomProperty component to use the recently added JSON tokens.

  • Deleted TypeScript design token objects
  • Replaced tokens directory with a single tokens.ts
    (This simplifies the implementation while still maintaining an opinion on the structure of the tokens object and how it's consumed in TypeScript files)
  • Updated tokens.ts to import the JSON token files

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

@aaronccasanova aaronccasanova added 🤖Skip Changelog Causes CI to ignore changelog update check. Get admin on Polaris mainline labels Dec 9, 2021
@aaronccasanova aaronccasanova self-assigned this Dec 9, 2021
@BPScott
Copy link
Member

BPScott commented Dec 9, 2021

https://github.com/Shopify/polaris-react/blob/main/loom.config.ts#L154 is a demo of adding additional rollup plugins. You'll need to add @rollup/plugin-json.

Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

🤩

return `${parseInt(px, 10) / BASE_FONT_SIZE}rem`;
function rem(value: string) {
return value.replace(
// https://regex101.com/r/RBL7EE/1
Copy link
Member

Choose a reason for hiding this comment

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

These links are temporary right? Not sure it's worth leaving as a comment. Easy enough just to copy and paste and add a few examples

Copy link
Member Author

Choose a reason for hiding this comment

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

image
In addition to the above screenshot, I verified in their GitHub API and FAQ wikis these are permalinks. I actually like having this reference as we can make sure the existing test cases still pass during future updates. That said, I'm also fine with removing the link as it's now checked into source control if we ever need it.

@aaronccasanova aaronccasanova merged commit c4f3d2e into feat/json-tokens Dec 9, 2021
@aaronccasanova aaronccasanova deleted the feat/json-token-custom-properties branch December 9, 2021 22:24
@aaronccasanova aaronccasanova restored the feat/json-token-custom-properties branch December 10, 2021 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖Skip Changelog Causes CI to ignore changelog update check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants