PostHog / posthog

🦔 PostHog provides open-source product analytics, session recording, feature flagging and A/B testing that you can self-host.
https://posthog.com
Other
19.53k stars 1.14k forks source link

Replace style prop with utility css classes #16826

Open thmsobrmlr opened 11 months ago

thmsobrmlr commented 11 months ago

We're cleaning up how we write css. Previously we would often use the style prop e.g. <div style={{position: 'absolute', top: 8}} '/> to style components. We're now replacing this with utility classes that are very similar to the tailwind classes.

More Context

The locations that need updating can be found by running the following command:

 pnpm eslint --rule "{react/forbid-dom-props: warn}" 

Please note that not all styles can be replaced with utility classes. When dynamic values are passed in, we keep the style prop (or at least keep it for the dynamic values) and add the eslint directive // eslint-disable-next-line react/forbid-dom-props to mark this occurance as ok.

Documentation

Contributors

When you want to work on this PR, don't try to do it all at once. Rather pick out a couple (1-10) files and replace the styles there. It often makes sense to pick a specific product area or page so that changes can easily be reviewed together.

Udit-takkar commented 11 months ago

@thmsobrmlr can i work on this issue?

ebrahim95 commented 11 months ago

Can multiple people work on this issue? If so then I would like to do it too.

Thank You

thmsobrmlr commented 11 months ago

You're both welcome to work on this and we don't typically assign issues. I'd like to avoid a situation where you both work on the same things though. Maybe @Udit-takkar can start from the top of eslint issues and @ebrahim95 from the bottom? Or align with each other and post a short notice of an area you work on.

BalanaguYashwanth commented 7 months ago

Is this Issue is open to contribute?

mariusandra commented 7 months ago

This is an long running project. Feel free to contribute if you'd like!

BalanaguYashwanth commented 7 months ago

ok

Dejvino commented 1 month ago

Looks like this is no longer reproducible and can be closed since PR #19134 started enforcing the eslint rule.

thmsobrmlr commented 1 month ago

@Dejvino

Looks like this is no longer reproducible and can be closed since PR #19134 started enforcing the eslint rule.

There are still quite a bunch of these in the codebase, they just have been ignored from the linter with eslint-disable-next-line react/forbid-dom-props directives.