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

New CSS properties for analyzer: (border radius, font-weight, letter-spacing, etc) #301

Open
KenjiCrosland opened this issue Feb 27, 2023 · 6 comments
Labels
✨ enhancement New feature or request

Comments

@KenjiCrosland
Copy link

Hey @bartveneman I'm working on a project that makes use of the css-analyzer to determine if the designs on a particular page are in alignment with our design tokens.

I noticed a new property for "line-height" was added about a week ago. There are a couple more properties I'd like to add to the analyzer if possible. Would it be possible to raise some PRs to include these properties?

Thanks,

Kenji

@bartveneman
Copy link
Member

Makes sense to have some of those in here as well, but I wonder to what point they'll be useful as 'standalone' reports: the point of a design system is the combination of font-families/sizes/weights/line-heights. The individual tokens themselves don't add much value.
I wonder if you expect these results to end up on projectwallace.com, or do you have another use case in mind for them?

@bartveneman bartveneman added the ✨ enhancement New feature or request label Feb 28, 2023
@KenjiCrosland
Copy link
Author

KenjiCrosland commented Feb 28, 2023

I've been using the raw data from the tool and comparing them against our token values in the design system showing a number of "incorrect" values vs. "correct" values -- a report card of sorts. This gives us a good idea of the level of adoption of our tokens for a given page of our site. I'm not using projectwallace.com for this.

I think you're right that font-weight and letter-spacing are less valuable because they are often tied to line-height, font-family and font-size which you already have. Generally if those properties match up then the others will too.

So I suppose the one I'd like to add is border-radius.

You can see our full token list here: https://rei.github.io/rei-cedar-docs/tokens/all-tokens/

Maybe spacing and inset (margin/padding values) but that may generate too much noise.

@bartveneman
Copy link
Member

That's a nice looking system! I'll take your suggestions in consideration.

Judging by your use case I should breathe in some new life into Constyble. Second time this week where it could be really useful. https://github.com/projectwallace/constyble

@bartveneman
Copy link
Member

Note to self: need to check for the following properties:

With vendor prefix check:

Without vendor prefix check

@bartveneman
Copy link
Member

@KenjiCrosland working on this issue, but I wonder how useful it is to cram everything under an single property. Any of these values could be set from a shorthand (border-radius), but also as one of the individual properties. Auditing this list would be pretty difficult, I imagine.

These are some examples from real-life websites:

{
        "0": 69,
        "6px": 109,
        "0 0 6px 6px": 2,
        ".5em": 1,
        "1px": 1,
        "6px 6px 0 0": 2,
        "4px": 46,
        "8px": 45,
        "50%": 3
}
{
        "0": 67,
        "8px": 9,
        "11px": 213,
        "2px": 22,
        ".125rem": 2,
        "6px": 4,
        "5.5px": 4,
        "4px": 4,
        "50%": 22,
        "0 11px 11px 0": 10,
        "20px": 2,
        "0 100% 100% 0": 2,
        "50% 0 0 50%": 2,
        "11px 11px 0 0": 16,
        "0 0 11px 11px": 2,
        "3px": 8,
        "22px": 4,
        ".5em": 4,
        "5px": 4,
        ".2em": 4,
        "100%": 2,
        ".3em": 2,
        "11px 0 0 0": 4,
        "0 11px 0 0": 4,
        "1.125em": 2,
        "1.125em 0 0 1.125em": 2,
        "11px 0 0 11px": 14,
        "6px 0 0 6px": 2,
        "0 6px 6px 0": 2,
        "0 1em 1em 0": 2
      }

bartveneman added a commit that referenced this issue Jun 8, 2024
Thanks to @KenjiCrosland and @Robbert for ideas and feedback!

refs #301
@bartveneman
Copy link
Member

Brought some new life into the border-radius stuff because nowadays we have itemsPerContext if we opt into that, meaning that we can keep track of the property that set a particular radius.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Development

No branches or pull requests

2 participants