meedan / check

Development environment for Meedan Check, a collaborative media annotation platform
https://meedan.com/check
MIT License
127 stars 53 forks source link

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

Open amoedoamorim opened 4 years ago

amoedoamorim commented 4 years ago

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/

sparkingdark commented 4 years ago

Want to work on it.

ghost commented 4 years ago

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

Hard-Coder05 commented 4 years ago

@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 commented 4 years ago

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

amoedoamorim commented 4 years ago

@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 commented 4 years ago

Hey @KarenEfereyan are you still working on this?

ghost commented 4 years ago

@abhisheknaiidu please go on

aloks98 commented 4 years ago

Can I take up this issue?

Hard-Coder05 commented 4 years ago

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

ghost commented 4 years ago

@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 commented 4 years ago

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

K-Kumar-01 commented 4 years ago

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

amoedoamorim commented 4 years ago

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 commented 4 years ago

Hello @amoedoamorim I installed Docker compoese(docker not installed). Then when i try to run sudo git clone --recursive git@github.com: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 commented 4 years ago

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

https://github.com/ome/devspace/issues/38#issuecomment-211515244

K-Kumar-01 commented 4 years ago

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 git@github.com: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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@amoedoamorim Any updates on this

K-Kumar-01 commented 4 years ago

@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 commented 4 years ago

@amoedoamorim Any updates on the PR I made?

herbdullah commented 4 years ago

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