Imperial system

[PierreBresson]

Summary

As an english citizen or ex british colony citizen, I want to be able to use the app in the imperial system instead of the international system of units (pounds instead of kilos, miles instead of kilometers...)

Proposed feature

Imperial units everywhere in the app + add in the settings a row with instead of a chevron, a switch, that will enable/disable that feature.

To Do List

Checklist, nice to have, but not mandatory

• [ ] propose a way to switch from metric to imperial in the settings, it can be only the initial mechanism and the task could be once we agree on the way to go, divided in small chunks

[ThomasLatham]

Hi @PierreBresson, I'd like to give this a shot. Can you please assign this to me?

[PierreBresson]

Hi @ThomasLatham, thanks having a look, let me know if you have any questions ;)

[ThomasLatham]

Okay, thanks!

[ThomasLatham]

So far I have:

• created a component (\<ListItemSwitch>) that's essentially \<ListItem> but with a Switch instead of a chevron. Also, instead of the onPress prop, it has onChange and value props. I haven't yet written tests or stories for this component yet.

• updated the userPreferences slice and selectors (and corresponding tests) to include a useMetricUnits boolean in the Redux store with a default value of true. The toggleUnits reducer, like toggleNotification, receives the desired units setting as a boolean payload in an action, rather than just receiving state and setting useMetricUnits to the opposite of its current value.

• created a new row in the Settings screen under About, with the English title "Metric units" (the translations were updated for all languages to reflect this, though). It is a \<ListItemSwitch>, receiving useMetricUnits's selector as its value prop and, for its onChange prop, a dispatch() of the toggleUnits action creator's returned action with the opposite of the value prop as its payload. I have not updated the Settings snapshot to reflect this.

All of this is to have a switch-row in settings that controls some application state for unit preferences. I haven't implemented anything that would apply that state where applicable because I want to make sure that what I have is okay first (and write tests for everything if so). Is there a way that this code can be reviewed without submitting a PR?

[PierreBresson]

Sounds like a good plan. GitHub doesn't support draft pr I think, so you can just submit the pr as it is, I will have a look and help if needed

[ThomasLatham]

Now that the settings switch and Redux stuff is set up, I'm going to start on the task of having units change throughout the app according to the useMetricUnits state. My plan right now is to keep that change as close to the user as possible, so to speak, so that only the display changes but behind the scenes everything is still in metric regardless of the user's unit preference (if that makes any sense). For example,

https://github.com/NMF-earth/nmf-app/blob/b5f43f9cafd86894e501e41d6840f0042644304c/app/screens/AddEmission/components/Electricity/Electricity.tsx#L57-L63

could become something like

<Text.H2 darkGray> <FormattedNumber value={ sliderValue * electricity[electricityCountry] * (useMetricUnits ? 1 : 2.205)} maximumFractionDigits={2} />{" "} <Text.Primary>{useMetricUnits ? "kgCO2eq" : "lbsCO2eq"}</Text.Primary> </Text.H2>

I don't know if there's a software development term for this kind of approach. This would also mean new features involving units would have to follow the same pattern. I'm definitely open to something more elegant and DRY though, or just better in some aspect that isn't immediately apparent to me. @PierreBresson what are your thoughts on this?

[PierreBresson]

@ThomasLatham I think the general idea is good. It could be slightly improved by using some kind of converter function, so there would be no need for copying and pasting useMetricUnits ? 1 : 2.205 in several part of the apps. So, something like this :

<Text.H2 darkGray>
  <FormattedNumber
    value={getImperialMetricValue(sliderValue * electricity[electricityCountry],useMetricUnits)}
    maximumFractionDigits={2}
  />{" "}
  <Text.Primary>{useMetricUnits ? "kgCO2eq" : "lbsCO2eq"}</Text.Primary>
</Text.H2>

If useMetricUnits is true, then getImperialMetricValue returns sliderValue * electricity[electricityCountry] If useMetricUnits is false, then getImperialMetricValue returns sliderValue * electricity[electricityCountry] * 2.205

Feel free to use a 3rd party library for the conversion, like this one or another. There is probably a better way of doing this, but it will be okay for now

[ThomasLatham]

Oh okay cool! Do you think getImperialMetricValue should take in a string indicating the physical quantity for which units are being converted (length and mass are the only two I've come across)? Or perhaps separate functions for different physical quantities? Also I'm guessing app/utils/calculation.ts would be a good places for this/these function/s?

[PierreBresson]

Correct, I forget a third parameter, which could be an enum, something like MeasureType { mass = 'mass', length = 'length' } and then getImperialMetricValue(sliderValue * electricity[electricityCountry],useMetricUnits,ImperialMetricType.mass) Yeap, calculation.ts is a good place for MeasureType & getImperialMetricValue

[ThomasLatham]

Okay neat. Thanks for the help!

[ThomasLatham]

Hi @PierreBresson, I've been tinkering away at this when I find the time, and I'm ready to commit changes for implementing preferred units during emission adding, but for some reason 10 unit tests are failing in app/screens/Budget/ducks/tests/BudgetScreen.selectors.test.ts. All the tests therein related to monthly carbon values are failing, with the received value being twice the expected (except in the case of electricity carbon value, where the received value is 3 and the expected is 2). I've manually traced the function calls, but I'm just not seeing where such doubling is happening down the received path -- or halving down the expected path. I'm also unsure how to use a debugger for Jest tests in a React Native project that uses Expo, but I'm looking into it. I stashed my changes such that I'm back to the state the app was in just before I submitted the draft PR for the units toggle, but still the same 10 tests are failing. I probably made some rookie mistake somewhere, but I haven't a clue where. Could you please help me out?

[PierreBresson]

Hey, Correct, the CI is currently not working for some reason and not able to catch errors on main branch. I've opened an issue and will have a look at it. In the meantime, feel free to push your changes, I will push a fix soon hopefully, but it's not anything wrong on your side.

[PierreBresson]

@ThomasLatham I've fixed the tests on main branch, you can merge main it into your branch, it should work. fyi : it was a january bug, since some emissions in tests are only "active" at the beginning of the year 😅

[ThomasLatham]

@PierreBresson Thanks for looking into and fixing this so fast. I had a feeling it might have something to do with the Y2k+23 switch haha. Sorry I haven't been doing much with this; getting back making some progress on this feature today!

[PierreBresson]

https://github.com/NMF-earth/nmf-app/pull/365