Removed inline styling #744

K-Kumar-01 · 2020-10-23T08:06:28Z

Changed the styling by material ui box component
Some changes made using styled components

Some functionalities like cursor:pointer and backgroundImage are not available as JSX attributes . So had to use styled component there.
Rest all has been done using material ui box
The issue link is here

Changed the styling by material ui box component Some changes made using styled components

amoedoamorim

Hello @K-Kumar-01 ! It's good to see you around here again! Thanks for this new PR, it looks much better than the first one. I'm sorry I failed to explicit that we DO use double quotes for element attributes, so in places like <Box clone justifyContent='flex-start' alignItems='center'> these should actually be double quotes! Please update your PR with the changes requested so we can accept it!

amoedoamorim · 2020-10-27T23:27:53Z

src/app/components/user/AttributionDialog.js

@@ -32,9 +33,9 @@ class AttributionDialog extends React.Component {
        </DialogTitle>
        <DialogContent>
          <Message message={this.props.message} />
-          <div style={{ marginBottom: units(2), marginTop: units(2) }}>
+          <Box my={units(2)}>


Please make it my={2} instead of my={units(2)}

amoedoamorim · 2020-10-27T23:29:18Z

src/app/components/task/Task.js

              <UserAvatars users={byPictures} />
-              <span style={{ lineHeight: '24px', paddingLeft: units(1), paddingRight: units(1) }}>
+              <Box component='span' lineHeight='24px' px={units(1)}>


Please remove the units() call around 1. Make it px={1} instead.

amoedoamorim · 2020-10-27T23:32:50Z

src/app/components/task/EditTaskDialog.js

@@ -209,21 +210,22 @@ class EditTaskDialog extends React.Component {
    const canRemove = this.state.options.length > 2;

    return (
-      <div style={{ marginTop: units(2) }}>
+      <Box mt={units(2)}>


Please make it mt={2}. In general, remove all units() calls when using Box.

amoedoamorim · 2020-10-27T23:33:22Z

src/app/components/task/EditTaskDialog.js

-                disabled={item.other}
-                style={{ padding: `${units(0.5)} ${units(1)}`, width: '75%' }}
-              />
+              <Box clone p={`${units(0.5)} ${units(1)}`} width={"75%"}>


Please remove units call

amoedoamorim · 2020-10-27T23:34:05Z

src/app/components/task/EditTaskDialog.js

@@ -236,7 +238,7 @@ class EditTaskDialog extends React.Component {
            </Row>
          </div>
        ))}
-        <div style={{ marginTop: units(1) }}>
+        <Box mt={units(2)} >


Please remove units call. Make it mt={2} instead.

amoedoamorim · 2020-10-27T23:36:55Z

src/app/components/task/GeolocationRespondTask.js

@@ -331,9 +332,9 @@ class GeolocationRespondTask extends Component {
          onBlur={() => this.setState({ openResultsPopup: false })}
          fullWidth
        />
-        <div style={{ font: caption, color: black54 }}>
+        <Box color={black54} fontFamily={fontStackSans} fontWeight={400} fontSize={units(1.5)} lineHeight={units(2.5)}>


Please revert. This is better handled with the material-ui Typography component.

amoedoamorim · 2020-10-27T23:39:34Z

src/app/components/task/GeolocationTaskResponse.js

 import Dialog from '@material-ui/core/Dialog';
 import DialogContent from '@material-ui/core/DialogContent';
 import { Map, Marker, TileLayer } from 'react-leaflet';
 import styled from 'styled-components';
 import config from 'config'; // eslint-disable-line require-path-exists/exists
 import ParsedText from '../ParsedText';
 import { safelyParseJSON } from '../../helpers';
-import { units, black05, black38, FlexRow } from '../../styles/js/shared';
+import { units, black05, black38, FlexRow, black54 } from '../../styles/js/shared';


What is black54 being imported for?

amoedoamorim · 2020-10-27T23:44:37Z

src/app/components/task/Task.js

              <UserAvatars users={byPictures} />
-              <span style={{ lineHeight: '24px', paddingLeft: units(1), paddingRight: units(1) }}>
+              <Box component='span' lineHeight='24px' px={units(1)}>


Please remove units() call. I think you got the idea already!

Added double quotes in attrbutes instead of single Removed the units function wherever necessary

K-Kumar-01 · 2020-10-28T06:40:27Z

@amoedoamorim
I have made the requested changes in the new PR.
Tell me if there is any further change needed.

amoedoamorim

Hello @K-Kumar-01 ! Thanks for your changes. We're almost there. There are still some changes needed to comply with coding guidelines. I'm pasting the eslint output to make it easier to detect what needs to change. Please fix those so we can accept and merge it! Thanks a lot!

amoedoamorim · 2020-10-29T14:18:55Z

src/app/components/task/GeolocationRespondTask.js

-              ref={(m) => { this.marker = m; }}
-            />
-          </Map>
+          <Box height={'400px'}>


height="400px"

375:23 error Curly braces are unnecessary here react/jsx-curly-brace-presence

amoedoamorim · 2020-10-29T14:20:41Z

src/app/components/task/GeolocationRespondTask.js

@@ -371,23 +372,24 @@ class GeolocationRespondTask extends Component {
          margin="normal"
        />
        <div>


Please remove this div as the newly added Box should take its place.

amoedoamorim · 2020-10-29T14:21:02Z

src/app/components/task/EditTaskDialog.js

@@ -236,7 +238,7 @@ class EditTaskDialog extends React.Component {
            </Row>
          </div>
        ))}
-        <div style={{ marginTop: units(1) }}>
+        <Box mt={2} >


amoedoamorim · 2020-10-29T14:38:59Z

src/app/components/task/DatetimeRespondTask.js

+        <Box clone mb={2}>
+          <FlexRow >
+            <Box clone flex={10} >
+              <Box clone justifyContent='flex-start' alignItems='center'>


214:41 error Unexpected usage of singlequote jsx-quotes
214:65 error Unexpected usage of singlequote jsx-quotes

amoedoamorim · 2020-10-29T14:39:35Z

src/app/components/task/DatetimeRespondTask.js

+                    <Autocomplete
+                      className="task__datetime-timezone"
+                      options={
+                        taskTimezones && taskTimezones.length ? taskTimezones : Object.values(timezones)


246:1 error Line 246 exceeds the maximum line length of 100 max-len

amoedoamorim · 2020-10-29T14:48:44Z

src/app/components/team/TeamBots.js

+          { team.team_bot_installations.edges.map((installation) => {
+            const bot = installation.node.team_bot;
+            const botExpanded = this.state.expanded === bot.dbid;
+


203:1 error Trailing spaces not allowed no-trailing-spaces

amoedoamorim · 2020-10-29T14:49:47Z

src/app/components/team/TeamInfoEdit.js

+import styled from 'styled-components';
+
+const StyledTeamAvatar = styled(TeamAvatar)`
+  background-image: url('${(props) => props.imageUrl}');
+`;


Please revert. Using styled-components is not in the scope of this ticket.

amoedoamorim · 2020-10-29T14:49:55Z

src/app/components/team/TeamInfoEdit.js

-            <TeamAvatar
-              style={{ backgroundImage: `url(${avatarPreview || team.avatar})` }}
+            <StyledTeamAvatar
+              imageUrl={avatarPreview || team.avatar}


Please revert. Using styled-components is not in the scope of this ticket.

amoedoamorim · 2020-10-29T14:50:52Z

src/app/components/team/TeamInviteMembers.js

+                  >
+                    <CancelIcon />
+                  </StyledIconButton>
+                </Row>  


305:23 error Trailing spaces not allowed no-trailing-spaces

amoedoamorim · 2020-10-29T14:51:27Z

src/app/components/team/TeamMembersComponent.js

+                    />
+                  ))}
+                </List>
+              </Card>  


90:22 error Trailing spaces not allowed no-trailing-spaces

K-Kumar-01 · 2020-10-29T14:59:46Z

@amoedoamorim
I will fix these changes ASAP

Reverted the styled components Removed the trailing spaces

K-Kumar-01 · 2020-10-29T15:23:27Z

@amoedoamorim
I have made the requested changes along with reverting of styled components made by me back to original form.
Please let me know if there is any change needed

amoedoamorim

Great, thanks! There are still some trailing spaces. Looks like you created those when fixing the other issues.

amoedoamorim · 2020-10-29T15:53:00Z

src/app/components/task/GeolocationTaskResponse.js

@@ -53,9 +54,11 @@ class GeolocationTaskResponse extends Component {
      <FlexRow className="task__geolocation-response">
        <span className="task__response"><ParsedText text={name} /></span>
        {coordinatesString ?
-          <span
+          <Box 


57:15 error Trailing spaces not allowed no-trailing-spaces

^ This is still unfixed

amoedoamorim · 2020-10-29T15:54:00Z

src/app/components/task/SingleChoiceTask.js

+            <Box 
+              display="flex" 
+              justifyContent="flex-start" 


168:17 error Trailing spaces not allowed no-trailing-spaces
169:29 error Trailing spaces not allowed no-trailing-spaces
170:42 error Trailing spaces not allowed no-trailing-spaces

^ These are still unfixed

amoedoamorim · 2020-10-30T12:37:07Z

Please update your branch with latest develop and watch out for conflicts: src/app/components/user/AttributionDialog.js

K-Kumar-01 · 2020-10-30T14:35:55Z

@amoedoamorim
I have updated my develop branch with the latest code from original. Let me if there are any changes required.

amoedoamorim

Thanks @K-Kumar-01 ! There are still some trailing spaces left! Please fix those and we should be in a good position to merge! Thank you so much for your effort in contributing! We appreciate it!

amoedoamorim · 2020-10-30T19:56:38Z

src/app/components/task/GeolocationTaskResponse.js

@@ -53,9 +54,11 @@ class GeolocationTaskResponse extends Component {
      <FlexRow className="task__geolocation-response">
        <span className="task__response"><ParsedText text={name} /></span>
        {coordinatesString ?
-          <span
+          <Box 


^ This is still unfixed

amoedoamorim · 2020-10-30T19:57:10Z

src/app/components/task/SingleChoiceTask.js

+            <Box 
+              display="flex" 
+              justifyContent="flex-start" 


^ These are still unfixed

K-Kumar-01 · 2020-10-31T04:55:07Z

@amoedoamorim I have removed the trailing spaces which were left
Let me if there is any change required.

amoedoamorim

Hey @K-Kumar-01 , thanks again! I notices a couple details left that are blocking the build. Please fix those so we can move on! Thank you!

amoedoamorim · 2020-11-02T21:55:04Z

src/app/components/search/SearchResultsTable/FillRemainingHeight.js

@@ -1,5 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import Box from '@material-ui/core/Box'


3:40 error Missing semicolon semi

amoedoamorim · 2020-11-02T21:57:51Z

src/app/components/task/Task.js

-              <a href={fileUploadPath} target="_blank" rel="noreferrer noopener" style={{ color: checkBlue }}>{response}</a>
-            </p>
+            <Box component="p" textAlign="center">
+              <Box 


418:19 error Trailing spaces not allowed no-trailing-spaces

K-Kumar-01 · 2020-11-03T05:55:12Z

@amoedoamorim Made the requested changes that were causing build error

amoedoamorim · 2020-11-08T14:45:34Z

Thanks @K-Kumar-01 and congrat's on your approved PR! That looks amazing now, I'll go ahead and merge it! Thank you so much for the effort you put in your contribution! We appreciate it!

K-Kumar-01 added 2 commits October 23, 2020 13:34

Removed inline styling

cacddaa

Changed the styling by material ui box component Some changes made using styled components

Added clone property in some box components

3a01031

This was referenced Oct 23, 2020

Removed inline styling #740

Closed

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

Open

amoedoamorim requested changes Oct 27, 2020

View reviewed changes

Made the requested Changes

81a6da3

Added double quotes in attrbutes instead of single Removed the units function wherever necessary

amoedoamorim requested changes Oct 29, 2020

View reviewed changes

Made the requested changes

f85359a

Reverted the styled components Removed the trailing spaces

amoedoamorim requested changes Oct 29, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into develop

f8a136f

amoedoamorim requested changes Oct 30, 2020

View reviewed changes

K-Kumar-01 added 2 commits October 31, 2020 10:22

Removed the trailing spaces

b670635

Merge remote-tracking branch 'upstream/develop' into develop

9ad0eba

Merge remote-tracking branch 'upstream/develop' into develop

100ccb8

amoedoamorim requested changes Nov 2, 2020

View reviewed changes

K-Kumar-01 added 2 commits November 3, 2020 11:15

Updated the missing semicolon and trailing space

3be867d

Merge remote-tracking branch 'upstream/develop' into develop

36129f5

amoedoamorim merged commit d952e3e into meedan:develop Nov 8, 2020

Removed inline styling #744

Removed inline styling #744

Conversation

K-Kumar-01 commented Oct 23, 2020 • edited Loading

amoedoamorim left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

K-Kumar-01 commented Oct 28, 2020

amoedoamorim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

K-Kumar-01 commented Oct 29, 2020

K-Kumar-01 commented Oct 29, 2020

amoedoamorim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amoedoamorim commented Oct 30, 2020

K-Kumar-01 commented Oct 30, 2020

amoedoamorim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

K-Kumar-01 commented Oct 31, 2020

amoedoamorim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

K-Kumar-01 commented Nov 3, 2020

amoedoamorim commented Nov 8, 2020

K-Kumar-01 commented Oct 23, 2020 •

edited

Loading

amoedoamorim left a comment •

edited

Loading