search

NMF-earth / nmf-app

Understand and reduce your carbon footprint 🌱 iOS & Android.
https://nmf.earth
GNU General Public License v3.0
498 stars 156 forks source link

wrap AddEmissions navigation in a function to fix currying error #193

Closed adamfitzgibbon closed 3 years ago

adamfitzgibbon commented 3 years ago

Addresses #135

I figured out that the currying that is done when calling the navigation in combination with React automatically passing in the button pressed event into onPressed was causing params to be that unserializable event rather than an empty object. Wrapping the call in a function prevents that behavior and allows for a params object to be passed in as an argument if we wanted to actually pass some in in the future.

This might be a problem in other areas where you're directly passing the navigation function into an onPressed prop, but didn't want to blow up the scope of the issue.

Also, I had to make some changes to the project config to be able to commit from Windows.

  1. I had to add endOfLine: "auto" to the .prettierrc.js file or the different style of line endings caused prettier errors in every file.
  2. I didn't commit this change, but I had to temporarily change the test script in project.json 
    "test": "SET TZ=GMT && jest",

    I didn't want to break the script for other machines which is why I didn't include it, but this could be a bother for other people developing on Windows. Do you mind if I submit an issue to look into making this completely cross-compatible?

UPDATE: while the above second step does let you run yarn test on windows, it doesn't successfully set the timezone causing my snapshots to get incorrectly updated. Searching around, it doesn't seem like there's a good way to set the TZ env var in windows prior to running jest 😢 For now, it suffices to update the snapshots and then revert any timezone changes.

adamfitzgibbon commented 3 years ago

~Also I'm not sure why the snapshot is failing, as my yarn test is successful locally after updating the snapshot. Any ideas here? I'm noticing the only updates to the snapshot are updating the timezones on some of the dates, maybe my proposed change to the timezone env var in the test script is not successfully working?~

Disregard, see the update in the above description. It was in fact my local time zone causing the issues.

PierreBresson commented 3 years ago

@adamfitzgibbon yes, feel free to create an issue for windows users :)