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

Remove inline styling in favor of material-ui Box component #18

Open
amoedoamorim opened this issue Oct 1, 2020 · 24 comments
Open

Remove inline styling in favor of material-ui Box component #18

amoedoamorim opened this issue Oct 1, 2020 · 24 comments
Labels
Component: Check Mark Browser extension client at https://github.com/meedan/check-mark Component: Check Web Web client at https://github.com/meedan/check-web Difficulty: Easy good first issue Tech: React

Comments

@amoedoamorim
Copy link
Contributor

amoedoamorim commented Oct 1, 2020

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?

  • Check Web
  • Check Mark

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/

@amoedoamorim amoedoamorim added Component: Check Web Web client at https://github.com/meedan/check-web Component: Check Mark Browser extension client at https://github.com/meedan/check-mark Difficulty: Easy Tech: React HacktoberFest good first issue labels Oct 1, 2020
@sparkingdark
Copy link

Want to work on it.

@ghost
Copy link

ghost commented Oct 2, 2020

Is this issue taken? Or can i work on it too ?

@Hard-Coder05
Copy link

@amoedoamorim I would like to work on this issue! Please assign this to me! Any of the two Check Web or Check Mark will work for me!

@amoedoamorim amoedoamorim assigned ghost Oct 2, 2020
@amoedoamorim
Copy link
Contributor Author

@karenefereyan I have assigned it to you! Thanks for your interest. Please go ahead and let us know if you have any trouble.

@amoedoamorim
Copy link
Contributor Author

@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.

@abhisheknaiidu
Copy link

Hey @karenefereyan are you still working on this?

@ghost
Copy link

ghost commented Oct 4, 2020

@abhisheknaiidu please go on

@aloks98
Copy link

aloks98 commented Oct 5, 2020

Can I take up this issue?

@Hard-Coder05
Copy link

@amoedoamorim Can I work on this issue? I guess @karenefereyan is not working on it!

@ghost
Copy link

ghost commented Oct 8, 2020

@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.

@Hard-Coder05
Copy link

@karenefereyan Okay cool!
Then @amoedoamorim Please assign me this issue!

@K-Kumar-01
Copy link

Is the issue still open. I would like to work on it @karenefereyan and @amoedoamorim

@amoedoamorim amoedoamorim unassigned ghost Oct 16, 2020
@amoedoamorim
Copy link
Contributor Author

Hello everyone! Thank you all for your interest in contributing! Please feel free to submit your PRs regardless of being assigned to the ticket!

@K-Kumar-01
Copy link

Hello @amoedoamorim
I installed Docker compoese(docker not installed).
Then when i try to run sudo git clone --recursive [email protected]:meedan/check.git && cd check it shows :
The authenticity of host 'github.com (13.234.210.38)' can't be established.
On clicking yes, it says Permission denied (publickey).
Could you tell me how to fix this

@amoedoamorim
Copy link
Contributor Author

Hey @K-Kumar-01, please try this and let me know if this works for you:

ome/devspace#38 (comment)

@K-Kumar-01
Copy link

Hello @amoedoamorim
So i went where i want to clone the repo
Entered the following command ssh-keyscan github.com >> ~/.ssh/known_hosts
Then tried sudo git clone --recursive [email protected]:meedan/check.git && cd check.
Still it shows

Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@K-Kumar-01
Copy link

K-Kumar-01 commented Oct 19, 2020

@amoedoamorim
Can't we simply clone the this [repo] (https://github.com/meedan/check-web/tree/ca90b5da133801388acb33eaa6d1464047551a95) and perform do the operations here.
Is there any way without installing the full check repo and only doing the submodule installation .

Also i tried creating using styled components in codesandbox the replica of inline styling which was done
Here

Can we do in this way or should i use another technique.

The replica was of of line 125

@jmakhack
Copy link

@amoedoamorim,

Just created a PR for that removes inline styling for 50 files in the check-web repo.
Another ~50 files still have inline styling within the check-web project for anyone else to pick up (as well as changes needed in the check-mark repo).

CC: @K-Kumar-01

@K-Kumar-01
Copy link

@jmakhack
I tried using the code

<div className="App" style={{width:"80%", margin:'0 auto'}}>
      <div style={{ display: "flex" }}>
        <div style={{ width: "50%" }}>
          <CardContainer>
            <Box clone p={0} pt={2} textAlign="center">
              <CardHeader
                // style={{ padding: 0, paddingTop: "16px" }}
                title={"Hello world"}
                subheader={true ? "Hello" : null}
              />
            </Box>
            <p>{"lorem iosunaf sm ajsbfajs fkjasn"}</p>
          </CardContainer>
        </div>
        <div style={{ width: "50%" }}>
          <CardContainer>
            <CardHeader
              style={{ padding: 0, paddingTop: "16px", textAlign:"center" }}
              title={"Hello world"}
              subheader={true ? "Hello" : null}
            />
            <p>{"lorem iosunaf sm ajsbfajs fkjasn"}</p>
          </CardContainer>
        </div>
      </div>
    </div>

but for this the padding was not removed as done when we did inline styling. Could you please tell me about this

@K-Kumar-01
Copy link

@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.

@K-Kumar-01
Copy link

@amoedoamorim
Any updates on this

@K-Kumar-01
Copy link

@amoedoamorim
I created a new PR with some changes in code. Please let me know if there are any changes required.
Link is here

@K-Kumar-01
Copy link

K-Kumar-01 commented Oct 26, 2020

@amoedoamorim
Any updates on the PR I made?

@herbdullah
Copy link

If this is not resolved yet, i will like to work on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Check Mark Browser extension client at https://github.com/meedan/check-mark Component: Check Web Web client at https://github.com/meedan/check-web Difficulty: Easy good first issue Tech: React
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants