Code Review - week 1

[VatsKan]

Hey team, looks like you have made an awesome start, and managed to do a lot in a couple build days! These are some general points... not all the points below are phrased in a way which actually makes sense, but let's go through this sometime next week and do a live refactoring session.

Compliments

• nice job getting in a simple test!
• nice providing fallback values: {auth().currentUser.displayName || auth().currentUser.email}!
• cool that you have implemented google sign up too. please show me how to do this.
• cool using circleCI --- please teach me :) :)
• nice start installing the cloud functions!

Reminders

• don't forget to do a readme! (especially helps for other people looking at your project)

File structure

• have folders with one file in it. let's talk about shallow file structure and file naming sometime.
• have a formstyle.js for styled-components, but in other components it isn't there. try to be consistent across codebase with how things are arranged/named. often helps to have a process guide for this. can put the stylings at the bottom of each component instead in a new file.

Bugs

• in data-helpers.js getAllMedicationDB, you are returning the array synchronously but have a promise before. this will lead to the array most likely being empty. you want to return the array inside the promise (instead of console.log) and return the whole db.collection promise instead.
• should the sign up form inputs be required?

Refactoring

• nice implementation of onAuthStatusChange. the logic in App.js is a little complex, could probably just use an if statement and return what you need if user logged in or logged out.
  e.g.

if(authStatus){
  return <Route>
      blahblah...some components
  </Route>
} else {
  return <Route>
    blahblah...some components
  </Route>
}

this would mean you would have to put a component level higher up.

• in user-management.js, you have functions which only have one line of code in it, better to just put in that one/two line(s) of code directly where you need it, also as it is unlikely you will be reusing these functions.

• probably don't need handleChange events and too much local state on your sign up/log in forms, as you can access the properties (like password, email etc) from event.currentTarget when submitting the form.

• try to avoid leaving in commented out code in your codebase as much as possible. too many comments in general --- make the code more readable instead.

• avoid console.logs as success status, for more than a temporary check while developing. better to handle errors appropriately and assume successful otherwise.

• try to avoid unused imports as much as possible.

• im not sure about this but i wonder if instaed of doing
  if (!inputs[ampmId]) inputs[ampmId] = "AM"; in SetSpecificTime.js
  you can just do value={inputs[ampmId] || "AM"}.

• not sure what pushData.js is for. perhaps consider deleting the file, or if its just a draft, often helpful to have a 'playground' folder for any experiments youre doing in the app.

• check out this video on how to import firebase better: ADD URL

Questions

(These are things i don't know about but was intrigued by, looking at your project).

• using local state to change between pages (if(page===1) return <SomeComponent/>). Think use react-router-dom instead as this is what its for. At the same time i guess it
  makes sense as you're using it just to switch between different parts of the same form
  so I'm actually not sure what the best practice for this is. But i have something similar
  in my current project which im going to ask oli about next week and update you (or vice versa)!
• i have no idea what the converData function is doing in helper.js. looks interesting though.
• why is most of the code in pushData commented out? why do you have firebase api keys in pushData as well as initialising the app in connection.js? would probably use the import...export syntax as you have done in connection.js, rather than require syntax.
• why does it say /** @Format */ at the top of a lot of files? is this a VScode editor thing which is only appearing on my code editor?
• have never set up eslint before but wonder if eslint should be installed as a dev dependency?
• did you set up .eslintignore yourselves? does it not ignore these automatically?
• what does react-hooks-helper library do out of interest?
• what are react-icons for? are they cool?