Distance component with metric support #885
Conversation
| @@ -0,0 +1,39 @@ | |||
| export interface DistanceParams { | |||
| unit: "meter" | "kilometer" | "foot" | "mile"; | |||
There was a problem hiding this comment.
Can you make this union type its own type? It could be useful for a consumer to have access to that type in case a component is built around this that accepts the unit on its own for some reason.
There was a problem hiding this comment.
I'm seeing now that this is only used internally and isn't exposed in any props, so this is actually fine!
| import MapillaryButton from "./mapillary-button"; | ||
|
|
||
| interface Props { | ||
| imperial: boolean; |
There was a problem hiding this comment.
Are you sure it makes the most sense to make this the prop, rather than and optional metric prop? Since we defaulted to imperial before, we could avoid this being a breaking change right?
| }; | ||
|
|
||
| export const isImperial = (config: Config): boolean => | ||
| !config.units || config.units === "imperial"; |
There was a problem hiding this comment.
Regarding my previous comment about using a metric boolean rather than imperial, maybe it would be better to just pass the string from config.units around instead, instead of creating an intermediate format that gets passed around? Stylistically I'm not sure which is better. I'd probably have just passed the string around personally.
| {leg.steps.map((step, k) => ( | ||
| <S.LegDetail key={k}> | ||
| <S.AccessLegStep step={step} /> | ||
| <S.AccessLegStep imperial={imperial} step={step} /> |
There was a problem hiding this comment.
Like, this line could just be units={config.units} and then the string comparison can be done once inside AccessLegStep.
| * The preferred unit system to use, either metric or imperial. | ||
| * The default is 'imperial' so that existing visuals with distances in feet/miles don't break. | ||
| */ | ||
| units?: "metric" | "imperial"; |
There was a problem hiding this comment.
Maybe this union type is the one that should be made into its own type and used instead of the boolean?
daniel-heppner-ibigroup
assigned
binh-dam-ibigroup and unassigned
daniel-heppner-ibigroup
2 participants
This PR introduces a
Distancecomponent with support of imperial and metric units. A new app config option is also introduced to display units asmetricorimperial.This PR should result in a new minor version number for the following packages:
Fix #116