Grading and feedback PR [DO NOT MERGE]

README.md

@@ -1 +1,3 @@
-# ramilyfecipes
+# ramilyfecipes
+
+Include instructions for developers to get setup and explain project structure and organization

src/recipe-deets/RecipeDeets.js

@@ -1,5 +1,11 @@
 import Component from '../Component.js';
 
+function toListItems(list) {
+    return list.map(ingredient => {
+        return `<li>${ingredient}</li>`;
+    }).join(' ');
+}
+
 class RecipeDeets extends Component {
 
     renderTemplate() {
@@ -8,14 +14,13 @@ class RecipeDeets extends Component {
             return `<div></div>`;
         }
         const image = recipe.imageURL || './assets/food_default.png';
-
-        const ingredients = recipe.ingredients.map(ingredient => {
-            return `<li>${ingredient}</li>`;
-        }).join(' ');
 
-        const instructions = recipe.instructions.map(instruction => {
-            return `<li>${instruction}</li>`;
-        }).join(' ');
+        // DRY === Don't Repeat Yourself
+        // Isolate common code, in this case into a function:
+        const ingredients = toListItems(recipe.ingredients);
+        const instructions = toListItems(recipe.instructions);
+
+        // Try and place list header type info _before_ the <ul>
 
         return /*html*/`
             <div id="recipe-deets-object">
@@ -25,8 +30,14 @@ class RecipeDeets extends Component {
                 <p>Cook Time: ${recipe.cookTime}</p>
                 <p>Ready In: ${recipe.readyIn}</p>
                 <p>Servings: ${recipe.servings}</p>
-                <ul>Ingredients: ${ingredients}</ul>
-                <ol>Instructions: ${instructions}</ol>
+                <p>
+                    Ingredients: 
+                    <ul>${ingredients}</ul>
+                </p>
+                <p>
+                    Instructions: 
+                    <ol>${instructions}</ol>
+                </p>
                 <p>Notes: ${recipe.notes}</p>
                 <p>Meal Type: ${recipe.mealType}</p>
                 <p>Diet Type: ${recipe.dietType}</p>

src/recipe-list/RecipeFilter.js

@@ -11,6 +11,11 @@ class RecipeFilter extends Component {
         form.addEventListener('input', () => {
             const dietFilterValue = form.elements['diet-type-filter'].value;
             const mealFilterValue = form.elements['meal-type-filter'].value;
+            // Better to use key/value pairs as [0] and [1] are cryptic:
+            // const filters = {
+            //     diet: dietFilterValue,
+            //     meal: mealFilterValue
+            // };
             let filters = ['', ''];
             filters[0] = dietFilterValue; 
             filters[1] = mealFilterValue;

src/recipe-list/RecipeListApp.js

@@ -16,6 +16,7 @@ class RecipeListApp extends Component {
         dom.insertBefore(header.render(), main);
 
         let allRecipes = [];
+        // Nice! way to cache the terms between calls
         let searchTerm;
         let filterTerm;
 

src/recipe-list/searchAndFilter.js

@@ -1,42 +1,31 @@
+// Really liked that you put this in a module.
+// Though would have liked to see a tested, test-first approach
 function searchAndFilter(search, filter, allRecipes) {
-
-    if(filter && !search) {
-        const filterByDiet = allRecipes.filter(recipe => {
-            return recipe.dietType.includes(filter[0]); 
-        });
 
-        const filtered = filterByDiet.filter(recipe => {
-            return recipe.mealType.includes(filter[1]);
-        });
+    // Here's an approach to avoid the duplication.
 
-        return filtered;
-    } 
-
-    if(search && !filter) {
-        const searchArray = allRecipes.filter(recipe => {
-            return recipe.recipeTitle.toLowerCase().includes(search) ||
-            recipe.cookbookTag.toLowerCase().includes(search);
-        });
+    let filtered = allRecipes;
 
-        return searchArray;
+    if(search) {
+        filtered = filtered.filter(recipe => {
+            return recipe.recipeTitle.toLowerCase().includes(search) 
+                || recipe.cookbookTag.toLowerCase().includes(search);
+        });
     }
 
-    if(filter && search) {
-        const searchArray = allRecipes.filter(recipe => {
-            return recipe.recipeTitle.toLowerCase().includes(search) ||
-            recipe.cookbookTag.toLowerCase().includes(search);
-        });
-
-        const filterByDiet = searchArray.filter(recipe => {
-            return recipe.dietType.includes(filter[0]);
+    if(filter && filter[0]) {
+        filtered = filtered.filter(recipe => {
+            return recipe.dietType.includes(filter[0]); 
         });
+    }
 
-        const filtered = filterByDiet.filter(recipe => {
+    if(filter && filter[1]) {
+        filtered = filtered.filter(recipe => {
             return recipe.mealType.includes(filter[1]);
         });
-
-        return filtered;
     }
+
+    return filtered;
 }
 
 export default searchAndFilter;

src/submit-recipe/SubmitRecipe.js

@@ -1,22 +1,49 @@
 import Component from '../Component.js';
 import { auth, recipesByUserRef, imageStorageRef } from '../services/firebase.js';
 
+// not a bad idea to clean up lists:
+function toList(items) {
+    return items
+        // split the string to an array on linebreak:
+        .split('\r\n')
+        // remove whitespace from beginning and end
+        .map(text => text.trim())
+        // remove empty list items
+        .filter(text => text !== '');
+}
+
+// easier to put these here and use template strings
+const INGREDIENTS_PLACEHOLDER = `
+2 cups of flour
+1/2 cup of sugar
+2 eggs
+etc...`;
+
+const INSTRUCTIONS_PLACEHOLDER = `
+Add flour and sugar to a large mixing bowl
+Thoroughly mix together
+Add eggs and whisk until desired consistency
+etc...`;
+
+
 class SubmitRecipe extends Component {
 
     render() {
         const form = this.renderDOM();
         const submitForm = form.querySelector('form');
 
         const submitButton = form.querySelector('#submit-button');
+        // This is the user's id under "recipes-by-user", not the recipe.
+        // Instead of passing in, you could just use auth.currentUser.uid
         const recipeKey = recipesByUserRef.child(this.props.key);
 
         submitButton.addEventListener('click', event => {
             event.preventDefault();
             const formData = new FormData(submitForm);
             const file = formData.get('image');
             const recipeRef = recipeKey.push();
-            const ingredients = formData.get('ingredients').split('\r\n');
-            const instructions = formData.get('instructions').split('\r\n');
+            const ingredients = toList(formData.get('ingredients'));
+            const instructions = toList(formData.get('instructions'));
 
             if(file.size === 0) {
                 saveRecipe();
@@ -53,7 +80,6 @@ class SubmitRecipe extends Component {
 
                 recipeRef.set(newRecipe).then(() => {
                     submitForm.reset();
-
                 });
             }
         });
@@ -62,14 +88,19 @@ class SubmitRecipe extends Component {
     }
 
     renderTemplate() {
-        const ingredientPlaceholder = '2 cups of flour\n1/2 cup of sugar\n2 eggs\nect...';
-        const instructionsPlaceholder = 'Add flour and sugar to a large mixing bowl\nThouroughly mix together\nAdd eggs and whisk until disired consistancy\nEct...';
+        // These can live as constants at top of module
+        // const ingredientPlaceholder = '2 cups of flour\n1/2 cup of sugar\n2 eggs\nect...';
+        // const instructionsPlaceholder = 'Add flour and sugar to a large mixing bowl\nThouroughly mix together\nAdd eggs and whisk until disired consistancy\nEct...';
 
         return /*html*/ `
             <div id="container">
                 <form id="form">
                     <div>
-                        <label>Recipe Title: <input id="title-input" required name="recipe-title" placeholder="Recipe Title..."></label>
+                        <!--Try and be more horizontal instead of long lines-->
+                        <label>
+                            Recipe Title: 
+                            <input id="title-input" required name="recipe-title" placeholder="Recipe Title...">
+                        </label>
                     </div>
 
                     <div>
@@ -105,11 +136,11 @@ class SubmitRecipe extends Component {
                     </div>
                     <div>
                         <p class="howto" for="ingredients">Ingredients: Type an ingredient then press enter (make sure each ingredient is on a new line! Without blank lines.).</p>
-                        <textarea id="ingredients" required name="ingredients" placeholder="${ingredientPlaceholder}"></textarea>
+                        <textarea id="ingredients" required name="ingredients" placeholder="${INGREDIENTS_PLACEHOLDER}"></textarea>
                     </div>
                     <div>
                         <p class="howto" for="instructions">Instructions: Write a step then press enter (make sure each step starts on a new line! Without blank lines).</p>
-                        <textarea id="instructions" required name="instructions" placeholder="${instructionsPlaceholder}"></textarea>
+                        <textarea id="instructions" required name="instructions" placeholder="${INSTRUCTIONS_PLACEHOLDER}"></textarea>
                     </div>
 
                     <div>

src/submit-recipe/SubmitRecipeApp.js

@@ -14,7 +14,8 @@ class SubmitRecipeApp extends Component {
         dom.insertBefore(header.render(), main);
 
         const keyRef = recipesByUserRef.child(auth.currentUser.uid);
-
+
+        // Isn't keyRef.key same thing as auth.currentUser.uid?
         const submitRecipe = new SubmitRecipe({ key: keyRef.key });
         main.appendChild(submitRecipe.render());