nav visual instructions base ready #25
Conversation
…atched into store.
changed main display from flex.
… wp maneuver type + wp name.
| @@ -5,4 +5,8 @@ | |||
| <div class="settingsButtonContainer"> | |||
| <button mat-raised-button (click)="startNavigation()">Start</button> | |||
| </div> | |||
| <div class="remaining-distance-display-container"> | |||
| <span class="remain-span">{{distanceToEndpoint$ | async}} </span> | |||
There was a problem hiding this comment.
This should be displayed as integer and in quantizations (for distance of 10-100km should update every 10km, for 1-10km show update every 1km, for 100-1000m show update every 100m an so on).
Also, the display unit should be shown (km/m)
Please add an issue for this
There was a problem hiding this comment.
#33
please advise - this is not the way to our main competitor works.
| constructor(private store: Store<any>) { } | ||
|
|
||
| ngOnInit() { | ||
| this.distanceToEndpoint$ = this.store.pipe(select(distanceToEndpointSelector)); |
There was a problem hiding this comment.
No reason to be on init.
Can be in constructor or before on the variables
| } | ||
|
|
||
| openSearchPage() { | ||
| this.store.dispatch(new SetShowSearchAction({isShowSearch: true})); | ||
| } | ||
|
|
||
| startNavigation() { | ||
| this.store.dispatch(new SetActiveNavAction({isActiveNav: true})); | ||
| this.store.dispatch(new SetPhrazeStateAction({phrazeState: PhrazeState.NAVIGATION})); | ||
| this.store.dispatch(new SetNextWaypointIndexAction({nextWaypointIndex: 1})); |
There was a problem hiding this comment.
Maybe should be part of set state reducer in order to avoid two actions
Also order matters, next should be before state
| @@ -0,0 +1,4 @@ | |||
| <div class="instructions-container"> | |||
| in {{distanceToNextWaypoint$ | async}} {{nextWaypointManeuver$ | async}} to {{nextWaypointName$ | async}} | |||
There was a problem hiding this comment.
Same remark of how to display km/m in proper quantizations.
Maneuver should be shown as image or at least separated to words
|
|
||
| ngOnInit() { | ||
|
|
||
| this.distanceToNextWaypoint$ = this.store.pipe(select(nextWaypointDistanceSelector)); |
There was a problem hiding this comment.
Should also be shown only on navigation and otherwise hidden/some value
| @@ -13,11 +14,43 @@ export class RoutesService { | |||
| constructor(@Inject(BINGPROVIDER_CONFIG) public bingProviderConfig: BingProviderInterface) { } | |||
|
|
|||
| getRoute(from, to): Observable<any> { | |||
There was a problem hiding this comment.
Refactor to using store action/effects instead of direct call
| } | ||
|
|
||
| const resultSubject = new Subject<any>(); | ||
| const routeDetails: RouteDetails = new class implements RouteDetails { |
There was a problem hiding this comment.
no need for new class, just object ({ ... }) with the required properties.
There is no need for this overhead.
| routeDetails.routeDuration = routeBase.travelDuration; | ||
| routeDetails.routeLegs = indexLegs(routeBase.routeLegs); | ||
| routeDetails.routeLegs = mapLegs(routeBase.routeLegs[0].itineraryItems); | ||
| routeDetails.routeLegs = indexLegs(routeDetails.routeLegs, routeDetails.routePoints); |
There was a problem hiding this comment.
indexLegs just adds/update the value, it's shouldn't override the routeLegs. i.e. routeLegs should be created one
| SetPhrazeStateAction, | ||
| SetRouteAction, | ||
| SetShowSearchAction | ||
| } from './nav.actions'; | ||
|
|
||
|
|
||
| export const NavState: NavInterface = { | ||
| routeDetails: { |
There was a problem hiding this comment.
default value is usually separated to another file (nav.init.ts)
| const nextWaypointDistance = (action as SetNextWaypointDistanceAction).payload.nextWaypointDistance; | ||
| return { ...state, nextWaypointDistance: nextWaypointDistance }; | ||
| } | ||
| case navActionTypes.SET_DISTANCE_TO_ENDPOINT: { |
There was a problem hiding this comment.
This can be part of SET_NEXT_WAYPOINT_DISTANCE as overall distance doesn't change without distance to next way point
No description provided.