fix: support distance display in metric units

[doncicuto]

Hi there, the following PR deals with issue #116

I've added a useMetricUnits props to the affected components reported in the issue:

• AccessLegSummary (package itinerary-body)
• AccessLeg (package printable-itinerary)
• TransitStopOption (package location-field)

This prop is optional with default value false which means that imperial units will be used. If useMetricUnits is set, metric units will be used instead.

I've also added some stories to test useMetricUnits in action:

As this is my first ever contribution to this project so I hope this first contribution is useful. Please don't hesitate to tell me what I should improve or change for my future contributions.

This an awesome project, thank's a lot for your effort and the quality of the code. It's great that you've published it as an open source project, hopefully I'll try to upload more contributions soon.

Cheers!

[miles-grant-ibigroup]

Thanks so much for contributing! And thanks so much for your kind words about the project. Your prop-based solution is well-implemented, but I think we're hoping to use our localization library to implement this feature

[doncicuto]

Hi @miles-grant-ibigroup, sorry I didn't know about your localization library. Should I close this PR, do you want me to check that localization library? Let me know if I can be of any help. Thanks

[miles-grant-ibigroup]

Hi @miles-grant-ibigroup, sorry I didn't know about your localization library. Should I close this PR, do you want me to check that localization library? Let me know if I can be of any help. Thanks

You can keep this PR open! We use react-intl. This exposes an intl object that should contain the current locale. Figuring out whether to use metric or imperial based on this locale is the challenge. Perhaps a fixed list of imperial languages (en-US)?

I hope this is helpful. This is on our backlog to do ourselves, but we probably won't get to it soon.

[doncicuto]

Hi there. It seems that only three countries still use the imperial units so I can hardcode those countries and use react-intl. If you are not in a hurry I'd like to work on this issue.

[miles-grant-ibigroup]

Hi there. It seems that only three countries still use the imperial units so I can hardcode those countries and use react-intl. If you are not in a hurry I'd like to work on this issue.

That sounds great! Please do it's much appreciated. I'll be here to review when you're ready :)

[doncicuto]

Great @miles-grant-ibigroup. How about using this?

  // Reference: https://en.wikipedia.org/wiki/Mile
  // While most countries abandoned the mile when switching to the metric system,
  // the international mile continues to be used in some countries, such as Liberia,
  // Myanmar, the United Kingdom and the United States. It is also used in a number
  // of territories with less than a million inhabitants, most of which are UK or US territories,
  // or have close historical ties with the UK or US: American Samoa, Bahamas, Belize,
  // British Virgin Islands, Cayman Islands, Dominica, Falkland Islands, Grenada,Guam,
  // The N. Mariana Islands, Samoa, St. Lucia, St. Vincent & The Grenadines, St. Helena,
  // St. Kitts & Nevis, the Turks & Caicos Islands, and the US Virgin Islands.
  // The mile is even encountered in Canada, though this is predominantly in rail transport
  // and horse racing, as the roadways have been metricated since 1977.
  const useMetricUnits = [
    "en-US",
    "en-GB",
    "en-LR",
    "en-AS",
    "sm-AS",
    "en-BS",
    "en-BZ",
    "es-BZ",
    "en-VG",
    "en-KY",
    "en-DM",
    "en-GD",
    "en-GU",
    "en-MP",
    "en-WS",
    "sm-WS",
    "en-LC",
    "en-VC",
    "en-SH",
    "en-KN",
    "en-TC",
    "en-VI",
    "my-MM"
  ].includes(intl.locale);

[doncicuto]

Hi @miles-grant-ibigroup I've modified the PR adding a function to check if according to react-intl locale we have to use metric or imperial units. Now fewer files are modified.

Here are two screenshots showing the changes of locale in action and how the units are changed.

Default locale (en-US):

Using Spanish:

Also, hope you don't mind, as I'm a Spaniard I've changed some entries for the itinerary-body es.yaml

Please let me know if you want me to change anything

Cheers!

[miles-grant-ibigroup]

This is a fantastic contribution thank you so much! I really appreciate your translation updates as well.

I have one review comment that would clean up the code. Once that's done I'll assign to a second reviewer! We do 2 reviews per PR -- once the second reviewer approves we will merge!

[miles-grant-ibigroup]

Would it be possible to refactor humanizeDistanceString to use the intl object it gets anyway to avoid having to import coreUtils everywhere?

[miles-grant-ibigroup]

I'd say don't remove the manual override (for backwards compatibility) but instead use it as an override -- a way to force metric even if the locale isn't metric

[miles-grant-ibigroup]

As for the failing unit tests running yarn unit -u will update the snapshots!

[doncicuto]

Thanks @miles-grant-ibigroup I'll prepare the changes