feat(settings): weight and temperature unit preferences#2595
Merged
Conversation
Adds user-configurable unit preferences for weight (kg/lb) and temperature (°C/°F) with locale-based defaults, persisted via the existing Legend State preferences store (SQLite + API sync). Key changes: - Extend UserPreferencesSchema with weightUnit and temperatureUnit fields - Add TEMPERATURE_UNITS constant to @packrat/constants - New useWeightUnit and useTemperatureUnit hooks read from preferencesStore with locale-based fallback (expo-localization) - WeightBadge now converts source weight to the preferred unit before display - formatWeatherData stores all temperatures in Celsius; display layer converts to the preferred unit via useTemperatureUnit - Fix computeCategorySummaries and usePackWeightAnalysis to use preferences store instead of the broken userStore.preferredWeightUnit reference - Settings screen gains a "Display Units" section with kg/lb and °C/°F toggles - All weight and temperature display sites updated for consistency
Contributor
|
Warning Review limit reached
More reviews will be available in 45 minutes and 30 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (18)
WalkthroughAdds user-configurable weight, temperature, and speed display units. A locale-aware defaults module and three hooks back preferences in ChangesUnit Preference System
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
Coverage Report for packages/mcp (./packages/mcp)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/analytics (./packages/analytics)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/units (./packages/units)
File CoverageNo changed files found. |
Contributor
Coverage Report for apps/expo (./apps/expo)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Contributor
Coverage Report for packages/overpass (./packages/overpass)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/api (./packages/api)
File CoverageNo changed files found. |
formatWeight, HorizontalCatalogItemCard, and usePackWeightAnalysis were emitting raw floats (e.g. 0.45359237 lb). Switch them to use parseFloat/ toFixed(2) or displayWeight from @packrat/units, which already rounds. Weight-analysis per-item rows now also convert to the preferred unit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/expo/app/(app)/pack-stats/[id].tsx (1)
66-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse preferred weight unit in history display.
Line 67 displays weight history with hardcoded
'g'units. For consistency with the rest of the screen, convert these values to the user's preferredweightUnit.♻️ Proposed fix
+ const { convertWeight } = useWeightUnit(); + // ... in the map function: <Text variant="caption2" className="text-muted-foreground"> - {item.weight.toFixed(1)} g + {convertWeight(item.weight, 'g').toFixed(1)} {weightUnit} </Text>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/pack-stats/[id].tsx around lines 66 - 68, The weight display in the Text component with variant="caption2" is using a hardcoded 'g' unit instead of the user's preferred weightUnit. Replace the hardcoded 'g' string with the appropriate weightUnit value, and convert the item.weight value to the correct unit before displaying it. Ensure the conversion logic matches how weight units are handled elsewhere on the screen for consistency. Reference how weightUnit is accessed and how weight conversions are performed in other parts of the component or utility functions.apps/expo/app/(app)/current-pack/[id].tsx (1)
160-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConvert pack weights to preferred unit.
WeightCarddisplayspack.totalWeightandpack.baseWeightwith hardcoded'g'units. These values are stored in grams but should be converted to the user's preferredweightUnitfor consistent display across the screen.♻️ Proposed fix
+ const { convertWeight } = useWeightUnit(); + const displayBaseWeight = convertWeight(pack?.baseWeight ?? 0, 'g'); + const displayTotalWeight = convertWeight(pack?.totalWeight ?? 0, 'g'); + <View className="mb-4 flex-row gap-3 px-4"> - <WeightCard title={t('packs.totalWeight')} weight={pack?.totalWeight ?? 0} /> - <WeightCard title={t('packs.baseWeight')} weight={pack?.baseWeight ?? 0} /> + <WeightCard title={t('packs.totalWeight')} weight={`${displayTotalWeight} ${weightUnit}`} /> + <WeightCard title={t('packs.baseWeight')} weight={`${displayBaseWeight} ${weightUnit}`} /> </View>Note: You may need to adjust
WeightCardcomponent props to accept string instead of number if not already done.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/current-pack/[id].tsx around lines 160 - 163, The WeightCard components displaying pack.totalWeight and pack.baseWeight are passing raw gram values without converting them to the user's preferred weightUnit. Retrieve the user's preferred weightUnit and use a weight conversion utility function to convert both pack.totalWeight and pack.baseWeight from grams to that preferred unit before passing them to the WeightCard components. This ensures the displayed weights are consistent with the user's preferences across the screen.apps/expo/app/(app)/weight-analysis/[id].tsx (1)
122-124: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider converting item weights to preferred unit.
Individual items display their original
item.weightUnit(line 123), while category summaries and totals usepreferredUnit. This inconsistency may confuse users. Consider converting item weights to the preferred unit for consistency, or document why the original units are preserved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/weight-analysis/[id].tsx around lines 122 - 124, The item weight display is using the original item.weightUnit while category summaries and totals use preferredUnit, creating inconsistency in the UI. Convert item.weight to the preferredUnit using the same conversion logic applied elsewhere in the component, then display the converted weight with preferredUnit instead of the original item.weightUnit to maintain consistency throughout the weight analysis view.apps/expo/features/packs/utils/__tests__/computeCategories.test.ts (1)
64-74: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd test coverage for
preferredUnitparameter.The existing tests only verify the default behavior (grams). Consider adding tests that explicitly pass different
preferredUnitvalues to ensure the conversion logic works correctly for kg, lb, and oz.Example test case
it('converts weight to specified preferred unit (kg)', () => { const items = [makeItem({ weight: 1000, weightUnit: 'g', category: 'Pack' })]; const result = computeCategorySummaries(makePack(items), 'kg'); expect(result[0]?.weight).toBe(1); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/packs/utils/__tests__/computeCategories.test.ts` around lines 64 - 74, The test file for computeCategorySummaries currently only tests the default behavior with grams as the preferred unit. Add new test cases that explicitly pass different preferredUnit parameter values ('kg', 'lb', 'oz') to the computeCategorySummaries function call to verify that weight conversion logic works correctly for each unit. Each test should convert weights from a source unit to the specified preferred unit and assert the converted value matches expectations, similar to the existing unit conversion tests.apps/expo/features/weather/screens/LocationPreviewScreen.tsx (1)
268-269:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDaily forecast temperatures not converted to preferred unit.
Lines 268-269 render
day.lowandday.highdirectly without unit conversion. If these values are in Celsius (as per the upstream weatherService changes), users who prefer Fahrenheit will see incorrect Celsius values labeled with a bare°symbol.Convert these temperatures using
displayTemperature()or at minimumtoPreferred().🔧 Proposed fix
- <Text className="min-w-[30px] text-right text-white/90">{day.low}°</Text> - <Text className="min-w-[30px] text-right text-white">{day.high}°</Text> + <Text className="min-w-[30px] text-right text-white/90">{displayTemperature(day.low)}</Text> + <Text className="min-w-[30px] text-right text-white">{displayTemperature(day.high)}</Text>Note: If
day.lowandday.highare already in the preferred unit from the data structure, then this comment is invalid. However, based on the AI summary and upstream changes, they should be in Celsius.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/weather/screens/LocationPreviewScreen.tsx` around lines 268 - 269, The Text components rendering day.low and day.high temperatures in LocationPreviewScreen are displaying raw Celsius values without converting to the user's preferred unit. Apply the displayTemperature() or toPreferred() function to convert day.low and day.high to the preferred temperature unit before rendering them in both Text elements on lines 268-269.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/app/`(app)/current-pack/[id].tsx:
- Line 130: The function computeCategorySummaries has been refactored to accept
an object parameter instead of positional parameters. Update the call to
computeCategorySummaries in this file to pass the arguments as an object with
properties instead of as separate positional arguments (change from
computeCategorySummaries(pack, weightUnit) to computeCategorySummaries({ pack,
weightUnit }) or similar object syntax matching the updated function signature).
In `@apps/expo/app/`(app)/pack-categories/[id].tsx:
- Line 66: The call to computeCategorySummaries on line 66 is using positional
parameters (pack, weightUnit) but the function signature has been updated to
accept an object parameter instead. Convert this function call to use object
parameter syntax by wrapping the arguments in curly braces as a destructured
object parameter, matching the updated function signature in
computeCategories.ts.
In `@apps/expo/app/`(app)/pack-stats/[id].tsx:
- Line 23: Update the call to computeCategorySummaries to use object parameter
syntax instead of positional parameters. Change the function invocation from
passing pack and weightUnit as separate arguments to passing them as properties
of a single object parameter. The function signature was refactored to expect an
object with pack and weightUnit properties, so the call site must be updated to
match this new interface.
In `@apps/expo/app/`(app)/settings/index.tsx:
- Around line 120-141: The TouchableOpacity buttons for weight unit selection
('kg' and 'lb' toggles) are missing accessibility attributes needed for screen
reader support. Add the accessibilityLabel prop to each TouchableOpacity
component with descriptive labels like "Select kilograms" and "Select pounds",
and add the accessibilityRole prop set to "button" to both components. Apply the
same accessibility pattern to the temperature unit toggles in the temperature
selection section to ensure consistent screen reader support across all unit
toggle buttons.
- Around line 107-178: The Display Units section contains hardcoded English
strings that should use the translation system like the rest of the Settings
screen. Replace all hardcoded strings including "Display Units", "Weight",
"Shown on pack totals and items", "Temperature", and "Shown in weather
forecasts" with corresponding t() function calls (e.g.,
t('settings.displayUnits'), t('settings.weight'), etc.). Update each Text
component in the Display Units View to use the translation function instead of
literal strings, ensuring consistency with the i18n pattern used elsewhere in
the file.
In `@apps/expo/features/auth/hooks/useTemperatureUnit.ts`:
- Around line 14-25: Wrap both the displayTemperature and toPreferred functions
with useCallback hooks to prevent them from being recreated on every render.
Each function should have unit as a dependency in the dependency array, since
both functions depend on the unit value from outer scope. This optimization is
important because these functions are passed to list-based components like
WeatherForecast, and memoizing them will prevent unnecessary re-renders of those
child components.
In `@apps/expo/features/auth/hooks/useWeightUnit.ts`:
- Around line 16-19: The convertWeight function violates the project's API
convention by accepting multiple parameters instead of an object parameter, and
it is recreated on every render because it captures the unit variable from the
outer scope. Refactor the convertWeight function to accept a single object
parameter with weight and fromUnit properties, wrap it in a useCallback hook to
memoize it and prevent unnecessary re-creation, and update all call sites that
invoke convertWeight to pass an object argument instead of separate positional
arguments.
In `@apps/expo/features/packs/hooks/usePackWeightAnalysis.ts`:
- Around line 38-41: The totalWeight calculation in the usePackWeightAnalysis
hook contains a redundant conversion where pack.totalWeight (which is already in
grams) is being converted from grams to grams via convertToGrams before
converting to the preferred unit. Simplify this by removing the convertToGrams
call and passing pack.totalWeight directly as the grams parameter to
convertFromGrams, then specify the target unit as preferredUnit.
- Around line 32-35: In the usePackWeightAnalysis hook's baseWeight assignment,
remove the redundant convertToGrams call since pack.baseWeight is already stored
in grams. Simplify the code by passing pack.baseWeight directly as the grams
parameter to the convertFromGrams function, eliminating the unnecessary
conversion from grams to grams that occurs when calling convertToGrams with unit
'g'.
In `@apps/expo/features/packs/utils/computeCategories.ts`:
- Around line 13-16: The computeCategorySummaries function signature violates
the linter rule requiring APIs with multiple parameters to use a single object
parameter. Refactor the function signature to accept a single object parameter
with properties pack and preferredUnit instead of accepting them as separate
parameters. Then update all call sites throughout the codebase where
computeCategorySummaries is invoked to pass an object with these properties
rather than individual arguments.
In `@apps/expo/features/trips/screens/TripWeatherDetailsScreen.tsx`:
- Around line 140-141: The temperature display in the TripWeatherDetailsScreen
has two issues: First, replace the toPreferred() function calls with
displayTemperature() to ensure consistent unit labeling across the component
(matching the pattern used on other screens). Second, replace the ?? 0 fallback
values with a more appropriate placeholder like '—' instead of using 0, which
incorrectly displays as real weather data (0°C or 32°F) when the
forecast.forecastday[0]?.day.maxtemp_c and mintemp_c values are missing, making
it ambiguous whether the data is actual or missing.
In `@apps/expo/features/weather/components/LocationCard.tsx`:
- Around line 112-117: The high and low temperature display is inconsistent with
the main temperature format. The main temperature uses displayTemperature()
which returns a fully formatted string including the unit suffix (e.g., "72°F"),
but the H/L temperature section uses toPreferred() which only returns the
numeric value, then manually appends "°" without the unit suffix. Replace the
toPreferred() calls for location.highTemp and location.lowTemp in the H/L
temperature Text component with displayTemperature() function calls to ensure
the high and low temperatures display with complete unit information matching
the main temperature format (e.g., "H:72°F L:65°F" instead of "H:72° L:65°").
In `@apps/expo/features/weather/components/WeatherForecast.tsx`:
- Around line 95-102: Replace the `toPreferred()` calls with
`displayTemperature()` for the day.low and day.high temperature displays in the
Text components to include the full unit suffix and maintain consistency across
the component. Additionally, the chart domain calculation using the hardcoded
Celsius values [4, 38] needs to be adapted based on the preferred temperature
unit since day.low and day.high are in Celsius but the visual scaling should
match the displayed unit preference. Convert the domain bounds [4, 38] to the
preferred unit when calculating the left and right percentages for the bar
positioning. First, destructure the `unit` return value from the
`useTemperatureUnit()` hook on line 45 to access the user's preferred
temperature unit.
In `@apps/expo/features/weather/screens/LocationDetailScreen.tsx`:
- Line 241: The temperature display on line 241 uses toPreferred() with a bare
degree symbol suffix, which is inconsistent with the displayTemperature()
function used elsewhere on the screen that properly includes the unit
abbreviation (F or C). Replace the toPreferred() calls for location.highTemp and
location.lowTemp with displayTemperature() calls to ensure all temperature
values on the screen show the full unit label (like "75°F" or "24°C") for
consistent formatting.
In `@apps/expo/features/weather/screens/LocationPreviewScreen.tsx`:
- Line 193: Replace the `toPreferred()` function calls with
`displayTemperature()` for both `weatherData.highTemp` and `weatherData.lowTemp`
on this line in LocationPreviewScreen.tsx. Remove the bare `°` symbols that
follow each function call, since `displayTemperature()` will include the
complete temperature unit suffix (like °F or °C) based on user preferences,
ensuring consistent temperature display format across the application.
In `@packages/schemas/src/users.ts`:
- Around line 11-12: The weightUnit and temperatureUnit fields in the
UserPreferencesSchema contain hardcoded enum values that duplicate WEIGHT_UNITS
and TEMPERATURE_UNITS constants from the '`@packrat/constants`' package, creating
maintenance overhead when constants change. Import WEIGHT_UNITS and
TEMPERATURE_UNITS from '`@packrat/constants`' and replace the hardcoded enum
arrays in the z.enum() calls for both weightUnit and temperatureUnit with the
imported constants to establish a single source of truth.
---
Outside diff comments:
In `@apps/expo/app/`(app)/current-pack/[id].tsx:
- Around line 160-163: The WeightCard components displaying pack.totalWeight and
pack.baseWeight are passing raw gram values without converting them to the
user's preferred weightUnit. Retrieve the user's preferred weightUnit and use a
weight conversion utility function to convert both pack.totalWeight and
pack.baseWeight from grams to that preferred unit before passing them to the
WeightCard components. This ensures the displayed weights are consistent with
the user's preferences across the screen.
In `@apps/expo/app/`(app)/pack-stats/[id].tsx:
- Around line 66-68: The weight display in the Text component with
variant="caption2" is using a hardcoded 'g' unit instead of the user's preferred
weightUnit. Replace the hardcoded 'g' string with the appropriate weightUnit
value, and convert the item.weight value to the correct unit before displaying
it. Ensure the conversion logic matches how weight units are handled elsewhere
on the screen for consistency. Reference how weightUnit is accessed and how
weight conversions are performed in other parts of the component or utility
functions.
In `@apps/expo/app/`(app)/weight-analysis/[id].tsx:
- Around line 122-124: The item weight display is using the original
item.weightUnit while category summaries and totals use preferredUnit, creating
inconsistency in the UI. Convert item.weight to the preferredUnit using the same
conversion logic applied elsewhere in the component, then display the converted
weight with preferredUnit instead of the original item.weightUnit to maintain
consistency throughout the weight analysis view.
In `@apps/expo/features/packs/utils/__tests__/computeCategories.test.ts`:
- Around line 64-74: The test file for computeCategorySummaries currently only
tests the default behavior with grams as the preferred unit. Add new test cases
that explicitly pass different preferredUnit parameter values ('kg', 'lb', 'oz')
to the computeCategorySummaries function call to verify that weight conversion
logic works correctly for each unit. Each test should convert weights from a
source unit to the specified preferred unit and assert the converted value
matches expectations, similar to the existing unit conversion tests.
In `@apps/expo/features/weather/screens/LocationPreviewScreen.tsx`:
- Around line 268-269: The Text components rendering day.low and day.high
temperatures in LocationPreviewScreen are displaying raw Celsius values without
converting to the user's preferred unit. Apply the displayTemperature() or
toPreferred() function to convert day.low and day.high to the preferred
temperature unit before rendering them in both Text elements on lines 268-269.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a8cf280e-5178-4306-a8f8-704cd28d0a17
📒 Files selected for processing (24)
apps/expo/app/(app)/current-pack/[id].tsxapps/expo/app/(app)/pack-categories/[id].tsxapps/expo/app/(app)/pack-stats/[id].tsxapps/expo/app/(app)/settings/index.tsxapps/expo/app/(app)/weight-analysis/[id].tsxapps/expo/components/initial/WeightBadge.tsxapps/expo/features/ai/components/LocationContext.tsxapps/expo/features/ai/components/WeatherGenerativeUI.tsxapps/expo/features/auth/hooks/useTemperatureUnit.tsapps/expo/features/auth/hooks/useWeightUnit.tsapps/expo/features/packs/hooks/usePackWeightAnalysis.tsapps/expo/features/packs/utils/__tests__/computeCategories.test.tsapps/expo/features/packs/utils/computeCategories.tsapps/expo/features/trips/screens/TripWeatherDetailsScreen.tsxapps/expo/features/weather/components/LocationCard.tsxapps/expo/features/weather/components/LocationPicker.tsxapps/expo/features/weather/components/WeatherForecast.tsxapps/expo/features/weather/components/WeatherTile.tsxapps/expo/features/weather/lib/weatherService.tsapps/expo/features/weather/screens/LocationDetailScreen.tsxapps/expo/features/weather/screens/LocationPreviewScreen.tsxapps/expo/lib/unitDefaults.tspackages/constants/src/index.tspackages/schemas/src/users.ts
Comment on lines
+107
to
+178
| <View> | ||
| <Text variant="subhead" className="mb-3"> | ||
| Display Units | ||
| </Text> | ||
| <View className="rounded-xl border border-border bg-card"> | ||
| <View className="flex-row items-center justify-between p-4"> | ||
| <View className="flex-1"> | ||
| <Text className="font-medium">Weight</Text> | ||
| <Text variant="footnote" className="mt-0.5 text-muted-foreground"> | ||
| Shown on pack totals and items | ||
| </Text> | ||
| </View> | ||
| <View className="flex-row overflow-hidden rounded-lg border border-border"> | ||
| <TouchableOpacity | ||
| className={`px-4 py-2 ${weightUnit === 'kg' ? 'bg-primary' : 'bg-card'}`} | ||
| onPress={() => setWeightUnit('kg')} | ||
| > | ||
| <Text | ||
| variant="footnote" | ||
| className={`font-medium ${weightUnit === 'kg' ? 'text-primary-foreground' : 'text-foreground'}`} | ||
| > | ||
| kg | ||
| </Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity | ||
| className={`px-4 py-2 ${weightUnit === 'lb' ? 'bg-primary' : 'bg-card'}`} | ||
| onPress={() => setWeightUnit('lb')} | ||
| > | ||
| <Text | ||
| variant="footnote" | ||
| className={`font-medium ${weightUnit === 'lb' ? 'text-primary-foreground' : 'text-foreground'}`} | ||
| > | ||
| lb | ||
| </Text> | ||
| </TouchableOpacity> | ||
| </View> | ||
| </View> | ||
| <View className="h-px bg-border mx-4" /> | ||
| <View className="flex-row items-center justify-between p-4"> | ||
| <View className="flex-1"> | ||
| <Text className="font-medium">Temperature</Text> | ||
| <Text variant="footnote" className="mt-0.5 text-muted-foreground"> | ||
| Shown in weather forecasts | ||
| </Text> | ||
| </View> | ||
| <View className="flex-row overflow-hidden rounded-lg border border-border"> | ||
| <TouchableOpacity | ||
| className={`px-4 py-2 ${temperatureUnit === 'C' ? 'bg-primary' : 'bg-card'}`} | ||
| onPress={() => setTemperatureUnit('C')} | ||
| > | ||
| <Text | ||
| variant="footnote" | ||
| className={`font-medium ${temperatureUnit === 'C' ? 'text-primary-foreground' : 'text-foreground'}`} | ||
| > | ||
| °C | ||
| </Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity | ||
| className={`px-4 py-2 ${temperatureUnit === 'F' ? 'bg-primary' : 'bg-card'}`} | ||
| onPress={() => setTemperatureUnit('F')} | ||
| > | ||
| <Text | ||
| variant="footnote" | ||
| className={`font-medium ${temperatureUnit === 'F' ? 'text-primary-foreground' : 'text-foreground'}`} | ||
| > | ||
| °F | ||
| </Text> | ||
| </TouchableOpacity> | ||
| </View> | ||
| </View> | ||
| </View> | ||
| </View> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add internationalization for new UI strings to maintain consistency.
The new "Display Units" section hardcodes English strings ("Display Units", "Weight", "Temperature", etc.), while the rest of the Settings screen uses the t() translation system (e.g., line 182: t('ai.modelManagement')). This creates an i18n gap.
Add translation keys and use t():
🌐 Suggested i18n integration
<View>
- <Text variant="subhead" className="mb-3">
- Display Units
- </Text>
+ <Text variant="subhead" className="mb-3">
+ {t('settings.displayUnits')}
+ </Text>
<View className="rounded-xl border border-border bg-card">
<View className="flex-row items-center justify-between p-4">
<View className="flex-1">
- <Text className="font-medium">Weight</Text>
+ <Text className="font-medium">{t('settings.weight')}</Text>
- <Text variant="footnote" className="mt-0.5 text-muted-foreground">
- Shown on pack totals and items
- </Text>
+ <Text variant="footnote" className="mt-0.5 text-muted-foreground">
+ {t('settings.weightDescription')}
+ </Text>
</View>
{/* ... */}
</View>
{/* ... */}
<View className="flex-row items-center justify-between p-4">
<View className="flex-1">
- <Text className="font-medium">Temperature</Text>
+ <Text className="font-medium">{t('settings.temperature')}</Text>
- <Text variant="footnote" className="mt-0.5 text-muted-foreground">
- Shown in weather forecasts
- </Text>
+ <Text variant="footnote" className="mt-0.5 text-muted-foreground">
+ {t('settings.temperatureDescription')}
+ </Text>
</View>
{/* ... */}
</View>
</View>
</View>Add corresponding keys to your translation files (e.g., en.json):
{
"settings": {
"displayUnits": "Display Units",
"weight": "Weight",
"weightDescription": "Shown on pack totals and items",
"temperature": "Temperature",
"temperatureDescription": "Shown in weather forecasts"
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/app/`(app)/settings/index.tsx around lines 107 - 178, The Display
Units section contains hardcoded English strings that should use the translation
system like the rest of the Settings screen. Replace all hardcoded strings
including "Display Units", "Weight", "Shown on pack totals and items",
"Temperature", and "Shown in weather forecasts" with corresponding t() function
calls (e.g., t('settings.displayUnits'), t('settings.weight'), etc.). Update
each Text component in the Display Units View to use the translation function
instead of literal strings, ensuring consistency with the i18n pattern used
elsewhere in the file.
Comment on lines
+95
to
+102
| left: `${Math.max(0, ((day.low - 4) / (38 - 4)) * 100)}%`, | ||
| right: `${Math.max(0, 100 - ((day.high - 4) / (38 - 4)) * 100)}%`, | ||
| }} | ||
| /> | ||
| </View> | ||
| </View> | ||
| <Text className="min-w-[30px] text-right text-white/90">{day.low}°</Text> | ||
| <Text className="min-w-[30px] text-right text-white">{day.high}°</Text> | ||
| <Text className="min-w-[30px] text-right text-white/90">{toPreferred(day.low)}°</Text> | ||
| <Text className="min-w-[30px] text-right text-white">{toPreferred(day.high)}°</Text> |
Contributor
There was a problem hiding this comment.
Inconsistent temperature unit display and incorrect chart scaling.
Two issues here:
-
Lines 101-102: Using
toPreferred()with a bare°suffix creates inconsistency. When the user prefers Fahrenheit, this shows "75°" instead of "75°F". Other temperatures in this component usedisplayTemperature()which includes the full unit suffix. UsedisplayTemperature()here for consistency. -
Lines 95-96: The chart domain
[4, 38]is fixed to Celsius, butday.lowandday.highare also in Celsius. When the user prefers Fahrenheit and sees "75°" displayed, the bar position is calculated using the Celsius value (~24°C) against a Celsius domain, making the visual position incorrect. The domain should adapt to the preferred unit.
🔧 Proposed fix
+ const minTemp = toPreferred(day.low);
+ const maxTemp = toPreferred(day.high);
+ // Domain for Celsius: [4, 38], for Fahrenheit: [39, 100]
+ const domainMin = unit === 'F' ? 39 : 4;
+ const domainMax = unit === 'F' ? 100 : 38;
<View className="h-1 flex-1 overflow-hidden rounded-full bg-white/30">
<View
className="absolute h-1 bg-white"
style={{
- left: `${Math.max(0, ((day.low - 4) / (38 - 4)) * 100)}%`,
- right: `${Math.max(0, 100 - ((day.high - 4) / (38 - 4)) * 100)}%`,
+ left: `${Math.max(0, ((minTemp - domainMin) / (domainMax - domainMin)) * 100)}%`,
+ right: `${Math.max(0, 100 - ((maxTemp - domainMin) / (domainMax - domainMin)) * 100)}%`,
}}
/>
</View>
</View>
- <Text className="min-w-[30px] text-right text-white/90">{toPreferred(day.low)}°</Text>
- <Text className="min-w-[30px] text-right text-white">{toPreferred(day.high)}°</Text>
+ <Text className="min-w-[30px] text-right text-white/90">{displayTemperature(day.low)}</Text>
+ <Text className="min-w-[30px] text-right text-white">{displayTemperature(day.high)}</Text>Note: You'll need to destructure unit from useTemperatureUnit() on line 45.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/weather/components/WeatherForecast.tsx` around lines 95 -
102, Replace the `toPreferred()` calls with `displayTemperature()` for the
day.low and day.high temperature displays in the Text components to include the
full unit suffix and maintain consistency across the component. Additionally,
the chart domain calculation using the hardcoded Celsius values [4, 38] needs to
be adapted based on the preferred temperature unit since day.low and day.high
are in Celsius but the visual scaling should match the displayed unit
preference. Convert the domain bounds [4, 38] to the preferred unit when
calculating the left and right percentages for the bar positioning. First,
destructure the `unit` return value from the `useTemperatureUnit()` hook on line
45 to access the user's preferred temperature unit.
Comment on lines
+11
to
+12
| weightUnit: z.enum(['kg', 'lb']).optional(), | ||
| temperatureUnit: z.enum(['C', 'F']).optional(), |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Consider importing enum values from @packrat/constants to eliminate duplication.
The enum values ['kg', 'lb'] and ['C', 'F'] are duplicated from WEIGHT_UNITS and TEMPERATURE_UNITS in the constants package. If the constants evolve (e.g., adding 'st' for stone), this schema must be manually synchronized. To maintain a single source of truth, consider:
import { WEIGHT_UNITS, TEMPERATURE_UNITS } from '`@packrat/constants`';
export const UserPreferencesSchema = z.object({
// ...
weightUnit: z.enum(['kg', 'lb'] satisfies readonly (typeof WEIGHT_UNITS[number])[]).optional(),
temperatureUnit: z.enum(['C', 'F'] satisfies readonly (typeof TEMPERATURE_UNITS[number])[]).optional(),
});Or define a subset explicitly if the schema intentionally restricts to a subset of all weight units.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/schemas/src/users.ts` around lines 11 - 12, The weightUnit and
temperatureUnit fields in the UserPreferencesSchema contain hardcoded enum
values that duplicate WEIGHT_UNITS and TEMPERATURE_UNITS constants from the
'`@packrat/constants`' package, creating maintenance overhead when constants
change. Import WEIGHT_UNITS and TEMPERATURE_UNITS from '`@packrat/constants`' and
replace the hardcoded enum arrays in the z.enum() calls for both weightUnit and
temperatureUnit with the imported constants to establish a single source of
truth.
WeightCard, ItemRow in current-pack, and weight history bars in pack-stats were all displaying raw gram values with a hardcoded 'g' label. Wire them through useWeightUnit so they convert to the preferred unit and round via displayWeight (max 2dp).
Both the local model system prompt and the remote API body now include the user's weightUnit and temperatureUnit so the AI can respond in the correct units (e.g. reply in kg/°C vs lb/°F without the user having to ask).
…urfaces Catalog item cards, select cards, similar items, detail screen (main weight chip + variant rows) all now display in the user's preferred unit via useWeightUnit/convertWeight. AddCatalogItemDetailsScreen also converts the weight value before saving to pack so the stored item uses the preferred unit.
…distance unit Bugs fixed: - WeightBadge: add space between converted value and unit label - Temperature H/L displays: replace toPreferred+° with displayTemperature so unit letter (F/C) is always shown alongside the value - WeatherForecast/LocationPreviewScreen: fix wind label mph→km/h and visibility label mi→km (weatherService now stores metric internally) - LocationPreviewScreen: fix daily forecast temp bar thresholds from °F range (40–100) to °C range (4–38) Conventions: - Settings: replace hand-rolled toggle buttons with SegmentedControl - Settings: add testID props (settings:weight-unit, settings:temperature-unit, settings:speed-unit) via testIds registry - Settings: broaden subtitle copy to describe unit scope, not specific screens Oversights: - HorizontalCatalogItemCard: convert weight via useWeightUnit (was the only catalog card still showing raw DB units) Type safety: - computeCategorySummaries: remove default 'g' parameter so callers must pass preferredUnit explicitly; add kg/lb conversion test cases Speed/distance locale preference (new): - SPEED_UNITS / SpeedUnit added to packages/constants - speedUnit added to UserPreferencesSchema (persists via API like weight/temp) - getDefaultSpeedUnit() in unitDefaults.ts reuses FAHRENHEIT_REGIONS (mph correlates with same locales as °F) - useSpeedUnit hook: displayWindSpeed(kph) and displayVisibility(km) - WeatherForecast + LocationPreviewScreen wired to hook - Settings: Wind & Distance row in Display Units card - AI context: speedUnit threaded into local and remote system prompts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/expo/app/(app)/ai-chat.tsx (1)
187-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocal AI unit preferences can become stale after settings changes.
Line 205–207 interpolates unit refs into a static
systemPrompt, but the local transport is keyed as'local'and memoized without unit dependencies. After a user toggles units, local-mode responses can keep using old units until another remount trigger occurs.Suggested fix
- return { - transport: new CustomChatTransport({ model, tools, systemPrompt }), - transportKey: 'local', - }; + return { + transport: new CustomChatTransport({ model, tools, systemPrompt }), + transportKey: `local:${weightUnit}:${temperatureUnit}:${speedUnit}`, + }; ... - }, [aiMode, isLocalReady, modelStatus, token, tools, userId]); + }, [aiMode, isLocalReady, modelStatus, token, tools, userId, weightUnit, temperatureUnit, speedUnit]);Also applies to: 267-267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/ai-chat.tsx around lines 187 - 223, The useMemo hook containing the transport and transportKey memoization is missing unit preference dependencies. The systemPrompt string interpolates weightUnitRef.current, temperatureUnitRef.current, and speedUnitRef.current values, but these refs are not included in the useMemo dependency array, causing stale unit preferences to persist in the CustomChatTransport even after user settings change. Add all three unit refs (weightUnitRef, temperatureUnitRef, speedUnitRef) to the dependency array of the useMemo hook so the transport is recreated whenever unit preferences change.packages/api/src/routes/chat.ts (1)
37-47:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe new unit fields are type-cast, not runtime-validated.
typedBodyassumesweightUnit/temperatureUnit/speedUnitunions, butbody: ChatRequestSchemacurrently permits any shape. That leaves the route contract unenforced at runtime and allows arbitrary values into system prompt context.Suggested fix
- const typedBody = body as { - messages?: UIMessage[] | undefined; - contextType?: string; - itemId?: string; - packId?: string; - location?: string; - date: string; - weightUnit?: 'kg' | 'lb'; - temperatureUnit?: 'C' | 'F'; - speedUnit?: 'kmh' | 'mph'; - }; + const typedBody = z + .object({ + messages: z.any().optional(), + contextType: z.string().optional(), + itemId: z.string().optional(), + packId: z.string().optional(), + location: z.string().optional(), + date: z.string(), + weightUnit: z.enum(['kg', 'lb']).optional(), + temperatureUnit: z.enum(['C', 'F']).optional(), + speedUnit: z.enum(['kmh', 'mph']).optional(), + }) + .parse(body);As per coding guidelines, “Validation schemas live in
src/schemas/(Zod).”Also applies to: 198-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/routes/chat.ts` around lines 37 - 47, The weightUnit, temperatureUnit, and speedUnit fields in the typedBody are type-cast but not runtime-validated, allowing arbitrary values to pass through into system prompt context. Update the ChatRequestSchema in src/schemas/ to include Zod validation for these three fields, ensuring weightUnit validates to only 'kg' or 'lb', temperatureUnit validates to only 'C' or 'F', and speedUnit validates to only 'kmh' or 'mph'. This will enforce the contract at runtime rather than relying on TypeScript casting alone.Source: Coding guidelines
apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx (1)
77-123:⚠️ Potential issue | 🟠 MajorGuard the async add flow with
try/catch/finally.If any awaited step fails,
isAddingcan remaintrueand the screen can get stuck in loading state. Wrap this handler intry/catch/finally, reset loading infinally, and capture the error incatchwith Sentry instrumentation from@sentry/react-nativeper coding guidelines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx` around lines 77 - 123, The handleAddToPack function contains awaited operations that could fail, leaving isAdding in a true state. Wrap the entire function body in a try/catch/finally block, moving the setIsAdding(false) call into the finally block to ensure it always executes. In the catch block, capture any errors that occur during image caching or item creation and log them to Sentry using `@sentry/react-native` instrumentation as per coding guidelines, then display an appropriate error toast to the user.Source: Coding guidelines
♻️ Duplicate comments (1)
apps/expo/features/packs/utils/computeCategories.ts (1)
13-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefactor
computeCategorySummariesto a single object parameter (currently CI-blocking).Line 13 still violates the owned-API max-params lint rule, and the pipeline is failing on it. Convert to one object argument and update callers (including
apps/expo/features/packs/hooks/usePackWeightAnalysis.tsandapps/expo/features/packs/utils/__tests__/computeCategories.test.ts).♻️ Proposed fix
-export function computeCategorySummaries(pack: Pack, preferredUnit: WeightUnit): CategorySummary[] { +export function computeCategorySummaries({ + pack, + preferredUnit, +}: { + pack: Pack; + preferredUnit: WeightUnit; +}): CategorySummary[] {-const categorySummaries = computeCategorySummaries(pack, preferredUnit); +const categorySummaries = computeCategorySummaries({ pack, preferredUnit });-expect(computeCategorySummaries(makePack([]), 'g')).toEqual([]); +expect(computeCategorySummaries({ pack: makePack([]), preferredUnit: 'g' })).toEqual([]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/packs/utils/computeCategories.ts` at line 13, The function computeCategorySummaries in computeCategories.ts violates the max-params lint rule due to having multiple parameters (pack and preferredUnit). Refactor the function to accept a single object parameter containing these properties instead of separate parameters. Then update all callers of this function to pass an object with the properties pack and preferredUnit, including the calls in usePackWeightAnalysis.ts and all test cases in the computeCategories.test.ts file.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/features/auth/hooks/useSpeedUnit.ts`:
- Around line 7-10: The hook is re-declaring the speed unit type as a string
literal union 'mph' | 'kmh' in multiple places instead of using a shared
SpeedUnit type, which can cause contract drift if the type definition changes
elsewhere. Replace the inline type declarations in both the unit variable and
the setSpeedUnit function parameter with the shared SpeedUnit type. Import the
SpeedUnit type from the appropriate constants or schema file and use it
consistently throughout the useSpeedUnit hook to maintain a single source of
truth.
In `@apps/expo/features/catalog/components/CatalogItemCard.tsx`:
- Around line 73-75: The convertWeight() call in the weight display logic does
not validate that item.weightUnit contains an actual valid unit value before
using it. While the null coalescing operator handles undefined/null cases, it
allows invalid string values like 'lbs' to pass through, causing convertWeight()
to return NaN. Replace the simple null coalescing fallback with proper
validation by using the existing isWeightUnit() guard to check if the weightUnit
is valid, and only pass it to convertWeight() if validation succeeds, otherwise
use a safe default unit like 'g' to ensure the function receives only valid enum
values.
In `@apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx`:
- Around line 88-95: The weight field assignment in
AddCatalogItemDetailsScreen.tsx (around line 88) is using displayWeight() to
persist the weight value, which rounds to 2 decimals and causes permanent data
loss. Remove the displayWeight() wrapper from the weight field assignment and
instead store the raw normalized weight value directly using the normalize()
function. Apply displayWeight() only at render-time when the value needs to be
displayed to the user, following the pattern used in useBulkAddCatalogItems.ts
line 36.
In `@apps/expo/features/packs/components/WeightAnalysisTile.tsx`:
- Line 19: The packWeight variable on line 19 is computed from
currentPack?.totalWeight, but on line 44 this value is labeled as "Base" in the
UI, which is a mismatch. Find the location where the "Base" label is displayed
alongside packWeight (around line 44) and update the label text to accurately
reflect that it represents the total weight of the pack, not just the base
weight.
In `@apps/expo/features/trips/screens/TripWeatherDetailsScreen.tsx`:
- Line 97: Move the useTemperatureUnit hook call to the very beginning of the
TripWeatherDetailsScreen component, before any conditional returns or early exit
statements. The hook must be initialized at the top level before any return
statements (including the early returns that exist in lines 76-93) to ensure the
hook is always called in the same order on every render, complying with React's
Rules of Hooks.
---
Outside diff comments:
In `@apps/expo/app/`(app)/ai-chat.tsx:
- Around line 187-223: The useMemo hook containing the transport and
transportKey memoization is missing unit preference dependencies. The
systemPrompt string interpolates weightUnitRef.current,
temperatureUnitRef.current, and speedUnitRef.current values, but these refs are
not included in the useMemo dependency array, causing stale unit preferences to
persist in the CustomChatTransport even after user settings change. Add all
three unit refs (weightUnitRef, temperatureUnitRef, speedUnitRef) to the
dependency array of the useMemo hook so the transport is recreated whenever unit
preferences change.
In `@apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx`:
- Around line 77-123: The handleAddToPack function contains awaited operations
that could fail, leaving isAdding in a true state. Wrap the entire function body
in a try/catch/finally block, moving the setIsAdding(false) call into the
finally block to ensure it always executes. In the catch block, capture any
errors that occur during image caching or item creation and log them to Sentry
using `@sentry/react-native` instrumentation as per coding guidelines, then
display an appropriate error toast to the user.
In `@packages/api/src/routes/chat.ts`:
- Around line 37-47: The weightUnit, temperatureUnit, and speedUnit fields in
the typedBody are type-cast but not runtime-validated, allowing arbitrary values
to pass through into system prompt context. Update the ChatRequestSchema in
src/schemas/ to include Zod validation for these three fields, ensuring
weightUnit validates to only 'kg' or 'lb', temperatureUnit validates to only 'C'
or 'F', and speedUnit validates to only 'kmh' or 'mph'. This will enforce the
contract at runtime rather than relying on TypeScript casting alone.
---
Duplicate comments:
In `@apps/expo/features/packs/utils/computeCategories.ts`:
- Line 13: The function computeCategorySummaries in computeCategories.ts
violates the max-params lint rule due to having multiple parameters (pack and
preferredUnit). Refactor the function to accept a single object parameter
containing these properties instead of separate parameters. Then update all
callers of this function to pass an object with the properties pack and
preferredUnit, including the calls in usePackWeightAnalysis.ts and all test
cases in the computeCategories.test.ts file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6c6e73a-528b-4189-b74f-4c154c9960e4
📒 Files selected for processing (32)
apps/expo/app/(app)/_layout.tsxapps/expo/app/(app)/ai-chat.tsxapps/expo/app/(app)/current-pack/[id].tsxapps/expo/app/(app)/pack-stats/[id].tsxapps/expo/app/(app)/settings/index.tsxapps/expo/app/(app)/weight-analysis/[id].tsxapps/expo/components/initial/WeightBadge.tsxapps/expo/features/auth/hooks/useSpeedUnit.tsapps/expo/features/catalog/components/CatalogItemCard.tsxapps/expo/features/catalog/components/CatalogItemSelectCard.tsxapps/expo/features/catalog/components/SimilarItems.tsxapps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsxapps/expo/features/catalog/screens/CatalogItemDetailScreen.tsxapps/expo/features/packs/components/CurrentPackTile.tsxapps/expo/features/packs/components/HorizontalCatalogItemCard.tsxapps/expo/features/packs/components/WeightAnalysisTile.tsxapps/expo/features/packs/hooks/usePackWeightAnalysis.tsapps/expo/features/packs/screens/CreatePackItemForm.tsxapps/expo/features/packs/utils/__tests__/computeCategories.test.tsapps/expo/features/packs/utils/computeCategories.tsapps/expo/features/trips/screens/TripWeatherDetailsScreen.tsxapps/expo/features/weather/components/LocationCard.tsxapps/expo/features/weather/components/WeatherForecast.tsxapps/expo/features/weather/screens/LocationDetailScreen.tsxapps/expo/features/weather/screens/LocationPreviewScreen.tsxapps/expo/features/weather/screens/LocationSearchScreen.tsxapps/expo/lib/testIds.tsapps/expo/lib/unitDefaults.tsapps/expo/utils/weight.tspackages/api/src/routes/chat.tspackages/constants/src/index.tspackages/schemas/src/users.ts
Comment on lines
+7
to
+10
| const unit: 'mph' | 'kmh' = stored ?? getDefaultSpeedUnit(); | ||
|
|
||
| const setSpeedUnit = (value: 'mph' | 'kmh') => { | ||
| preferencesStore.speedUnit.set(value); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use shared SpeedUnit type to avoid contract drift.
This hook re-declares 'mph' | 'kmh' instead of using the exported shared type, which can desync from constants/schema over time.
Suggested patch
+import type { SpeedUnit } from '`@packrat/constants`';
import { use$ } from '`@legendapp/state/react`';
import { preferencesStore } from 'expo-app/features/auth/store/preferences';
import { getDefaultSpeedUnit } from 'expo-app/lib/unitDefaults';
export function useSpeedUnit() {
const stored = use$(preferencesStore.speedUnit);
- const unit: 'mph' | 'kmh' = stored ?? getDefaultSpeedUnit();
+ const unit: SpeedUnit = stored ?? getDefaultSpeedUnit();
- const setSpeedUnit = (value: 'mph' | 'kmh') => {
+ const setSpeedUnit = (value: SpeedUnit) => {
preferencesStore.speedUnit.set(value);
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/auth/hooks/useSpeedUnit.ts` around lines 7 - 10, The hook
is re-declaring the speed unit type as a string literal union 'mph' | 'kmh' in
multiple places instead of using a shared SpeedUnit type, which can cause
contract drift if the type definition changes elsewhere. Replace the inline type
declarations in both the unit variable and the setSpeedUnit function parameter
with the shared SpeedUnit type. Import the SpeedUnit type from the appropriate
constants or schema file and use it consistently throughout the useSpeedUnit
hook to maintain a single source of truth.
Comment on lines
+73
to
+75
| {item.weight != null | ||
| ? `${convertWeight(item.weight, item.weightUnit ?? 'g')} ${unit}` | ||
| : ''} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect CatalogItem type shape and weightUnit typing
rg -n -C3 --type=ts 'interface\s+CatalogItem|type\s+CatalogItem|weightUnit' apps/expo/features/catalog
# 2) Locate schema/validation points for catalog payloads
rg -n -C3 --type=ts 'CatalogItemSchema|z\\.object\\(|weightUnit|parseWeightUnit\\(' apps/expo/features/catalog packages/schemasRepository: PackRat-AI/PackRat
Length of output: 10667
🏁 Script executed:
# Find CatalogItemSchema definition
rg -n "CatalogItemSchema\s*=" apps/expo/features/catalog packages/schemas -A 20
# Find convertWeight implementation
rg -n "function convertWeight|const convertWeight" apps/expo -A 10Repository: PackRat-AI/PackRat
Length of output: 3541
🏁 Script executed:
# Find WEIGHT_UNITS definition
rg -n "WEIGHT_UNITS\s*=" packages/schemas apps/expo -B 2 -A 5
# Find normalize function
rg -n "function normalize|const normalize" apps/expo packages -A 8Repository: PackRat-AI/PackRat
Length of output: 17633
🏁 Script executed:
# Find TO_GRAMS definition
rg -n "TO_GRAMS\s*=" packages/units -A 10Repository: PackRat-AI/PackRat
Length of output: 588
Guard weightUnit against invalid values in convertWeight() calls.
The code safely handles null/undefined with the ?? 'g' fallback, but invalid string values bypass this check. The useCatalogItems hook comments confirm this pattern: "live DB has weightUnit values outside the enum like 'lbs'". When validation fails, the fallback returns raw items, allowing invalid units to reach the UI. Accessing TO_GRAMS[invalidUnit] returns undefined, causing convertWeight() to produce NaN.
Parse or validate the unit before conversion, or update the fallback strategy to coerce invalid units to a safe default (e.g., via isWeightUnit() guard that already exists in the codebase).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/catalog/components/CatalogItemCard.tsx` around lines 73 -
75, The convertWeight() call in the weight display logic does not validate that
item.weightUnit contains an actual valid unit value before using it. While the
null coalescing operator handles undefined/null cases, it allows invalid string
values like 'lbs' to pass through, causing convertWeight() to return NaN.
Replace the simple null coalescing fallback with proper validation by using the
existing isWeightUnit() guard to check if the weightUnit is valid, and only pass
it to convertWeight() if validation succeeds, otherwise use a safe default unit
like 'g' to ensure the function receives only valid enum values.
Comment on lines
+88
to
+95
| weight: displayWeight({ | ||
| grams: normalize({ | ||
| weight: catalogItem.weight || 0, | ||
| unit: parseWeightUnit({ value: catalogItem.weightUnit }), | ||
| }), | ||
| unit: preferredWeightUnit, | ||
| }), | ||
| weightUnit: preferredWeightUnit, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify rounding behavior and persistence usage
rg -n "export function displayWeight|toFixed\\(2\\)" packages/units/src
rg -n "weight:\\s*displayWeight\\(" apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx -C4Repository: PackRat-AI/PackRat
Length of output: 636
🏁 Script executed:
#!/bin/bash
# Get the full function context around line 88
sed -n '70,120p' apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx
# Check if there's a mutation or API call that uses this itemData
rg -n "itemData" apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx -A2 -B2 | head -50Repository: PackRat-AI/PackRat
Length of output: 1706
🏁 Script executed:
#!/bin/bash
# Find the createItem function definition
rg -n "createItem|useMutation|useAddItem" apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx -B2 -A2
# Look for where createItem is imported or defined
rg -n "function createItem|const createItem|export.*createItem" apps/expo/features --type ts --type tsxRepository: PackRat-AI/PackRat
Length of output: 561
🏁 Script executed:
#!/bin/bash
# Find useCreatePackItem hook
rg -n "useCreatePackItem" apps/expo/features --type ts -A30 | head -60
# Also search for it more broadly
fd "useCreatePackItem" apps/expo/featuresRepository: PackRat-AI/PackRat
Length of output: 5307
🏁 Script executed:
#!/bin/bash
# Get the full implementation of useCreatePackItem
cat -n apps/expo/features/packs/hooks/useCreatePackItem.tsRepository: PackRat-AI/PackRat
Length of output: 1606
🏁 Script executed:
#!/bin/bash
# Check useBulkAddCatalogItems to see how it handles weight (line 36 shows weight: catalogItem.weight || 0)
# This is the non-rounded approach. Let's verify if this is the pattern difference
# Check CreatePackItemForm to see how it handles weight
sed -n '1,100p' apps/expo/features/packs/screens/CreatePackItemForm.tsx | grep -n "weight" -A2 -B2
# Also check the types to understand PackItemInput
cat -n apps/expo/features/packs/types.ts | grep -A10 "PackItemInput\|interface.*Weight"Repository: PackRat-AI/PackRat
Length of output: 1186
🏁 Script executed:
#!/bin/bash
# Check the input types file
cat -n apps/expo/features/packs/types/input.ts
# Also verify the difference: compare how weight is handled in both functions
echo "=== AddCatalogItemDetailsScreen (uses displayWeight) ==="
sed -n '88,92p' apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx
echo "=== useBulkAddCatalogItems (does NOT use displayWeight) ==="
sed -n '33,37p' apps/expo/features/packs/hooks/useBulkAddCatalogItems.tsRepository: PackRat-AI/PackRat
Length of output: 707
🏁 Script executed:
#!/bin/bash
# Find the input types file
fd "input" apps/expo/features/packs/types
# Check the actual file path
cat -n apps/expo/features/packs/types.ts | grep -A20 "PackItemInput"Repository: PackRat-AI/PackRat
Length of output: 724
🏁 Script executed:
#!/bin/bash
# Find where PackItemInput is defined
rg -n "type PackItemInput|interface PackItemInput" apps/expo/features/packs
# Check if input.ts exists in types directory
ls -la apps/expo/features/packs/types/Repository: PackRat-AI/PackRat
Length of output: 228
🏁 Script executed:
#!/bin/bash
# Get the full PackItemInput definition
cat -n apps/expo/features/packs/input.tsRepository: PackRat-AI/PackRat
Length of output: 498
Persist non-rounded weight values.
displayWeight() rounds to 2 decimals, but using it at line 88 for persistence causes permanent data loss. Compare with useBulkAddCatalogItems.ts (line 36), which correctly stores the raw weight without rounding. Store the converted weight directly, and apply displayWeight() only at render-time.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsx` around
lines 88 - 95, The weight field assignment in AddCatalogItemDetailsScreen.tsx
(around line 88) is using displayWeight() to persist the weight value, which
rounds to 2 decimals and causes permanent data loss. Remove the displayWeight()
wrapper from the weight field assignment and instead store the raw normalized
weight value directly using the normalize() function. Apply displayWeight() only
at render-time when the value needs to be displayed to the user, following the
pattern used in useBulkAddCatalogItems.ts line 36.
| const alertRef = useRef<AlertMethods>(null); | ||
|
|
||
| const packWeight = currentPack?.totalWeight ?? 0; | ||
| const packWeight = convertWeight(currentPack?.totalWeight ?? 0, 'g'); |
Contributor
There was a problem hiding this comment.
Label mismatch: total weight is displayed as “Base”.
Line 19 computes packWeight from currentPack.totalWeight, but Line 44 labels it as Base. This misreports the metric in the UI.
♻️ Proposed fix
- <Text className="mr-2">{`Base: ${packWeight} ${unit}`}</Text>
+ <Text className="mr-2">{`${t('packs.totalWeight')}: ${packWeight} ${unit}`}</Text>Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/packs/components/WeightAnalysisTile.tsx` at line 19, The
packWeight variable on line 19 is computed from currentPack?.totalWeight, but on
line 44 this value is labeled as "Base" in the UI, which is a mismatch. Find the
location where the "Base" label is displayed alongside packWeight (around line
44) and update the label text to accurately reflect that it represents the total
weight of the pack, not just the base weight.
|
|
||
| const location = weather.location; | ||
| const current = weather.current; | ||
| const { displayTemperature } = useTemperatureUnit(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/expo/features/trips/screens/TripWeatherDetailsScreen.tsx | head -110Repository: PackRat-AI/PackRat
Length of output: 4277
Move hook initialization before early returns.
Line 97 calls useTemperatureUnit() after conditional returns (lines 76-93). This violates React hooks rules—conditional rendering causes the hook count to vary across renders, triggering "Rendered more hooks than during the previous render" errors.
Proposed fix
export default function TripWeatherDetailsScreen() {
const { lat, lon, locationName } = useLocalSearchParams<{
lat: string;
lon: string;
locationName?: string;
}>();
const latitude = Number(lat);
const longitude = Number(lon);
const tripLocationName = locationName;
const [weather, setWeather] = useState<WeatherAPIForecastResponse | null>(null);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
const [gradientColors, setGradientColors] = useState<[string, string, ...string[]]>([
'`#4c669f`',
'`#3b5998`',
'`#192f6a`',
]);
+ const { displayTemperature } = useTemperatureUnit();
const fetchWeather = async () => {
try {
setLoading(true);
setError(null);
@@
if (loading) {
return (
<View className="flex-1 items-center justify-center">
<ActivityIndicator />
</View>
);
}
if (error || !weather) {
return (
<View className="flex-1 items-center justify-center">
<Text>{error || 'Something went wrong'}</Text>
<TouchableOpacity onPress={fetchWeather}>
<Text>Retry</Text>
</TouchableOpacity>
</View>
);
}
const location = weather.location;
const current = weather.current;
- const { displayTemperature } = useTemperatureUnit();
// Use the trip's location name if provided, otherwise fall back to weather API location📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { displayTemperature } = useTemperatureUnit(); | |
| export default function TripWeatherDetailsScreen() { | |
| const { lat, lon, locationName } = useLocalSearchParams<{ | |
| lat: string; | |
| lon: string; | |
| locationName?: string; | |
| }>(); | |
| const latitude = Number(lat); | |
| const longitude = Number(lon); | |
| const tripLocationName = locationName; | |
| const [weather, setWeather] = useState<WeatherAPIForecastResponse | null>(null); | |
| const [loading, setLoading] = useState(true); | |
| const [error, setError] = useState<string | null>(null); | |
| const [gradientColors, setGradientColors] = useState<[string, string, ...string[]]>([ | |
| '`#4c669f`', | |
| '`#3b5998`', | |
| '`#192f6a`', | |
| ]); | |
| const { displayTemperature } = useTemperatureUnit(); | |
| const fetchWeather = async () => { | |
| try { | |
| setLoading(true); | |
| setError(null); | |
| // ... rest of fetchWeather implementation | |
| } | |
| }; | |
| if (loading) { | |
| return ( | |
| <View className="flex-1 items-center justify-center"> | |
| <ActivityIndicator /> | |
| </View> | |
| ); | |
| } | |
| if (error || !weather) { | |
| return ( | |
| <View className="flex-1 items-center justify-center"> | |
| <Text>{error || 'Something went wrong'}</Text> | |
| <TouchableOpacity onPress={fetchWeather}> | |
| <Text>Retry</Text> | |
| </TouchableOpacity> | |
| </View> | |
| ); | |
| } | |
| const location = weather.location; | |
| const current = weather.current; | |
| // Use the trip's location name if provided, otherwise fall back to weather API location |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/trips/screens/TripWeatherDetailsScreen.tsx` at line 97,
Move the useTemperatureUnit hook call to the very beginning of the
TripWeatherDetailsScreen component, before any conditional returns or early exit
statements. The hook must be initialized at the top level before any return
statements (including the early returns that exist in lines 76-93) to ensure the
hook is always called in the same order on every render, complying with React's
Rules of Hooks.
…text wrapping on Android Also refactor LocationPreviewScreen header to use Stack.Screen options API and fix LocationSearchScreen search input top spacing.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/expo/features/weather/screens/LocationPreviewScreen.tsx (1)
254-260:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTemperature range bar doesn't adjust for user's preferred unit.
The daily forecast bar positions use a hardcoded Celsius range (4-38°C) to calculate the visual placement of the low/high temperature indicators. When the user prefers Fahrenheit, the displayed temperature labels show converted values (e.g., "50°F", "68°F") via
displayTemperature(), but the bar positioning still uses the unconverted Celsius values (10, 20). This causes the visual bar to misalign with the numeric labels shown.🔧 Suggested approach
The bar range should adjust based on the user's preferred temperature unit. You'll need to:
- Import
useTemperatureUnithook (already imported at line 4)- Convert the range bounds or use unit-aware min/max values
- Apply the same unit conversion to
day.lowandday.highfor bar calculationExample approach:
const { unit: temperatureUnit, toPreferred } = useTemperatureUnit(); // Define ranges per unit const rangeMin = temperatureUnit === 'C' ? 4 : 39; // 4°C ≈ 39°F const rangeMax = temperatureUnit === 'C' ? 38 : 100; // 38°C ≈ 100°F // Convert stored Celsius values to preferred unit for bar positioning const lowInPreferredUnit = toPreferred(day.low); const highInPreferredUnit = toPreferred(day.high); // Then use these in the bar calculation left: `${Math.max(0, ((lowInPreferredUnit - rangeMin) / (rangeMax - rangeMin)) * 100)}%`, right: `${Math.max(0, 100 - ((highInPreferredUnit - rangeMin) / (rangeMax - rangeMin)) * 100)}%`,Alternatively, if
displayTemperaturereturns only the formatted string, extract the numeric conversion logic into a separate helper or add atoPreferred(celsius)function to the hook.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/weather/screens/LocationPreviewScreen.tsx` around lines 254 - 260, The temperature range bar uses hardcoded Celsius bounds (4-38°C) in the style calculations for left and right properties, but displays labels converted to the user's preferred unit via displayTemperature(), causing misalignment in Fahrenheit mode. Fix this by using the useTemperatureUnit hook (already imported) to retrieve the user's preferred temperature unit and conversion function. Define unit-specific range bounds (4°C/39°F minimum, 38°C/100°F maximum), then convert the day.low and day.high values to the preferred unit using the hook's conversion function before applying them to the left and right calculations in the style object. This ensures the bar positioning aligns with the numeric temperature labels shown to the user.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/app/`(app)/settings/index.tsx:
- Line 117: The main unit labels "Weight", "Temperature", and "Wind & Distance"
in the Display Units section are hardcoded in English while their subtitles
already use the t() function for internationalization, creating inconsistency.
Replace each of these three hardcoded label strings with corresponding t()
function calls to use the translation keys from your i18n configuration,
ensuring all user-facing text in the Display Units section is properly
internationalized.
---
Outside diff comments:
In `@apps/expo/features/weather/screens/LocationPreviewScreen.tsx`:
- Around line 254-260: The temperature range bar uses hardcoded Celsius bounds
(4-38°C) in the style calculations for left and right properties, but displays
labels converted to the user's preferred unit via displayTemperature(), causing
misalignment in Fahrenheit mode. Fix this by using the useTemperatureUnit hook
(already imported) to retrieve the user's preferred temperature unit and
conversion function. Define unit-specific range bounds (4°C/39°F minimum,
38°C/100°F maximum), then convert the day.low and day.high values to the
preferred unit using the hook's conversion function before applying them to the
left and right calculations in the style object. This ensures the bar
positioning aligns with the numeric temperature labels shown to the user.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 54c0eb0d-a53f-49ef-af3e-1c6e43e342b5
📒 Files selected for processing (4)
apps/expo/app/(app)/settings/index.tsxapps/expo/features/weather/screens/LocationPreviewScreen.tsxapps/expo/features/weather/screens/LocationSearchScreen.tsxapps/expo/lib/i18n/locales/en.json
| <View className="rounded-xl border border-border bg-card"> | ||
| <View className="flex-row items-center justify-between p-4"> | ||
| <View className="flex-1"> | ||
| <Text className="font-medium">Weight</Text> |
Contributor
There was a problem hiding this comment.
Internationalize the main unit labels for consistency.
The subtitles now use t() for i18n, but the main labels ("Weight", "Temperature", "Wind & Distance") are still hardcoded English. This creates an inconsistency within the Display Units section.
🌐 Complete the i18n integration
<View className="flex-1">
- <Text className="font-medium">Weight</Text>
+ <Text className="font-medium">{t('settings.weight')}</Text>
<Text variant="footnote" className="mt-0.5 text-muted-foreground"> <View className="flex-1">
- <Text className="font-medium">Temperature</Text>
+ <Text className="font-medium">{t('settings.temperature')}</Text>
<Text variant="footnote" className="mt-0.5 text-muted-foreground"> <View className="flex-1">
- <Text className="font-medium">Wind & Distance</Text>
+ <Text className="font-medium">{t('settings.windDistance')}</Text>
<Text variant="footnote" className="mt-0.5 text-muted-foreground">Add corresponding keys to your translation file (e.g., apps/expo/lib/i18n/locales/en.json):
{
"settings": {
"weight": "Weight",
"temperature": "Temperature",
"windDistance": "Wind & Distance"
}
}Also applies to: 134-134, 151-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/app/`(app)/settings/index.tsx at line 117, The main unit labels
"Weight", "Temperature", and "Wind & Distance" in the Display Units section are
hardcoded in English while their subtitles already use the t() function for
internationalization, creating inconsistency. Replace each of these three
hardcoded label strings with corresponding t() function calls to use the
translation keys from your i18n configuration, ensuring all user-facing text in
the Display Units section is properly internationalized.
…gle object param Fixes the no-owned-max-params CI lint violation by converting both functions from positional to object-destructured signatures, and updates all 14+ call sites across screens, components, hooks, and tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WeightBadge, weight analysis, pack stats, pack categories, current-pack) now convert to the preferred unit consistentlyTripWeatherDetailsScreen); all now store °C internally and convert to the preferred unit at render timecomputeCategorySummariesandusePackWeightAnalysiswere reading fromuserStore.preferredWeightUnit(which doesn't exist on theUsertype and always fell back to'g')Changes
packages/constantsTEMPERATURE_UNITS/TemperatureUnitpackages/schemas/src/users.tsweightUnitandtemperatureUnittoUserPreferencesSchemaapps/expo/lib/unitDefaults.tsexpo-localizationapps/expo/features/auth/hooks/useWeightUnit.tsconvertWeighthelperapps/expo/features/auth/hooks/useTemperatureUnit.tsdisplayTemperature/toPreferredhelpersapps/expo/components/initial/WeightBadgeapps/expo/app/(app)/settings/index.tsxapps/expo/features/weather/lib/weatherService.tsformatWeatherDatanow stores all temps in °CuseTemperatureUnitfor consistent conversionuseWeightUnitpreference viacomputeCategorySummariesTest plan
bun test:expopasses (363 tests)Summary by CodeRabbit