Skip to content
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

Removed inline styling #740

Closed
wants to merge 1 commit into from
Closed

Removed inline styling #740

wants to merge 1 commit into from

Conversation

K-Kumar-01
Copy link
Contributor

Used styled components and material ui box instead.

Partial fix for the issue.
Done the fix for 26 files using the styled components and Material UI BOX component.
Work done can be considered as continuation in this PR

Used styled components and material ui box instead
Copy link
Contributor

@amoedoamorim amoedoamorim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @K-Kumar-01 thanks for your interest in contributing! We appreciate it!

While it looks like you're genuinely trying to contribute, I unfortunately cannot accept this PR.
In this issue specifically we are attempting to use material-ui Box for some simple layout/spacing needs.

It is important to respect our coding guidelines and standards, this means:

  • We use single quotes for strings. Unless extraordinarily needed otherwise, use single quotes.
  • It's not necessary to break down components into multiple lines if line < 100 char.
  • Avoid styled-components. We're trying to stick with @material-ui for our styling needs.

I'm sorry to waste your effort, but I guess it's simpler for all of us to just discard this PR.
Please attempt a smaller and more focused contribution that don't drastically modify the codebase with undesired changes. Again, we appreciate your interest and effort in contributing! Any doubts, please reach out in the issue discussion!

@K-Kumar-01
Copy link
Contributor Author

@amoedoamorim
I will make the changes and will follow the guidelines in the new PR.
Thanks for the review and clarification.

@K-Kumar-01
Copy link
Contributor Author

@amoedoamorim
I have made a new PR.
Please let me know if there are any changes needed here.
Also some components require styled components usage as described in the Pull request. Rest all have been described in the PR description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants