Fixed errors on routes [/, /courses, /events, /campusLeaders] #99

aymenhamada · 2020-09-25T20:41:29Z

issue: #92

GangaChatrvedi · 2020-09-26T04:58:58Z

@Abhishek-kumar09 Can yo do detailed analysis of this one? And provide insights.

Abhishek-kumar09

Here are descriptions and some questions asked on changes, But we have to additionally cross-check and compare the original and this deployed version twice before actually merging the PR.

Thanks @aymenhamada for your efforts.

Abhishek-kumar09 · 2020-09-26T06:11:26Z

src/layouts/MainLayout/TopBar/index.js

@@ -139,6 +139,7 @@ function TopBar({ className, onMobileNavOpen, ...rest }) {
          <Box ml={2} flexGrow={1} />
          {navItems.map((item, index) => (
            <Item
+              key={index}


Each child in a list should have a unique "key" prop.

Abhishek-kumar09 · 2020-09-26T07:05:30Z

src/views/pages/CLView/ApplyNowModal.js

-                    return <MenuItem value={`+${code}`}>+{code}</MenuItem>;
+                  {countryCodes.map((code, index) => {
+                    return (
+                      <MenuItem key={index} value={`+${code}`}>


Each child in a list should have a unique "key" prop.

Abhishek-kumar09 · 2020-09-26T07:07:08Z

src/views/pages/CoursesView/Courses.js

@@ -128,8 +128,6 @@ export default function Courses() {
          xs={12}
          align="center"
          display="flex"
-          justifyContent="center"
-          alignItems="center"


AlignItems must be used on containers.

Abhishek-kumar09 · 2020-09-26T07:08:03Z

src/views/pages/CoursesView/Courses.js

@@ -166,8 +164,6 @@ export default function Courses() {
          xs={12}
          align="center"
          display="flex"
-          justifyContent="center"


AlignItems must be used on containers.

Abhishek-kumar09 · 2020-09-26T07:09:46Z

src/views/pages/HomeView/Courses.js

@@ -167,8 +167,6 @@ export default function Courses() {
          xs={12}
          align="center"
          display="flex"
-          justifyContent="center"
-          alignItems="center"


Abhishek-kumar09 · 2020-09-26T07:31:54Z

src/views/pages/CLView/StandFor.js

                </Typography>
+                <Typography variant="h4">- An Initiative taker</Typography>


❌ The text looks actually bigger than the original one.

Try changin to h5 or h6 to better comparable to the original design.

Abhishek-kumar09 · 2020-09-26T07:32:25Z

src/views/pages/CLView/StandFor.js

                </Typography>
+                <Typography variant="h4">- A Resource Seeker</Typography>


❌ Here also, The bold text looks actually bigger than the original one.

Abhishek-kumar09 · 2020-09-26T07:32:34Z

src/views/pages/CLView/StandFor.js

              </Box>
              <Box mt={2}>
                <Typography variant="body1">
                  Most sought after value - an effort maker and giving
-                  peronality <h4>- An Investor</h4>


Abhishek-kumar09 · 2020-09-26T07:37:32Z

src/views/pages/CoursesView/Courses.js

@@ -191,7 +187,7 @@ const CoursesGrid = ({ courses }) => {
        return (
          <Grid
            item
-            key={course.id}
+            key={cname}


⁉️ why cname, not course.id

course.id was undefined, and cname was unique

Abhishek-kumar09 · 2020-09-26T07:42:50Z

src/views/pages/common/Apply.js

@@ -73,7 +73,7 @@ function Apply({ className, ...rest }) {
      }}
      {...rest}
    >
-      <Grid container maxWidth="lg">


React does not recognize the maxWidth prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase maxwidth instead. If you accidentally passed it from a parent component, remove it from the DOM element.

I think it was intentionally put here, so please change maxWidth to maxwidth

Abhishek-kumar09 · 2020-09-26T08:35:30Z

This should be bold, See actual website.

Abhishek-kumar09 · 2020-09-26T08:38:12Z

The bold letters should be small, see sandbox.codeforcause.org

Abhishek-kumar09 · 2020-09-26T08:41:23Z

This should be one shade bold, see sandbox.codeforcause

Abhishek-kumar09 · 2020-09-28T08:51:13Z

please rebase to resolve conflicts.

aymenhamada · 2020-09-28T20:07:07Z

idk how to fix that issue, help needed

Abhishek-kumar09 · 2020-09-29T08:16:21Z

idk how to fix that issue, help needed

Which issue are you talking about, rebasing?

aymenhamada · 2020-09-30T02:30:16Z

y

Abhishek-kumar09 · 2020-09-30T06:59:16Z

Steps you have to do:

git remote add upstream the **git** url of the cfc repo.
git fetch upstream
git rebase upstream/development
You will have conflicts generated in 3 files.
You will have to delete the code written in yours file, and keep the code coming from upstream.

Fixed errors on routes [/, /courses, /events, /campusLeaders]

61d0380

GangaChatrvedi requested review from Abhishek-kumar09 and GangaChatrvedi and removed request for Abhishek-kumar09 September 26, 2020 04:59

Abhishek-kumar09 reviewed Sep 26, 2020

View reviewed changes

Fix some requirements

657c00d

aymenhamada closed this Sep 28, 2020

aymenhamada reopened this Sep 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed errors on routes [/, /courses, /events, /campusLeaders] #99

Fixed errors on routes [/, /courses, /events, /campusLeaders] #99

aymenhamada commented Sep 25, 2020

GangaChatrvedi commented Sep 26, 2020 •

edited

Loading

Abhishek-kumar09 left a comment

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 Sep 26, 2020

aymenhamada Sep 27, 2020 •

edited

Loading

Abhishek-kumar09 Sep 26, 2020

Abhishek-kumar09 commented Sep 26, 2020

Abhishek-kumar09 commented Sep 26, 2020

Abhishek-kumar09 commented Sep 26, 2020

Abhishek-kumar09 commented Sep 28, 2020

aymenhamada commented Sep 28, 2020

Abhishek-kumar09 commented Sep 29, 2020

aymenhamada commented Sep 30, 2020

Abhishek-kumar09 commented Sep 30, 2020

		</Typography>
		<Typography variant="h4">- An Initiative taker</Typography>

		</Typography>
		<Typography variant="h4">- A Resource Seeker</Typography>

Fixed errors on routes [/, /courses, /events, /campusLeaders] #99

Are you sure you want to change the base?

Fixed errors on routes [/, /courses, /events, /campusLeaders] #99

Conversation

aymenhamada commented Sep 25, 2020

GangaChatrvedi commented Sep 26, 2020 • edited Loading

Abhishek-kumar09 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

aymenhamada Sep 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-kumar09 commented Sep 26, 2020

Abhishek-kumar09 commented Sep 26, 2020

Abhishek-kumar09 commented Sep 26, 2020

Abhishek-kumar09 commented Sep 28, 2020

aymenhamada commented Sep 28, 2020

Abhishek-kumar09 commented Sep 29, 2020

aymenhamada commented Sep 30, 2020

Abhishek-kumar09 commented Sep 30, 2020

GangaChatrvedi commented Sep 26, 2020 •

edited

Loading

aymenhamada Sep 27, 2020 •

edited

Loading