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

Replace inline stylings with Material-UI Box attributes #748

Closed
wants to merge 10 commits into from

Conversation

amaaniqbal
Copy link
Contributor

This PR replaces inline stylings present in tag/ and team/ directories inside check-web/src/app/components/ with the corresponding Material-UI Box component attributes.

Fixes #18

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.

Thanks @amaaniqbal for your contribution! We appreciate it! Please notice that it won't work if you don't import material-ui Box on those files! Please fix as needed so we are able to accept it! Thanks again for contributing!

@amaaniqbal
Copy link
Contributor Author

Oops, missed it! Fixing it right away...

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.

Hello @amaaniqbal ! Thank you so much for your interest in contributing! Please notice that some coding standards need to be met and we require a few changes to accept this PR. I kindly ask you to review your PR and apply these changes. Mainly the string props should be enclosed in double quotes, and unused imports should be removed.

src/app/components/team/TeamInvitedMemberItem.js Outdated Show resolved Hide resolved
src/app/components/team/TeamBots.js Outdated Show resolved Hide resolved
src/app/components/team/TeamReport.js Outdated Show resolved Hide resolved
src/app/components/team/TeamTags.js Show resolved Hide resolved
src/app/components/team/TeamTags.js Outdated Show resolved Hide resolved
src/app/components/team/TeamReport.js Show resolved Hide resolved
@amaaniqbal
Copy link
Contributor Author

@amoedoamorim I did the suggested changes. Please have a look!

@amoedoamorim
Copy link
Contributor

Thanks @amaaniqbal for your contribution! We appreciate it! There are a couple of conflicts in your PR in relation to current develop branch. Would you solve these so we can move forward with merging your code, please?

@amaaniqbal
Copy link
Contributor Author

Fixed the conflicts @amoedoamorim!

@amoedoamorim
Copy link
Contributor

Hello @amaaniqbal ! Thanks for your contribution! We appreciate it! Your PR looks good in general, but there are some conflicts resulting from merges of PRs from others. Please solve those so we can go ahead and merge yours!

@amaaniqbal
Copy link
Contributor Author

Fixed the merge conflicts again @amoedoamorim!

@ghost
Copy link

ghost commented Dec 4, 2020

Hello @amaaniqbal, I was away for the past weeks and was unable to merge your PR. Would you please fix this conflict so I can merge your PR? Thank you!

@amaaniqbal
Copy link
Contributor Author

Hello @alexmeedan, this PR was up since Hacketoberfest and since then I am just resolving merge conflicts. I resolved them once again, but please review this before merging any other PRs this time if possible. Also, please consider this as a part of Hacktoberfest.

PS: src/app/components/team/TeamProjects.js was deleted by someone which resulted in merge conflicts, so I added it back. Let me know if it should be deleted, and I will remove the same.

@ghost
Copy link

ghost commented Dec 4, 2020

Yes, I'll merge as soon as it's fixed! I couldn't do it before because your previous commit was sent in while I was away. It will still count for Hacktoberfest and eligible for Meedan swag. I apologize for the inconvenience. The file TeamProjects.js is not in use anymore and should be deleted.

@amaaniqbal
Copy link
Contributor Author

Thanks a lot for the positive response @alexmeedan! I removed the unused file TeamProjects.js. You can review the PR now.

@amoedoamorim
Copy link
Contributor

Hello @amaaniqbal . I did some fixing, merging and cleaning up myself and merged your contribution on another branch. Thanks again for participating in Hacktoberfest with Meedan! We appreciate your contribution!

@amaaniqbal
Copy link
Contributor Author

Thanks a lot for merging my changes @amoedoamorim! I can see them in the PR over here. May I know what made my previous commits fail Travis build and your commit passed?
PS: I am just curious to know this.

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