-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove inline styling in favor of material-ui Box component #18
Comments
Want to work on it. |
Is this issue taken? Or can i work on it too ? |
@amoedoamorim I would like to work on this issue! Please assign this to me! Any of the two |
@karenefereyan I have assigned it to you! Thanks for your interest. Please go ahead and let us know if you have any trouble. |
@Hard-Coder05 Thank you for your interest! I assigned this issue to @karenefereyan but it doesn't mean you cannot submit meaningful contributions too. You can browse the code for similar patterns and fixes. |
Hey @karenefereyan are you still working on this? |
@abhisheknaiidu please go on |
Can I take up this issue? |
@amoedoamorim Can I work on this issue? I guess @karenefereyan is not working on it! |
@abhisheknaiidu was supposed to work on it as I gave him the go ahead to. As you can see in my comment above. So ask him and if he's not I guess you can work on it. |
@karenefereyan Okay cool! |
Is the issue still open. I would like to work on it @karenefereyan and @amoedoamorim |
Hello everyone! Thank you all for your interest in contributing! Please feel free to submit your PRs regardless of being assigned to the ticket! |
Hello @amoedoamorim |
Hey @K-Kumar-01, please try this and let me know if this works for you: |
Hello @amoedoamorim
|
@amoedoamorim Also i tried creating using styled components in codesandbox the replica of inline styling which was done Can we do in this way or should i use another technique. The replica was of of line 125 |
Just created a PR for that removes inline styling for 50 files in the CC: @K-Kumar-01 |
@jmakhack
but for this the padding was not removed as done when we did inline styling. Could you please tell me about this |
@amoedoamorim I created a PR where i changed the inline styling to styled components or box components for 26 files. Please let me know if there is any change needed. |
@amoedoamorim |
@amoedoamorim |
@amoedoamorim |
If this is not resolved yet, i will like to work on it |
Tell us about your request
Remove inline styling in favor of material-ui Box component
Which service(s) is this request for?
Let us know which services(s) you want this for?
Tell us about the problem you're trying to solve. What are you trying to do, and why is it hard?
In order to achieve a better code organization, we're shifting away from inline styling.
Also, by using material-ui's css-in-js solution we benefit from the rtl layout handling.
When styling for spacing such as margins and paddings, it makes sense to use the Box component.
Sample code locations (not limited to):
https://github.com/meedan/check-web/blob/develop/src/app/components/source/UserSecurity.js#L479
https://github.com/meedan/check-web/blob/develop/src/app/components/tag/TagInput.js#L98
https://github.com/meedan/check-web/blob/develop/src/app/components/team/TeamTasks.js
Additional context
To know more about React Box component: https://material-ui.com/components/box/
The text was updated successfully, but these errors were encountered: