Overall system mixin strategy

[dzucconi]

Opening this to get consensus on how to proceed.

It feels to me like the overall design of what system props are available on which components is unintuitive — but it's also possible I don't have a good mental model of the overall strategy with regard to primitives.

Here's some of the things that bit me in the past hour:

• Box has bg but Flex doesn't
• Flex has top/bottom but not left/right
• Typography supports as aliased as element but nothing else does

A good first step I think would be to move the flex mixin into Box, and then have Flex just become Box with display: flex as default.

Additionally I'd be interested in what is required to support as universally on our primitive elements.

[damassi]

A good first step I think would be to move the flex mixin into Box, and then have Flex just become Box with display: flex as default.

I fully endorse this.

Some history around Palette is that when @zephraph and I first got it off the ground, it was patched into a flury other first-ever tasks, such as our new ssr relay pages and architecture. It was also a gamble that Katarina helped us advocate for with considerable risk. Since then, things have been non-stop for the both of us.

I think now with you on board we're in a good place to revisit what we've done and make it sturdy, as well as cleanup. For example:

Flex has top/bottom but not left/right

The history of this is someone was using flex, needed top, and opened a quick PR to add it. Same with bottom. So far no one has needed left or right, until you stumbled upon it. All of this is ripe for cleanup.

[just-be-dev]

While I agree with the direction I firmly believe we shouldn’t do it on the current version of styled-systems.

We desperately need to upgrade. The latest version of styled-system has a drastically reduced set of helpers that makes concerted effort towards providing consistent props a relatively trivial matter.

I know it’s gonna be hell to upgrade, but we really should. Take a look at my WIP pr. I’ll try to get back around to that next week.

[damassi]

I've also tried to upgrade but found our space scale stuff broke across the board in scary way. I can dig up my PR if its not listed in @zephraph's.

[dzucconi]

Interesting, something to do with us using unitless values in the theme? Could convert them to px and then have the space function do the parseInt? (I'm taking a look at this now as well)

[dzucconi]

https://styled-system.com/guides/migrating/

Number values are no longer converted to strings with px units. Most CSS-in-JS libraries now handle this, but if you need to use string values with units, provide them in your theme object and in prop values.

[damassi]

unitless values in the theme

I believe so, since this was a major breaking change.

The other issue i ran into was ratio being removed from the API: https://github.com/artsy/palette/blob/master/packages/palette/src/elements/Image/Image.shared.tsx#L62. I ended up using styled-systems custom prop util to recreate it.

Also, we use a Tag component to allow-list props; that was deprecated in styled-system and it broke things in the upgrade.

Other than those things, I was able to rough things in, minus some test / type issues (which i think are less relevant now that we have skipLibCheck: true): #521. Check out the screenshot.

[just-be-dev]

We should definitely try that. Afraid it’ll break places where we explicitly add pixels in our code (definitely a thing), but those would be easier to find and fix.

Let’s just make sure it doesn’t break Eigen

[pepopowitz]

Context for this:

Typography supports as aliased as element but nothing else does

When @zephraph and I paired on adding that prop to typography, there was weirdness with using a prop named as. Something swallowed that prop, and it would never get passed through to styled-system. So we aliased it to element which did not get swallowed.

It would definitely be great to remove that band-aid, but just a heads up that if someone makes an attempt, watch out for something like as="h1" not getting rendered as an h1.