Replace style prop with utility css classes #16826
Comments
thmsobrmlr
added
bug
thmsobrmlr
changed the title
|
@thmsobrmlr can i work on this issue? |
|
Can multiple people work on this issue? If so then I would like to do it too. Thank You |
|
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. |
|
Is this Issue is open to contribute? |
|
This is an long running project. Feel free to contribute if you'd like! |
|
ok |
|
Looks like this is no longer reproducible and can be closed since PR #19134 started enforcing the eslint rule. |
|
@thmsobrmlr I believe that the purpose of issues under the "good first issue" label in open source projects is for developers to get familiar with the code base, PR process, etc. In light of the above, can I continue to open more PRs myself for this task or shall I leave it as an opportunity for others to learn from this task while I pick up other tasks? I believe that frontend tasks like these provide an opportunity to quickly learn about the product from an end-user POV as well. |
|
@thmsobrmlr Please have a look at the above PR and let me know your thoughts. Meanwhile, I'll investigate if I can run the snapshot tests (even though I am following the instructions at #23699). |
We're cleaning up how we write css. Previously we would often use the
styleprop 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-propsto 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.
The text was updated successfully, but these errors were encountered: