Skip to content

add batch total volume to cocktail sheet #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bpolack merged 1 commit into from
Jul 29, 2025
Merged

add batch total volume to cocktail sheet #21

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 44 additions & 3 deletions src/components/cocktails/cocktail-sheet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import {
Plus,
Settings2,
Share,
X
X,
} from 'lucide-react'
import { Fragment, useState } from 'react'
import { Fragment, useMemo, useState } from 'react'
import {
convertToPreferredUnits,
simplifyValue,
Expand Down Expand Up @@ -79,6 +79,34 @@ export function CocktailSheet({
const { data } = cocktailQuery
const { data: userCocktailData } = userCocktailQuery

const batchTotalVolume = useMemo(() => {
if (!data) return { value: 0, units: 'ml' }

let units = 'ml'

const totalValue = data.ingredients.reduce((total, ingredient) => {
if (ingredient.units === 'oz' || ingredient.units === 'ml') {
const volume = convertToPreferredUnits(
ingredient.amount
? new Prisma.Decimal(ingredient.amount).toNumber() * batchValue
Comment on lines +87 to +91
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

Creating a new Prisma.Decimal instance on each iteration could be inefficient. Consider converting the amount to number once and reusing it, or handle the Decimal multiplication directly without converting to number first.

Suggested change
const totalValue = data.ingredients.reduce((total, ingredient) => {
if (ingredient.units === 'oz' || ingredient.units === 'ml') {
const volume = convertToPreferredUnits(
ingredient.amount
? new Prisma.Decimal(ingredient.amount).toNumber() * batchValue
const batchDecimal = new Prisma.Decimal(batchValue);
const totalValue = data.ingredients.reduce((total, ingredient) => {
if (ingredient.units === 'oz' || ingredient.units === 'ml') {
const volume = convertToPreferredUnits(
ingredient.amount
? new Prisma.Decimal(ingredient.amount).mul(batchDecimal).toNumber()

Copilot uses AI. Check for mistakes.
: 0,
ingredient.units,
user?.preferredUnits,
)
units = volume.units || 'ml'
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The units variable is being overwritten on each iteration. This means the final units will be determined by the last ingredient processed, which could be inconsistent. Consider determining units based on the user's preferred units or the majority of ingredients instead.

Suggested change
units = volume.units || 'ml'
if (volume.units) unitsArray.push(volume.units)

Copilot uses AI. Check for mistakes.
return total + volume.value
} else {
// Ignore non-volume ingredients
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The comment should be more specific about what constitutes 'non-volume ingredients'. Consider listing examples like 'dashes', 'pinches', 'garnishes', etc. for better code documentation.

Suggested change
// Ignore non-volume ingredients
// Ignore non-volume ingredients (e.g., dashes, pinches, garnishes)

Copilot uses AI. Check for mistakes.
return total
}
}, 0)

return {
value: totalValue,
units,
}
}, [data, batchValue, user])

const handlePreferredUnitsChange = async (preferredUnits: PreferredUnits) => {
if (updateUser.isPending) {
return
Expand Down Expand Up @@ -235,7 +263,7 @@ export function CocktailSheet({
<TooltipTrigger asChild>
<Button
className="[&_svg]:size-5"
variant='outline'
variant="outline"
size="icon"
onClick={handleShareClick}
>
Expand Down Expand Up @@ -354,6 +382,19 @@ export function CocktailSheet({
)
})}
</div>
{/* Show a badge for the batch total volume */}
{batchValue !== 1 && (
<Badge variant="secondary" className="text-xs mt-4 ml-4">
Total batch volume:{' '}
{simplifyValue(
batchTotalVolume.value,
batchTotalVolume.units,
)}{' '}
<span className="text-muted-foreground">
{batchTotalVolume.units}
</span>
</Badge>
)}
</div>
<div className="cocktail__instructions mb-6">
<div className="cocktail__label text-sm text-muted-foreground py-2 pl-4">
Expand Down
Loading