feat: Add custom dropdown options for Species, From State, and To State#12
feat: Add custom dropdown options for Species, From State, and To State#12
Conversation
paccloud
commented
Jan 17, 2026
paccloud
commented
- Implement CreatableCombobox component that allows users to type custom values
- Users can add their own Fish species, From States, and To States even if not in database
- Custom entries are persisted to localStorage for future use
- Visual indicators show when custom values are being used
- Prompt users to enter yield percentage manually for custom conversions
- Save functionality remains restricted to logged-in users only
- Full keyboard accessibility with arrow navigation and Enter to select/add
- Implement CreatableCombobox component that allows users to type custom values - Users can add their own Fish species, From States, and To States even if not in database - Custom entries are persisted to localStorage for future use - Visual indicators show when custom values are being used - Prompt users to enter yield percentage manually for custom conversions - Save functionality remains restricted to logged-in users only - Full keyboard accessibility with arrow navigation and Enter to select/add
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a CreatableCombobox for species/from/to selections, persistent custom-entry storage in localStorage, integration of custom entries into data computations, ARIA-accessible tooltip components and improved form labeling in Calculator, and updates About page copy/contact link to a mailto address. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Calculator UI
participant Combo as CreatableCombobox
participant Store as localStorage
participant Calc as Calculator Logic
User->>Combo: Type or select value
activate Combo
Combo->>Combo: Filter options, show "create" option
User->>Combo: Confirm custom entry
Combo->>Store: Save custom entry (customFishSpecies/customFromStates/customToStates)
deactivate Combo
Store-->>Calc: Persisted custom entries available
Calc->>Calc: Merge custom entries into speciesList/fromStates/toStates
Calc-->>UI: Render selections with custom indicators and updated to-state yields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/components/Calculator.jsx`:
- Around line 369-381: The three useState initializers
(customSpecies/setCustomSpecies, customFromStates/setCustomFromStates,
customToStates/setCustomToStates) call JSON.parse directly and will throw on
malformed JSON; wrap the parsing in a try/catch inside each initializer so a
parse error returns an empty array (and optionally remove the bad key from
localStorage or log the error) to avoid crashing the component during
initialization.
🧹 Nitpick comments (6)
app/src/components/Calculator.jsx (6)
17-18: Consider memoizing default function props to avoid potential stale closures.The default function parameters
getOptionLabelandgetOptionValueare recreated on each render. Since they're used inuseMemoanduseEffectdependency arrays (lines 34, 40, 56, 63), callers passing inline arrow functions will cause unnecessary recalculations. This is unlikely to cause visible issues but worth noting for performance-sensitive use cases.
142-158: Addaria-activedescendantfor improved screen reader navigation.The combobox has good ARIA attributes but is missing
aria-activedescendantto indicate which option is currently highlighted during keyboard navigation. This helps screen reader users track the active option.♿ Proposed accessibility fix
<input ref={inputRef} id={id} type="text" value={inputValue} onChange={handleInputChange} onFocus={() => setIsOpen(true)} onKeyDown={handleKeyDown} disabled={disabled} placeholder={placeholder} role="combobox" aria-expanded={isOpen} aria-controls={listboxId} aria-haspopup="listbox" aria-autocomplete="list" + aria-activedescendant={isOpen && highlightedIndex >= 0 ? `${listboxId}-option-${highlightedIndex}` : undefined} className="..." />And update the
<li>elements to include matching IDs:<li key={optionValue} + id={`${listboxId}-option-${index}`} role="option" ...
115-119: Simplify redundant Enter key handling.Lines 117-118 are unreachable since if
showAddCustomis true andhighlightedIndex === filteredOptions.length, it would already be handled by lines 115-116. IfhighlightedIndexis -1 or less thanfilteredOptions.length, the condition on line 117 acts as a fallback to add custom when Enter is pressed without explicit selection, which may be intentional UX. If so, consider adding a comment for clarity.
970-989: Consider adding user feedback for export failures.Export errors are logged to console (line 987) but the user receives no feedback when the export fails. Consider setting a status message similar to the save functionality.
536-575: Consider adding ability to remove custom entries.Custom species, states, and products can only be added but never removed. Users who accidentally add a typo or unwanted entry have no way to clear it except by manually editing localStorage. Consider adding a delete/clear mechanism in a future iteration.
Would you like me to open an issue to track adding the ability to manage (edit/delete) custom entries?
383-392: Wrap localStorage writes in try/catch for robustness.In private browsing mode or when storage quota is exceeded,
localStorage.setItemcan throw. While unlikely to be an issue in practice, defensive error handling would prevent silent failures.🛡️ Proposed fix
useEffect(() => { - localStorage.setItem('customFishSpecies', JSON.stringify(customSpecies)); + try { + localStorage.setItem('customFishSpecies', JSON.stringify(customSpecies)); + } catch (e) { + console.warn('Failed to persist custom species:', e); + } }, [customSpecies]);Apply similar pattern to the other two persistence effects.
Wrap JSON.parse calls in try/catch blocks for customSpecies, customFromStates, and customToStates state initialization. If parsing fails due to malformed JSON: - Log error to console for debugging - Remove the corrupted key from localStorage - Return empty array to allow component to initialize safely
- Remove Local Catch network affiliation content - Update heading from 'About Local Catch' to 'About This Calculator' - Add contact section with ryan@pacificcloudseafoods.com - Simplify origin story text - Replace ExternalLink icon with Mail icon for contact
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/components/Calculator.jsx`:
- Around line 589-600: When the "To Product" selection changes in
handleToChange, reset all yield-related state to avoid stale inputs: clear
fields such as yield value and yield range by calling the yield-related setters
(e.g., setYield, setYieldRange, setYieldMin, setYieldMax or whatever yield-state
setters exist in this component) right after setToState and before/when
setResult(null) so the UI and validation reflect the change; keep the existing
setCustomToStates behavior and rely on the currentConversion effect to
repopulate known conversion values.
- Around line 369-413: The component currently calls localStorage.getItem and
setItem directly inside the initializers and useEffect callbacks (see
customSpecies/customFromStates/customToStates state initializers and their
useEffect savers), which can throw in SSR or blocked storage; wrap each access
in a try/catch and guard for window/localStorage availability before calling
getItem/setItem, returning a safe default ([]) on error, and log the error; also
ensure the useEffect setters similarly guard setItem calls so persisting doesn’t
crash the component if storage is unavailable or throws.
🧹 Nitpick comments (1)
app/src/components/Calculator.jsx (1)
142-227: Addaria-activedescendant+ option IDs for SR combobox navigation.Line 142-227: The input declares
role="combobox", but there’s noaria-activedescendant, so screen readers won’t announce the highlighted option. Add IDs to options and wirearia-activedescendant. Please verify against the WAI‑ARIA combobox pattern.♿ Suggested a11y wiring
<input @@ aria-controls={listboxId} aria-haspopup="listbox" aria-autocomplete="list" + aria-activedescendant={ + isOpen && highlightedIndex >= 0 + ? `${listboxId}-opt-${highlightedIndex}` + : undefined + } className="w-full bg-slate-100 dark:bg-slate-800 border border-slate-300 dark:border-slate-600 rounded-lg p-3 pr-10 focus:ring-2 focus:ring-cyan-500 outline-none text-slate-800 dark:text-white disabled:opacity-50" /> @@ {filteredOptions.map((option, index) => { + const optionId = `${listboxId}-opt-${index}`; const optionValue = getOptionValue(option); const isSelected = value === optionValue; const isHighlighted = highlightedIndex === index; return ( <li + id={optionId} key={optionValue} role="option" aria-selected={isSelected} @@ {showAddCustom && ( <li + id={`${listboxId}-opt-${filteredOptions.length}`} role="option" aria-selected={false}
| // Custom entries state (persisted in localStorage) | ||
| const [customSpecies, setCustomSpecies] = useState(() => { | ||
| const saved = localStorage.getItem('customFishSpecies'); | ||
| if (!saved) return []; | ||
| try { | ||
| return JSON.parse(saved); | ||
| } catch (e) { | ||
| console.error('Failed to parse customFishSpecies from localStorage:', e); | ||
| localStorage.removeItem('customFishSpecies'); | ||
| return []; | ||
| } | ||
| }); | ||
| const [customFromStates, setCustomFromStates] = useState(() => { | ||
| const saved = localStorage.getItem('customFromStates'); | ||
| if (!saved) return []; | ||
| try { | ||
| return JSON.parse(saved); | ||
| } catch (e) { | ||
| console.error('Failed to parse customFromStates from localStorage:', e); | ||
| localStorage.removeItem('customFromStates'); | ||
| return []; | ||
| } | ||
| }); | ||
| const [customToStates, setCustomToStates] = useState(() => { | ||
| const saved = localStorage.getItem('customToStates'); | ||
| if (!saved) return []; | ||
| try { | ||
| return JSON.parse(saved); | ||
| } catch (e) { | ||
| console.error('Failed to parse customToStates from localStorage:', e); | ||
| localStorage.removeItem('customToStates'); | ||
| return []; | ||
| } | ||
| }); | ||
|
|
||
| // Persist custom entries to localStorage | ||
| useEffect(() => { | ||
| localStorage.setItem('customFishSpecies', JSON.stringify(customSpecies)); | ||
| }, [customSpecies]); | ||
| useEffect(() => { | ||
| localStorage.setItem('customFromStates', JSON.stringify(customFromStates)); | ||
| }, [customFromStates]); | ||
| useEffect(() => { | ||
| localStorage.setItem('customToStates', JSON.stringify(customToStates)); | ||
| }, [customToStates]); |
There was a problem hiding this comment.
Guard localStorage access against unavailable/blocked storage.
Line 369-413: localStorage.getItem/setItem can throw (SSR, privacy modes, quota). Right now only JSON.parse is guarded, so the component can still crash before render. Wrap storage access in a try/catch and check for window availability.
🛡️ Proposed hardening (apply same pattern to the other two keys)
const [customSpecies, setCustomSpecies] = useState(() => {
- const saved = localStorage.getItem('customFishSpecies');
- if (!saved) return [];
- try {
- return JSON.parse(saved);
- } catch (e) {
- console.error('Failed to parse customFishSpecies from localStorage:', e);
- localStorage.removeItem('customFishSpecies');
- return [];
- }
+ try {
+ const saved = typeof window !== 'undefined'
+ ? window.localStorage.getItem('customFishSpecies')
+ : null;
+ if (!saved) return [];
+ return JSON.parse(saved);
+ } catch (e) {
+ console.error('Failed to read customFishSpecies from localStorage:', e);
+ if (typeof window !== 'undefined') {
+ window.localStorage.removeItem('customFishSpecies');
+ }
+ return [];
+ }
});
@@
- useEffect(() => {
- localStorage.setItem('customFishSpecies', JSON.stringify(customSpecies));
- }, [customSpecies]);
+ useEffect(() => {
+ if (typeof window === 'undefined') return;
+ try {
+ window.localStorage.setItem('customFishSpecies', JSON.stringify(customSpecies));
+ } catch (e) {
+ console.warn('Failed to persist customFishSpecies:', e);
+ }
+ }, [customSpecies]);🤖 Prompt for AI Agents
In `@app/src/components/Calculator.jsx` around lines 369 - 413, The component
currently calls localStorage.getItem and setItem directly inside the
initializers and useEffect callbacks (see
customSpecies/customFromStates/customToStates state initializers and their
useEffect savers), which can throw in SSR or blocked storage; wrap each access
in a try/catch and guard for window/localStorage availability before calling
getItem/setItem, returning a safe default ([]) on error, and log the error; also
ensure the useEffect setters similarly guard setItem calls so persisting doesn’t
crash the component if storage is unavailable or throws.
| // Handle to state change | ||
| const handleToChange = (e) => { | ||
| const t = e.target.value; | ||
| const isCustom = e.target.isCustom; | ||
|
|
||
| if (isCustom && t && !customToStates.includes(t)) { | ||
| setCustomToStates(prev => [...prev, t]); | ||
| } | ||
|
|
||
| setToState(t); | ||
| setResult(null); | ||
| }; |
There was a problem hiding this comment.
Reset yield inputs when “To Product” changes to avoid stale calculations.
Line 589-600: switching to a custom or different “To Product” keeps the prior yield/range, so calculations can use stale data and the button stays enabled. Clear yield-related state on change; the existing currentConversion effect will repopulate known conversions.
🐛 Proposed fix
const handleToChange = (e) => {
const t = e.target.value;
const isCustom = e.target.isCustom;
if (isCustom && t && !customToStates.includes(t)) {
setCustomToStates(prev => [...prev, t]);
}
setToState(t);
+ setYieldPercent('');
+ setYieldRange(null);
+ setUseRangeMin(false);
+ setUseRangeMax(false);
setResult(null);
};🤖 Prompt for AI Agents
In `@app/src/components/Calculator.jsx` around lines 589 - 600, When the "To
Product" selection changes in handleToChange, reset all yield-related state to
avoid stale inputs: clear fields such as yield value and yield range by calling
the yield-related setters (e.g., setYield, setYieldRange, setYieldMin,
setYieldMax or whatever yield-state setters exist in this component) right after
setToState and before/when setResult(null) so the UI and validation reflect the
change; keep the existing setCustomToStates behavior and rely on the
currentConversion effect to repopulate known conversion values.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| <button | ||
| onClick={calculate} | ||
| disabled={!species || !toState} | ||
| disabled={!species || !toState || !yieldPercent} |
There was a problem hiding this comment.
Bug: Whitespace-only input for yieldPercent enables the button and silently defaults the calculation to 100% yield, producing an incorrect result.
Severity: HIGH
Suggested Fix
Trim the yieldPercent string before the truthiness check in the disabled prop. Additionally, add validation within the calculate function to handle non-numeric or invalid parsed values more gracefully, for example by disabling the button or showing an error if isNaN(parseFloat(yieldPercent.trim())) is true.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/components/Calculator.jsx#L960
Potential issue: The `disabled` check for the calculate button on line 960 only verifies
the truthiness of the `yieldPercent` state. Inputs that are truthy but not valid
numbers, such as a whitespace-only string (e.g., " "), will enable the button. In the
`calculate` function, `parseFloat(" ")` results in `NaN`. The fallback logic `(NaN ||
100)` then causes the calculation to silently proceed with a 100% yield. This leads to
incorrect financial calculations without any warning to the user, as they may have
unintentionally entered whitespace.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
