Code review by matiasgarcia91 · Pull Request #8 · NicoleKuong/let-GO-client

matiasgarcia91 · 2020-03-12T09:21:25Z

No description provided.

Feat/select location

Feat/post item

Feat/item dashboard

Send image url to server

Feat/item details

Send owner email

Feat/searchbar by location

matiasgarcia91

Looking good Nicole! Left a few comments

matiasgarcia91 · 2020-03-12T09:45:46Z

+              Make an offer
+            </Nav.Link>
+          </Nav.Item>
+        </Nav> */}


remove this commented out nav bar

Removed, thanks

matiasgarcia91 · 2020-03-12T09:57:33Z

+
+export const NEW_EMAIL = "NEW_EMAIL";
+
+const databaseUrl = "http://localhost:4000";


Get this out to some other constant file so you don't have it repeated all around. another solution for this (but using axios) is doing this in an axios.js file:

import axios from "axios"; const instance = axios.create({ baseURL: "http://localhost:4000" }); export default instance;

And then you import axios from this file when you make requests and it's already set

created a constants.js file

matiasgarcia91 · 2020-03-12T09:58:52Z

+  // console.log("*****", emailData, ownerEmail);
+  // console.log("email getState", getState().user);
+  const token = getState().user.token;
+  const data = { ...emailData, ownerEmail };


thank you :)

matiasgarcia91 · 2020-03-12T10:01:47Z

+  const state = getState();
+  const { items } = state;
+
+  if (!items.length) {


cant' you check for this before even calling this function? I like that you are preventing over fetching. If not then I like to write this things in the early-return style

export const getItems = () => (dispatch, getState) => { const state = getState(); const { items } = state; if (items.length) return; // then do request if there are no items }

I used the early-return style, thank you!

matiasgarcia91 · 2020-03-12T10:04:50Z

+  telephoneNumber,
+  latitude,
+  longitude,
+  history


when passing so many params to a function it's recommendable to pass them as an object or named arguments, for two reasons:

export const signUp = ({ username, email, password, firstName, ... })

which you call later like signUp({ username, email, password, ... })

The order in which you pass them doesn't matter => less chance of mistakes

It allows you to also call the function like signUp(user) where the user is an object with all this things making your code way shorter and cleaner

great advice, thank you!

matiasgarcia91 · 2020-03-12T10:12:23Z

+import React, { Component } from "react";
+import LoginForm from "./LoginForm";
+import { login } from "../../actions/user";
+import { connect } from "react-redux";


Same as in the backend we have an order for the imports:

Libraries/modules

my files (components, actions)

css files

thank you for the correction!

matiasgarcia91 · 2020-03-12T10:13:16Z

+        this.state.telephoneNumber,
+        this.state.latitude,
+        this.state.longitude,
+        this.props.history


if you switch it to named arguments like I commented above you can do this.props.dispatch(signUp(this.props.state)) 😄

changed it and much cleaner 😄

matiasgarcia91 · 2020-03-12T10:14:04Z

+        alert("An unknown error occurred.");
+        break;
+      default:
+        alert("An unknown error occurred.");


nice, take a look at bootstrap's or material-ui's Snackbar component, it's a nice alert/message popup

thank you for the css advice, I will check them out

matiasgarcia91 · 2020-03-12T10:14:36Z

+
+export default (state = initialState, action = {}) => {
+  switch (action.type) {
+    case NEW_ITEM: {


you are importing NEW_EMAIL but using NEW_ITEM careful

Oh..thank you for pointing it out, I will be careful

matiasgarcia91 · 2020-03-12T10:15:17Z

+        houseNumber: action.payload.houseNumber,
+        city: action.payload.city,
+        latitude: action.payload.latitude,
+        longitude: action.payload.longitude


return {..state, ..action.payload }

This is great, thank you 😄

Feat/payment page

logout function for user

NicoleKuong and others added 30 commits March 4, 2020 14:06

install dependencies and set up login and signup components

610f7ee

updated signup and login componenet and set up reducer and action

5690146

make routes for signin and login page

0a61e55

add reducer for user

57b0aea

set up signup form with address and css

202b6d1

get user current coordinates with geolocation

36e1404

update user action and sign up component for sending coords to db

7e5a128

add handle error for geolocation

a2d9f76

create item component

16d07d4

add item form

746dd15

Merge pull request #1 from NicoleKuong/feat/select-location

6f1f5df

Feat/select location

use cloudinary to upload image

c8e7e62

testing drag and drop image feature

367ca54

second test drag and drop feature

eb8de57

create item list component and display items on homepage

f77860f

create item list component and display items on homepage

53b9c8d

Merge pull request #2 from NicoleKuong/feat/post-item

e19118a

Feat/post item

Merge pull request #3 from NicoleKuong/feat/item-dashboard

11f0ae5

Feat/item dashboard

successfully send cloudinary image url to db

292414f

successful adding itemId of image db

65782d6

Merge pull request #4 from NicoleKuong/send-imageUrl-to-server

20d1cee

Send image url to server

modified user action and reduce to include location info

cec74c4

rearrange and merged drag and drop bo with item form

6fba36e

show map on item details with owner location

55a95a9

Merge pull request #5 from NicoleKuong/feat/item-details

a9b0f10

Feat/item details

delete unnecessary image action and reducer

99e4761

updated item details page css

f6d58f6

add email component and create action and reducer for sending out email

3d7cfd1

successfully send renter message to owner email

a2b7953

Merge pull request #6 from NicoleKuong/send-owner-email

1dddf25

Send owner email

Merge pull request #7 from NicoleKuong/feat/searchbar-by-location

dcf070d

Feat/searchbar by location

matiasgarcia91 commented Mar 12, 2020

View reviewed changes

NicoleKuong and others added 28 commits March 13, 2020 01:05

cleaned code

4075576

attempt to fix the search bar refresh problem

9e01062

add button for search bar to clear search result

874a78b

add react-stripe-checkout dependencies and updated payment component

78ca173

install react-toastify for displaying notifications

44bcf8c

successfully make payment with stripe

e530b37

add payment css and cleaned up code

05770b0

remove unnecessary comments

3d533d3

add constants file

bbf977a

modified environment variable file

7184def

Merge pull request #9 from NicoleKuong/feat/payment-page

bf5a034

Feat/payment page

minor modifications on css

2881fbd

updated README.md

cc487c4

update README.md

2f839b8

updated README.md

4b90755

add logout function for user

c0d502a

Merge pull request #10 from NicoleKuong/feat/logout

23c9666

logout function for user

add app demo gif and updated README.md

0fc1624

updated README.md

c58dd88

deploy application on netlify

97a2f1b

rearrange payment public key into constants.js

a8fa701

modified public key

4516e36

updated README.md with deployed site

df70c3b

updated README.md

de34fb2

minor fix

83e420b

minor fix

cfe2992

minor fix

83ea4ef

minor fix

dc7fe58


		export const NEW_EMAIL = "NEW_EMAIL";

		const databaseUrl = "http://localhost:4000";

Conversation

matiasgarcia91 commented Mar 12, 2020

Uh oh!

matiasgarcia91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicoleKuong Mar 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matiasgarcia91 Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NicoleKuong Mar 23, 2020 •

edited

Loading

matiasgarcia91 Mar 12, 2020 •

edited

Loading