Skip to content

Show empty input when serving size is cleared#1431

Merged
CodeWithCJ merged 2 commits into
CodeWithCJ:mainfrom
GasimGasimzada:fix-empty-serving-size-input
Jun 4, 2026
Merged

Show empty input when serving size is cleared#1431
CodeWithCJ merged 2 commits into
CodeWithCJ:mainfrom
GasimGasimzada:fix-empty-serving-size-input

Conversation

@GasimGasimzada

@GasimGasimzada GasimGasimzada commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Description

  • Use NumericInput for number inputs in variant card
  • Always use undefined or number values for variant nutrition fields
  • Add Nix dev shell for easy development on NixOS. Removed nix flake files but kept some of the bash scripts to work with bash regardless of bash path (makes my life easier in NixOS without issues in macOS or other linux distros)

What problem does this PR solve?

When input field is cleared in serving size, the output value becomes "0" and one cannot remove it. This PR fixes that and allows the input to become empty.

How did you implement the solution?

  • Use NumericInput for number inputs in variant card and nutrition grid (optional)
  • Instead of converting the value to Number, which converts undefined/null to 0, just set the value to undefined
  • Normalize TS types for food nutrition fields to be either number of undefined (remove null and string types)\
  • (Optional) Since I am using NixOS, I made the docker dev deployment work with nix shells. I can revert the change if that's not something to be desired for this repo.

Linked Issue: No attached issue.

How to Test

  1. Check out this branch and start the app
  2. Navigate to Food > Create Food > Custom food
  3. Type value in serving size, then clear it
  4. It should be empty field instead of dislaying "0"

PR Type

  • Issue (bug fix)
  • Refactor

Checklist

All PRs:

  • [MANDATORY - ALL] Integrity & License: I certify this is my own work, free of malicious code, and I agree to the License terms.

Frontend changes (SparkyFitnessFrontend/):

  • [MANDATORY for Frontend changes] Quality: I have run pnpm run validate and it passes.

UI changes (components, screens, pages):

  • [MANDATORY for UI changes] Screenshots: I have attached Before/After screenshots below.

Screenshots

Click to expand

Before

Kooha-2026-06-02-20-02-37.webm

After

Kooha-2026-06-02-20-03-14.webm

Notes for Reviewers

Optional — use this for anything that doesn't fit above: known tradeoffs, areas you'd like specific feedback on, qustions you have or context that helps reviewers.

@github-actions github-actions Bot added bug Something isn't working frontend refactor labels Jun 2, 2026
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

PR Validation Results

Change Detection

  • 🖥️ Frontend changes detected

❌ Required Actions (1)

  • [MANDATORY for UI changes] You checked the Screenshots box but no images were found in the PR body.

