GetJobber / atlantis

🔱 Atlantis
https://atlantis.getjobber.com
MIT License
27 stars 30 forks source link

refactor(design): Decouple border width tokens from spacing #2113

Closed chris-at-jobber closed 1 week ago

chris-at-jobber commented 1 week ago

Motivations

We don't want to use spacing for things that aren't spacing (ie dimensions like height and width). This should extend to border thickness as well.

(Noted by @Beccaneer in a related PR for Divider)

Changes

Changed

Testing

On the Cloudflare build (or locally, once you've run npm run bootstrap && npm start), you can go to a component that references the border tokens (ie Card) and inspect the CSS custom prop, then follow the custom prop to its' definition and see that it no longer resolves to the space value. image image

Changes can be tested via Pre-release


In Atlantis we use Github's built in pull request reviews.

Random photo of Atlantis

ericchernuka commented 1 week ago

Any chance of leveraging Rems rather than pixels?

chris-at-jobber commented 1 week ago

@ericchernuka we certainly could, but I'm not sure that we want to couple border thicknesses to font size. It could be inferred that if a user is increasing their font-size that they might want borders to increase in thickness, but I'm not sure it's a given? (pixels still scale with browser zoom if you want everything to zoom)

ericchernuka commented 1 week ago

Ah, fair. Just realized even tailwind doesn't even have rem-based borders (just pixels). Disregard my ridiculous question!

chris-at-jobber commented 1 week ago

No such thing 😄