nickslick03 / messiah-meal-planner

1 stars 0 forks source link

Top half of frontend part 1 #14

Closed cdrice26 closed 1 month ago

cdrice26 commented 1 month ago

Top half of the frontend is implemented with the exception of the sorting modal. Normally I'd wait to PR until complete, but I had to implement some state management just to avoid having errors particularly on the meal plan enter info. In doing so I had to modify the Input component fairly significantly so we can store the state of each input in its original type rather than converting everything to strings. TypeScript isn't happy about this component, I had to add some pretty messy type annotations just to get it to stop yelling at me. I feel like annotating the setValue function calls in particular shouldn't be necessary but apparently it is. I don't have a ton of experience with TS so it could just be me though, feel free to clean it up if it's at all bothersome (personally I'm fine leaving it for now). As part of the necessary state management, I created a new 'context.ts' file under static to hold the contexts for storing global state. Note: If you want to merge Sprint 2 into master, I think you will need to accept that PR before this one or else this will also merge into master which we probably don't want to do yet.

nickslick03 commented 1 month ago

I can see why using just strings as input gets complicated, and I saw you have a check for the number data type in the input handleChange function so it definitely works for our purposes. If we wanted to make it a bit cleaner, we could try using generics and/or pass a converter function to the input component so we don't have to store everything as strings. Or, since there's only like 3 different type of input data types we need (date, boolean, number), we could risk going to programmer hell and violate the DRY principle by having multiple input components for each data type. Also, the useEffect function in MealPlanInfo.tsx wasn't reading the actual values of the context state so I fixed that and some other minor things. I think it's ready to merge, let me know if there's anything else we should look at :)

cdrice26 commented 1 month ago

Ok cool, if the Input component works for you, then I'm good to leave it. Good catch on the useEffect problem. I found that there were several issues with my CustomMealAddModal component, namely there were two add buttons since there's already one provided by ModalContainer and that if you left the location as default, it would not register. I did significant refactoring to fix this, so I won't merge yet, but if all this looks good to you, then I'm fine going through with the merge.

nickslick03 commented 1 month ago

added a disabled feature and validator to the customMealModal but everything else looks good! I'm good to merge if you are.

cdrice26 commented 1 month ago

Just made a few trivial changes, looks great, going to merge.