From 637b8c06fd1f31ff5b895529a9b634d3c7c7f36e Mon Sep 17 00:00:00 2001 From: Talia Date: Mon, 22 Mar 2021 23:21:37 -0700 Subject: [PATCH 1/4] fixed typo --- src/screens/EmergencyContacts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/screens/EmergencyContacts.js b/src/screens/EmergencyContacts.js index 7bc67b5..70244fe 100644 --- a/src/screens/EmergencyContacts.js +++ b/src/screens/EmergencyContacts.js @@ -39,7 +39,7 @@ const EmergencyContacts = ({ navigation }) => { return ( - Who you would like to call during an Emergency? + Who would you like to call during an Emergency? {/* TODO: change wording */} Name From 9039657c42fadc59b3c468b12646d3207c46f9a9 Mon Sep 17 00:00:00 2001 From: Ian Lizarda Date: Wed, 24 Mar 2021 03:33:18 -0700 Subject: [PATCH 2/4] Code Review - Added comments for review --- src/screens/EmergencyContacts.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/screens/EmergencyContacts.js b/src/screens/EmergencyContacts.js index 70244fe..9e3f044 100644 --- a/src/screens/EmergencyContacts.js +++ b/src/screens/EmergencyContacts.js @@ -21,9 +21,14 @@ import { // TODO: remove extraneous comments const EmergencyContacts = ({ navigation }) => { + // [Ian]: I would recommend updating to React Navigation 5.x.x so you can use Hooks and + // reduce the ability to run into any deprecating errors. let object = navigation.getParam("object", "missing"); const [name, setName] = useState(""); const [contactPhone, setContactPhone] = useState(""); + // [Ian]: I'm not well versed in this package, but if you find yourself setting fonts + // and waiting for loading in your component, I would recommend maybe rendering + // it at the top level with your App.js (if possible). let [fontsLoaded] = useFonts({ Nunito_400Regular, Nunito_600SemiBold, @@ -44,6 +49,8 @@ const EmergencyContacts = ({ navigation }) => { Name { } }; +// [Ian]: This function should be moved above the `return` to keep with React convention. +// [Ian]: I would also change the name of the function as it is a bit confusing. function objectifyAndNav(navigation, object, name, contactPhone) { // add new items to our object + // [Ian]: I would also try to use something more verbose and explicit instead of `object`. object.set("eName", name); object.set("eNumber", contactPhone); @@ -111,6 +121,8 @@ function objectifyAndNav(navigation, object, name, contactPhone) { } export default EmergencyContacts; +// [Ian]: I would either move this to a separate styles file (my pref) or +// move it above the React function. const styles = StyleSheet.create({ container: { flex: 1, From 04cbf4763d638f0af6601ba95a23a37fe09644a7 Mon Sep 17 00:00:00 2001 From: Ian Lizarda Date: Wed, 24 Mar 2021 12:17:57 -0700 Subject: [PATCH 3/4] Code Review - Added comments --- src/screens/EmergencyContacts.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/screens/EmergencyContacts.js b/src/screens/EmergencyContacts.js index 9e3f044..0e7b1d9 100644 --- a/src/screens/EmergencyContacts.js +++ b/src/screens/EmergencyContacts.js @@ -1,23 +1,23 @@ -import React, { useState } from "react"; -import { StyleSheet, Text, View, TouchableOpacity } from "react-native"; -import { TextInput } from "react-native-gesture-handler"; -import { registerNewUser } from "../../firebase/firebase.util"; -import { colors } from "../styles/colors.js"; -import { AppLoading } from "expo"; import { - useFonts, - Nunito_600SemiBold, Nunito_400Regular, + Nunito_600SemiBold, Nunito_800ExtraBold, + useFonts, } from "@expo-google-fonts/nunito"; -import { CoveredByYourGrace_400Regular } from "@expo-google-fonts/covered-by-your-grace"; import { - // Quicksand_300Light, Quicksand_400Regular, Quicksand_500Medium, Quicksand_600SemiBold, Quicksand_700Bold, } from "@expo-google-fonts/quicksand"; +import React, { useState } from "react"; +import { StyleSheet, Text, TouchableOpacity, View } from "react-native"; + +import { AppLoading } from "expo"; +import { CoveredByYourGrace_400Regular } from "@expo-google-fonts/covered-by-your-grace"; +import { TextInput } from "react-native-gesture-handler"; +import { colors } from "../styles/colors.js"; +import { registerNewUser } from "../../firebase/firebase.util"; // TODO: remove extraneous comments const EmergencyContacts = ({ navigation }) => { From 52240c9f351e530a949acf0b93c6c771aa93b4af Mon Sep 17 00:00:00 2001 From: Ian Lizarda Date: Wed, 24 Mar 2021 12:32:59 -0700 Subject: [PATCH 4/4] Updated review --- src/screens/EmergencyContacts.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/screens/EmergencyContacts.js b/src/screens/EmergencyContacts.js index 0e7b1d9..57f2f22 100644 --- a/src/screens/EmergencyContacts.js +++ b/src/screens/EmergencyContacts.js @@ -121,8 +121,6 @@ function objectifyAndNav(navigation, object, name, contactPhone) { } export default EmergencyContacts; -// [Ian]: I would either move this to a separate styles file (my pref) or -// move it above the React function. const styles = StyleSheet.create({ container: { flex: 1,