From 006ed4618df5a1d79ad9353d6c8bfa61ae3e9cf8 Mon Sep 17 00:00:00 2001 From: Peter Date: Tue, 7 Jun 2022 19:50:57 +0200 Subject: [PATCH] do not zoom if marker was dragged (#193) * do not zoom if marker was dragged * formatting --- src/actions/Actions.ts | 4 +++- src/api/graphhopper.d.ts | 1 + src/layers/UseQueryPointsLayer.tsx | 13 +++++++----- src/map/Popup.tsx | 15 ++++++++------ src/sidebar/search/Search.tsx | 28 ++++++++++++++++---------- src/stores/MapActionReceiver.ts | 2 +- src/stores/QueryStore.ts | 4 ++++ test/NavBar.test.ts | 13 +++++++----- test/routing/Api.test.ts | 7 +++++++ test/stores/QueryStore.test.ts | 32 +++++++++++++++++++----------- test/stores/RouteStore.test.ts | 2 +- 11 files changed, 79 insertions(+), 42 deletions(-) diff --git a/src/actions/Actions.ts b/src/actions/Actions.ts index 25820eff..5b071c9a 100644 --- a/src/actions/Actions.ts +++ b/src/actions/Actions.ts @@ -14,9 +14,11 @@ export class InfoReceived implements Action { export class SetPoint implements Action { readonly point: QueryPoint + readonly zoom: boolean - constructor(point: QueryPoint) { + constructor(point: QueryPoint, zoom: boolean) { this.point = point + this.zoom = zoom } } diff --git a/src/api/graphhopper.d.ts b/src/api/graphhopper.d.ts index 378930c7..cf378488 100644 --- a/src/api/graphhopper.d.ts +++ b/src/api/graphhopper.d.ts @@ -7,6 +7,7 @@ export interface RoutingArgs { readonly points: [number, number][] readonly profile: string readonly maxAlternativeRoutes: number + readonly zoom: boolean } export interface RoutingRequest { diff --git a/src/layers/UseQueryPointsLayer.tsx b/src/layers/UseQueryPointsLayer.tsx index 8fd6eebd..4a3951fe 100644 --- a/src/layers/UseQueryPointsLayer.tsx +++ b/src/layers/UseQueryPointsLayer.tsx @@ -87,11 +87,14 @@ function addDragInteractions(map: Map, queryPointsLayer: VectorLayer) { const coordinateLonLat = toLonLat(feature.getGeometry().getCoordinates()) const coordinate = { lng: coordinateLonLat[0], lat: coordinateLonLat[1] } Dispatcher.dispatch( - new SetPoint({ - ...point, - coordinate, - queryText: coordinateToText(coordinate), - }) + new SetPoint( + { + ...point, + coordinate, + queryText: coordinateToText(coordinate), + }, + false + ) ) }) modify.set('gh:drag_query_point', true) diff --git a/src/map/Popup.tsx b/src/map/Popup.tsx index c3a1ae91..21e67d26 100644 --- a/src/map/Popup.tsx +++ b/src/map/Popup.tsx @@ -21,12 +21,15 @@ export function PopupComponent({ const dispatchSetPoint = function (point: QueryPoint, coordinate: Coordinate) { onSelect() Dispatcher.dispatch( - new SetPoint({ - ...point, - coordinate: coordinate, - queryText: coordinateToText(coordinate), - isInitialized: true, - }) + new SetPoint( + { + ...point, + coordinate: coordinate, + queryText: coordinateToText(coordinate), + isInitialized: true, + }, + true + ) ) } diff --git a/src/sidebar/search/Search.tsx b/src/sidebar/search/Search.tsx index 20853c80..09311d66 100644 --- a/src/sidebar/search/Search.tsx +++ b/src/sidebar/search/Search.tsx @@ -73,17 +73,23 @@ const SearchBox = ({ onAddressSelected={(queryText, coordinate) => Dispatcher.dispatch( coordinate - ? new SetPoint({ - ...point, - isInitialized: true, - queryText: queryText, - coordinate: coordinate, - }) - : new SetPoint({ - ...point, - isInitialized: false, - queryText: queryText, - }) + ? new SetPoint( + { + ...point, + isInitialized: true, + queryText: queryText, + coordinate: coordinate, + }, + true + ) + : new SetPoint( + { + ...point, + isInitialized: false, + queryText: queryText, + }, + true + ) ) } onChange={onChange} diff --git a/src/stores/MapActionReceiver.ts b/src/stores/MapActionReceiver.ts index cd9f2c9e..7e7cf9c3 100644 --- a/src/stores/MapActionReceiver.ts +++ b/src/stores/MapActionReceiver.ts @@ -36,7 +36,7 @@ export default class MapActionReceiver implements ActionReceiver { // this assumes that always the first path is selected as result. One could use the // state of the routeStore as well but then we would have to make sure that the route // store digests this action first, which our Dispatcher can't at the moment. - fitBounds(this.map, action.result.paths[0].bbox!, isSmallScreen) + if (action.request.zoom) fitBounds(this.map, action.result.paths[0].bbox!, isSmallScreen) } else if (action instanceof SetSelectedPath) { fitBounds(this.map, action.path.bbox!, isSmallScreen) } else if (action instanceof PathDetailsRangeSelected) { diff --git a/src/stores/QueryStore.ts b/src/stores/QueryStore.ts index 6bd7293e..27066653 100644 --- a/src/stores/QueryStore.ts +++ b/src/stores/QueryStore.ts @@ -27,6 +27,7 @@ export interface QueryStoreState { readonly currentRequest: CurrentRequest readonly maxAlternativeRoutes: number readonly routingProfile: RoutingProfile + readonly zoom: boolean } export interface QueryPoint { @@ -81,6 +82,7 @@ export default class QueryStore extends Store { routingProfile: { name: '', }, + zoom: true, } } @@ -112,6 +114,7 @@ export default class QueryStore extends Store { const newState: QueryStoreState = { ...state, queryPoints: QueryStore.replacePoint(state.queryPoints, action.point), + zoom: action.zoom, } return this.routeIfAllPointsSet(newState) @@ -297,6 +300,7 @@ export default class QueryStore extends Store { points: coordinates, profile: state.routingProfile.name, maxAlternativeRoutes: state.maxAlternativeRoutes, + zoom: state.zoom, } } diff --git a/test/NavBar.test.ts b/test/NavBar.test.ts index 3078ceac..ac825f61 100644 --- a/test/NavBar.test.ts +++ b/test/NavBar.test.ts @@ -104,7 +104,7 @@ describe('NavBar', function () { // modify state of stores which the nav bar depends on for (const point of points) { - queryStore.receive(new SetPoint(point)) + queryStore.receive(new SetPoint(point, true)) } queryStore.receive(new SetVehicleProfile(profile)) mapStore.receive(new SelectMapStyle(style)) @@ -190,10 +190,13 @@ describe('NavBar', function () { href: 'https://origin.com', } Dispatcher.dispatch( - new SetPoint({ - ...queryStore.state.queryPoints[0], - isInitialized: true, - }) + new SetPoint( + { + ...queryStore.state.queryPoints[0], + isInitialized: true, + }, + true + ) ) //act diff --git a/test/routing/Api.test.ts b/test/routing/Api.test.ts index fdec7a39..103f5361 100644 --- a/test/routing/Api.test.ts +++ b/test/routing/Api.test.ts @@ -79,6 +79,7 @@ describe('route', () => { points: [], maxAlternativeRoutes: 1, profile: 'profile', + zoom: true, } const mockedDispatcher = jest.spyOn(Dispatcher, 'dispatch') const ghApi = 'https://some.api/' @@ -105,6 +106,7 @@ describe('route', () => { points: [], maxAlternativeRoutes: 1, profile: 'car', + zoom: true, } const expectedBody: RoutingRequest = { @@ -138,6 +140,7 @@ describe('route', () => { points: [], maxAlternativeRoutes: 2, profile: 'car', + zoom: true, } const expectedBody: RoutingRequest = { @@ -177,6 +180,7 @@ describe('route', () => { ], maxAlternativeRoutes: 1, profile: 'bla', + zoom: true, } fetchMock.mockResponseOnce(JSON.stringify(getEmptyResult())) @@ -197,6 +201,7 @@ describe('route', () => { ], maxAlternativeRoutes: 1, profile: 'bla', + zoom: true, } const error: ErrorResponse = { @@ -219,6 +224,7 @@ describe('route', () => { profile: 'car', points: [], maxAlternativeRoutes: 3, + zoom: true, } fetchMock.mockResponse(() => Promise.resolve({ status: 500 })) await expect(new ApiImpl('https://some.api/', 'key').route(args)).rejects.toThrow('Route calculation timed out') @@ -231,6 +237,7 @@ describe('route', () => { points: [], maxAlternativeRoutes: 1, profile: 'car', + zoom: true, } const expectedBody: RoutingRequest = { diff --git a/test/stores/QueryStore.test.ts b/test/stores/QueryStore.test.ts index 8fd33d0b..6f1bc749 100644 --- a/test/stores/QueryStore.test.ts +++ b/test/stores/QueryStore.test.ts @@ -56,7 +56,7 @@ describe('QueryStore', () => { routingProfile: { name: 'car' }, } - const state = store.reduce(storeState, new SetPoint(point)) + const state = store.reduce(storeState, new SetPoint(point, true)) expect(state.queryPoints[0]).toEqual(point) }) @@ -69,7 +69,7 @@ describe('QueryStore', () => { } for (const point of store.state.queryPoints) { - state = store.reduce(state, new SetPoint({ ...point, isInitialized: true })) + state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }, true)) } // the store should not send anything unless points and routing profile are specified @@ -88,7 +88,7 @@ describe('QueryStore', () => { routingProfile: { name: 'car' }, } for (const point of store.state.queryPoints) { - state = store.reduce(state, new SetPoint({ ...point, isInitialized: true })) + state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }, true)) } expect(state.queryPoints.every(point => point.isInitialized)).toBeTruthy() @@ -106,7 +106,7 @@ describe('QueryStore', () => { } state.queryPoints.push({ ...state.queryPoints[0], id: 2 }) for (const point of store.state.queryPoints) { - state = store.reduce(state, new SetPoint({ ...point, isInitialized: true })) + state = store.reduce(state, new SetPoint({ ...point, isInitialized: true }, true)) } expect(state.queryPoints.every(point => point.isInitialized)).toBeTruthy() @@ -273,17 +273,23 @@ describe('QueryStore', () => { // initialize all query points so that the store will issue a route request. state = store.reduce( state, - new SetPoint({ - ...state.queryPoints[0], - isInitialized: true, - }) + new SetPoint( + { + ...state.queryPoints[0], + isInitialized: true, + }, + true + ) ) state = store.reduce( state, - new SetPoint({ - ...state.queryPoints[1], - isInitialized: true, - }) + new SetPoint( + { + ...state.queryPoints[1], + isInitialized: true, + }, + true + ) ) state = store.reduce( state, @@ -323,6 +329,7 @@ describe('QueryStore', () => { maxAlternativeRoutes: 1, points: [], profile: 'some-profile', + zoom: true, } const subRequest: SubRequest = { state: RequestState.SENT, @@ -350,6 +357,7 @@ describe('QueryStore', () => { maxAlternativeRoutes: 1, points: [], profile: 'some-profile', + zoom: true, } const subRequest: SubRequest = { state: RequestState.SENT, diff --git a/test/stores/RouteStore.test.ts b/test/stores/RouteStore.test.ts index 0be329c1..6a8cc563 100644 --- a/test/stores/RouteStore.test.ts +++ b/test/stores/RouteStore.test.ts @@ -12,7 +12,7 @@ describe('RouteStore', () => { describe('clear route information', () => { it('should revert to initial state on ClearPoints Action', () => { - executeTest(new SetPoint(createEmptyQueryPoint())) + executeTest(new SetPoint(createEmptyQueryPoint(), true)) }) it('should revert to initial state on ClearRoute Action', () => { executeTest(new ClearRoute())