⚠️ Recommendations (1)

  • Please link a related GitHub issue (Linked Issue: Closes #123).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a custom NumericInput component to improve numeric field handling, updates nutrient form types to use undefined instead of empty strings or nulls, adds a Nix development environment, and refactors Docker configurations. While these changes improve consistency, several issues need to be addressed: a falsy check on calories in NutrientFormGrid.tsx will incorrectly clear the input when the value is 0; changing the nutrient type to undefined in useFoodForm.tsx causes clearing fields to evaluate to NaN in updateVariant; and mounting the host directory directly to /var/lib/postgresql in docker-compose.dev.yml can lead to permission and configuration conflicts.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread SparkyFitnessFrontend/src/components/FoodSearch/NutrientFormGrid.tsx Outdated
Comment thread SparkyFitnessFrontend/src/hooks/Foods/useFoodForm.tsx
Comment thread docker/docker-compose.dev.yml Outdated
POSTGRES_PASSWORD: ${SPARKY_FITNESS_DB_PASSWORD:?Variable is required and must be set}
volumes:
- ./docker_volume/postgresql:/var/lib/postgresql/data:z
- ./docker_volume/postgresql18:/var/lib/postgresql:z

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Mounting the host directory directly to /var/lib/postgresql instead of /var/lib/postgresql/data is highly discouraged for the official PostgreSQL Docker image. This can cause permission issues on the root of the volume and may overwrite or conflict with internal configuration files and the home directory of the postgres user inside the container. It is recommended to use a subdirectory like /var/lib/postgresql/data.

      - ./docker_volume/postgresql18:/var/lib/postgresql/data:z

@GasimGasimzada GasimGasimzada Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mounting postgresql/data directly throws error for me in Docker for postgres.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sparkyfitness-db | Error: in 18+, these Docker images are configured to store database data in a
sparkyfitness-db | format which is compatible with "pg_ctlcluster" (specifically, using
sparkyfitness-db | major-version-specific directory names). This better reflects how
sparkyfitness-db | PostgreSQL itself works, and how upgrades are to be performed.
sparkyfitness-db |
sparkyfitness-db | See also docker-library/postgres#1259
sparkyfitness-db |
sparkyfitness-db | Counter to that, there appears to be PostgreSQL data in:
sparkyfitness-db | /var/lib/postgresql/data (unused mount/volume)
sparkyfitness-db |
sparkyfitness-db | This is usually the result of upgrading the Docker image without
sparkyfitness-db | upgrading the underlying database using "pg_upgrade" (which requires both
sparkyfitness-db | versions).
sparkyfitness-db |
sparkyfitness-db | The suggested container configuration for 18+ is to place a single mount
sparkyfitness-db | at /var/lib/postgresql which will then place PostgreSQL data in a
sparkyfitness-db | subdirectory, allowing usage of "pg_upgrade --link" without mount point
sparkyfitness-db | boundary issues.
sparkyfitness-db |
sparkyfitness-db | See docker-library/postgres#37 for a (long)
sparkyfitness-db | discussion around this process, and suggestions for how to do so.

Comment thread docker/docker-helper.sh
@@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/env bash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note: Nix stores bin files in its immutable stores; so, /bin/bash does not exist there. Using env bash should work across platforms.

@GasimGasimzada GasimGasimzada force-pushed the fix-empty-serving-size-input branch 3 times, most recently from 4bdee6a to d286d3a Compare June 2, 2026 18:12
id={gridId(variantIndex, key)}
label={`${cn.name} (${cn.unit})`}
value={variant.custom_nutrients?.[cn.name] ?? ''}
value={typeof value === 'number' ? value : undefined}

@GasimGasimzada GasimGasimzada Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note: Custom nutrients also only work with numbers but its type still allows for string value. I did not change it here as it required even bigger refactor. If you like this direction for type safety, I can do this in a separate PR.

@GasimGasimzada GasimGasimzada force-pushed the fix-empty-serving-size-input branch 7 times, most recently from a320c27 to caa844d Compare June 2, 2026 18:32
const hasMedia =
/!\[.*?\]\(.+?\)/i.test(body) ||
/<(?:img|video)\s/i.test(body) ||
/\[[^\]]+\]\([^)]*\.(?:apng|avif|gif|jpe?g|mov|mp4|png|svg|webm)(?:[?#][^)]*)?\)/i.test(body)

@GasimGasimzada GasimGasimzada Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note: Videos in Github markdown are just markdown links displayed as embedded videos. So, added a regex that support video as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@CodeWithCJ it seems like pr-validation workflow is not being triggered from the PR. Would it make sense to create a separate PR for it; so, screenshots support videos; then rebase this PR for validation to pass?

@GasimGasimzada GasimGasimzada force-pushed the fix-empty-serving-size-input branch 2 times, most recently from 2a4fd3d to 01a38e0 Compare June 2, 2026 19:45
- Use NumericInput for number inputs in variant card
- Always use `undefined` or number values for variant nutrition fields
- Add Nix dev shell for easy development on NixOS
@GasimGasimzada GasimGasimzada force-pushed the fix-empty-serving-size-input branch from 01a38e0 to bec13b7 Compare June 2, 2026 19:46
@CodeWithCJ

Copy link
Copy Markdown
Owner

@apedley @Sim-sat Could you review this PR.

@GasimGasimzada I dont use Flake. if I dont update, it could get outdated. so not sure if I include it!!!!

Also could you take a look at the co-pilot comment
image

let newVariant: FormFoodVariant & { equivalents?: EquivalentUnit[] };
if (isCustomNutrient) {
newVariant = {
...currentVariant,
custom_nutrients: {
...currentVariant.custom_nutrients,
[field]: value === '' ? '' : Number(value),
},
};
} else if (isNutrientField) {
newVariant = {
...currentVariant,
[field as keyof FormFoodVariant]: value === '' ? '' : Number(value),
};
} else {

image

@GasimGasimzada

Copy link
Copy Markdown
Contributor Author

@CodeWithCJ The fix Copilot suggested is already there. I may have force pushed while fixing linter errors, which confused it.

@CodeWithCJ CodeWithCJ merged commit 232c454 into CodeWithCJ:main Jun 4, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants