Skip to content

DiningHallMenuDetailView #512

Open
Chhumbucket wants to merge 5 commits intomasterfrom
chhum-485
Open

DiningHallMenuDetailView #512
Chhumbucket wants to merge 5 commits intomasterfrom
chhum-485

Conversation

@Chhumbucket
Copy link
Collaborator

Added the information from the webscrapper to now be presented on the app. The menu are not clickable that appends the view into the stack.

Screen.Recording.2026-03-06.at.2.33.15.AM.mov

On top of adding recipes and allergens. I added a macronutrients bar that tells the user the ratios between protein, fat, and carbs that they are eating.

Screenshot 2026-03-06 at 2 44 56 AM

Let me know if you want other features added on this. Also notice a bug where we have two brown's Cafe. For the Browns the recipe works but for Brown's Cafe the recipe does not work. The scrapper does not seem to add to brown cafe. Might need to remove Brown's cafe and change it.

Copy link
Collaborator

@justinwongeecs justinwongeecs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love the design! just a few comments

Comment on lines -144 to -150
} header: {
HStack {
Text(mealCategory.categoryName)
.font(Font(BMFont.bold(20)))
.font(.headline)
Spacer()
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it in the section header because maybe in the future if we decide to add collapsible headers then we can really easily do it.

Also if we decide to change up the list or form style, putting the view in the header gives us automatic styling changes.

Comment on lines -54 to -62
List {
ScrollView {
DiningItemsListView(selectedTabIndex: $selectedTabIndex,
categoriesAndMenuItems: categoriesAndMenuItems,
diningHall: diningHall,
filteredTabNames: filteredTabNames)
}
.listStyle(.plain)
.scrollContentBackground(.hidden)
.contentMargins(.top, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep the List because it's more performant. A ScrollView keeps all of its subviews in memory regardless if it's visible. A List has cell reusability like what UITableView and UICollectionView do.


// MARK: - DiningMenuItemDetailView

struct DiningMenuItemDetailView: View {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor DiningMenuItemDetailView into another file.

Comment on lines +218 to +227
if let servingSize = menuDetail.servingSize {
VStack(alignment: .leading, spacing: 8) {
Text("Serving Size")
.font(Font(BMFont.bold(20)))
DiningDetailRowView {
Text(servingSize)
.font(Font(BMFont.regular(15)))
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's refactor these subviews into private var under DiningDetailView for better readability

Comment on lines +212 to +216
let calories = Double(nutrition["Calories (kcal)"] ?? "0") ?? 0
let remainingNutrition = nutrition.filter { $0.key != "Calories (kcal)" }
let protein = Int(nutrition["Protein (g)"]?.filter { $0.isNumber } ?? "0") ?? 0
let carb = Int(nutrition["Carbohydrate (g)"]?.filter { $0.isNumber } ?? "0") ?? 0
let fat = (Int(nutrition["Total Lipid/Fat (g)"]?.filter { $0.isNumber } ?? "0") ?? 0) + (Int(nutrition["Trans Fat (g)"]?.filter { $0.isNumber } ?? "0") ?? 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have these as computed properties under BMMenuItemDetail?

Comment on lines +284 to +292
VStack(alignment: .leading, spacing: 8) {
Text("Ingredients")
.font(Font(BMFont.bold(20)))
Text(ingredients)
.font(Font(BMFont.regular(15)))
}
.padding()
.frame(maxWidth: .infinity, alignment: .leading)
.shadowfy()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks a bit weird how the header "Ingredients" is within the padded rectangle but "Serving Size", "MacroNutrients", and "Nutrition Facts" are "outside the content" if you get what I mean.

For consistency, we should bring the header "Ingredients" out.

}
.padding()
.frame(maxWidth: .infinity)
.shadowfy()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we don't .shadowfy()? The rest of the DiningDetailRowView don't have shadows so it's kinda of weird how "MacroNutrients" and "Ingredients" are the ones that only have shadows.

("Fat", .orange)
]
VStack(alignment: .leading, spacing: 8) {
Text("MacroNutrients")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: MacroNutrientsMacronutrients

}
}

struct ProgressCapsule: View {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rename it to something more specific? ProgressCapsule sounds like it's a generic view that can be used for many different scenarios. But it looks like it's built specifically to show macronutrients. So something like MacronutrientsBreakdownCapsuleView?

Circle()
.fill(macro.color)
.frame(width: 12, height: 12)
Text(macro.label)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel "Protein", "Carb", and "Fact" font size should be smaller than the "Calories" one since they're just like the legend for a map. Right now, they are larger than the calories text which is the primary text we want users to focus on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants