search

sourcegraph / stylelint-config

Shared stylelint config for all Sourcegraph projects
Apache License 2.0
4 stars 4 forks source link

Allow px for margin, padding, height, & width #200

Closed unclejustin closed 2 years ago

unclejustin commented 2 years ago

Forcing font-based sizing onto non-font-based elements is a mistake.

This article does a great job of explaining why: https://medium.com/@sascha.wolff/dont-use-rem-em-for-paddings-margins-and-more-94e19026b000

This PR adds px as an available unit for margin, padding, height, & width.

I also believe we should refactor rems to px in the rest of the codebase and update this rule to enforce px only, but we can address that later if everyone agrees with me here.

Slack conversation here https://sourcegraph.slack.com/archives/C0HMGV90V/p1661848404811549

lrhacker commented 2 years ago

@unclejustin Not seeing the addition of px unit for height and width in the diff. Missing a commit?

FWIW, I do agree with the change (especially for margin and padding).

unclejustin commented 2 years ago

@lrhacker, thanks for the callout! My bad. Fixed 🙌