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

WIP: script to create a property-variable map for autocomplete & stylelint #664

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

siddharthkp
Copy link
Member

@siddharthkp siddharthkp commented Jun 23, 2023

(Do not merge, see demo in https://github.com/github/primer/issues/2008#issuecomment-1604286039)

This is a first draft, the implementation is based on running filter functions on generated variables (tokens-next-private/**/*.css

Example:

paddingBlock: ({type, name, file}) => {
    if (file.name === 'size.css') {
       if (type === 'base') return true
       else if (name.includes('padding') && !name.includes('paddingInline')) return true
    }
},

generated output:

{
  "paddingBlock": [
    { "name": "--stack-padding-normal", "value": "1rem" },
    { "name": "--stack-padding-spacious", "value": "1.5rem" },
    { "name": "--base-size-4", "value": "0.25rem" },
    { "name": "--base-size-8", "value": "0.5rem" }
  ]
}

see full file


I'd like to explore adding property information to each token and treating intellisense as a platform as an alternative for this script

example:

 {
  "control": {
    "stack": {
      "padding": {
        "condensed": {
          "$value": "{base.size.8}",
          "$type": "dimension",
+         "$property": "padding"
        },
        "normal": {
          "$value": "{base.size.16}",
          "$type": "dimension",
+         "$property": "padding"
        },
        "spacious": {
          "$value": "{base.size.24}",
          "$type": "dimension",
+         "$property": "padding"
        }
      }
    }
  }
}

@changeset-bot
Copy link

changeset-bot bot commented Jun 23, 2023

⚠️ No Changeset found

Latest commit: c6ae056

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2023

Variables changed
No variables changed

@siddharthkp siddharthkp temporarily deployed to github-pages June 23, 2023 12:51 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview June 23, 2023 12:51 Inactive
@siddharthkp siddharthkp temporarily deployed to github-pages June 23, 2023 13:28 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview June 23, 2023 13:28 Inactive
@siddharthkp siddharthkp temporarily deployed to github-pages June 28, 2023 14:12 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview June 28, 2023 14:12 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview June 28, 2023 17:59 Inactive
@siddharthkp siddharthkp changed the title WIP: script to create a property-variable map for intellisense WIP: script to create a property-variable map for autocomplete & stylelint Jul 4, 2023
@lukasoppermann
Copy link
Contributor

lukasoppermann commented Jul 12, 2023

Hey @siddharthkp to make the figma token plugin work, I am adding scopes to tokens: https://github.com/primer/primitives/pull/665/files#diff-a22f7c98b98666dfd7ade4b02294caee8eef00a58a4c80b0584f845ec3383d2eR10

I need specific scopes but I am sure we could work something out that works for both cases.

Any additional meta data should be added into the $extensions property according to the w3c specs.

I currently nest my stuff into a org.primer.figma prop, but we can rename this to work for both our cases.

This is what I currently use:

$extensions: {
        'org.primer.figma': {
          collection: 'mode',
          mode: 'dark',
          scopes: ['fgColor'],
        },
      },

The scopes I have are:

  all: ['ALL_SCOPES'],
  radius: ['CORNER_RADIUS'],
  size: ['WIDTH_HEIGHT'],
  gap: ['GAP'],
  bgColor: ['FRAME_FILL', 'SHAPE_FILL'],
  fgColor: ['TEXT_FILL'],
  borderColor: ['STROKE'],

I will need to be able to keep up the mapping in a way so that the output stays the same, but we can add more granular scopes, e.g. padding, margin, gap could all map to GAP in figma but stay separate for your case.

@siddharthkp
Copy link
Member Author

Ooooh, that seems like the one!

but we can add more granular scopes, e.g. padding, margin, gap could all map to GAP in figma but stay separate for your case.

That would be perfect!

@siddharthkp
Copy link
Member Author

siddharthkp commented Jul 12, 2023

I currently nest my stuff into a org.primer.figma prop, but we can rename this to work for both our cases.

I'd suggest we start in another extension key, something like "org.primer.vscode", and then we can merge them once we have confidence in the output?

Don't want to come in your way with my experimental gibberish 😅

@lukasoppermann
Copy link
Contributor

I'd suggest we start in another extension key, something like "org.primer.vscode", and then we can merge them once we have confidence in the output?

Great idea. 👍

Comment on lines +211 to +219
// const unusedVariables = logUnassignedVariables()

// if (unusedVariables.length > 0) {
// // eslint-disable-next-line no-console
// console.log(`Found unused variables, failing build`)
// // eslint-disable-next-line no-console
// console.log(unusedVariables)
// process.exit(1)
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, I disabled this temporarily until work continues.

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